Skip to content

Commit 8087fc2

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
ESLint: fix cannot read property type of undefined error
I got this error in my editor a few times when editing TimelineUIUtils.ts. I'm not sure exactly where from, but these test cases added in this CL failed and recreated it. This CL makes some of the checks slightly stricter to catch and early exit. Bug: none Change-Id: Ie849179dc18a3acb7c26bb95e7b0ea8d98e776c0 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7557440 Auto-Submit: Jack Franklin <jacktfranklin@chromium.org> Commit-Queue: Danil Somsikov <dsv@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
1 parent d59296a commit 8087fc2

4 files changed

Lines changed: 25 additions & 5 deletions

File tree

scripts/eslint_rules/lib/no-imperative-dom-api/ast.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ export function getEnclosingExpression(node: Node): Node|null {
6363
return null;
6464
}
6565

66-
export function getEnclosingProperty(node: Node): Node|null {
66+
export function getEnclosingProperty(node: Node|undefined): Node|null {
67+
if (!node) {
68+
return null;
69+
}
6770
if (isMemberExpression(
6871
node, n => n.type === 'ThisExpression', n => ['Identifier', 'PrivateIdentifier'].includes(n.type))) {
6972
return node;

scripts/eslint_rules/lib/no-imperative-dom-api/dom-api.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,20 @@ export const domApi: RuleCreator = {
6868
if (isIdentifier(property, 'setAttribute')) {
6969
const attribute = firstArg;
7070
const value = secondArg;
71-
if (attribute.type === 'Literal' && value && value.type !== 'SpreadElement' && attribute.value) {
71+
if (attribute?.type === 'Literal' && value && value.type !== 'SpreadElement' && attribute.value) {
7272
domFragment.attributes.push({key: attribute.value.toString(), value});
7373
return true;
7474
}
7575
}
76-
if (isIdentifier(property, 'appendChild')) {
76+
if (isIdentifier(property, 'appendChild') && firstArg) {
7777
domFragment.appendChild(firstArg, sourceCode);
7878
return true;
7979
}
8080
if (domFragment.tagName === 'select' && isIdentifier(property, 'add')) {
8181
if (secondArg) {
8282
const index = domFragment.children.indexOf(DomFragment.getOrCreate(secondArg, sourceCode));
8383
domFragment.insertChildAt(secondArg, index, sourceCode);
84-
} else {
84+
} else if (firstArg) {
8585
domFragment.appendChild(firstArg, sourceCode);
8686
}
8787
return true;

scripts/eslint_rules/lib/no-imperative-dom-api/dom-fragment.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,10 @@ export class DomFragment {
277277
}
278278
}
279279

280-
function getEnclosingVariable(node: Node, sourceCode: SourceCode): Variable|null {
280+
function getEnclosingVariable(node: Node|undefined, sourceCode: SourceCode): Variable|null {
281+
if (!node) {
282+
return null;
283+
}
281284
if (node.type === 'Identifier') {
282285
let scope: Scope|null = sourceCode.getScope(node);
283286
const variableName = node.name;

scripts/eslint_rules/tests/no-imperative-dom-api.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@ new RuleTester().run('no-imperative-dom-api', rule, {
1616
}
1717
}`,
1818
},
19+
{
20+
filename: 'front_end/ui/components/component/file.ts',
21+
code: `
22+
const el = document.createElement('div');
23+
el.appendChild();
24+
`,
25+
},
26+
{
27+
filename: 'front_end/ui/components/component/file.ts',
28+
code: `
29+
const el = document.createElement('div');
30+
el.setAttribute();
31+
`,
32+
},
1933
],
2034

2135
invalid: [

0 commit comments

Comments
 (0)