Skip to content

Commit

Permalink
prefer-dom-node-{append,remove}: Check optional chaining (#2061)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker authored Apr 7, 2023
1 parent 0f4f492 commit 443999b
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 2 deletions.
1 change: 1 addition & 0 deletions rules/prefer-dom-node-append.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const selector = [
methodCallSelector({
method: 'appendChild',
argumentsLength: 1,
includeOptionalMember: true,
}),
notDomNodeSelector('callee.object'),
notDomNodeSelector('arguments.0'),
Expand Down
3 changes: 2 additions & 1 deletion rules/prefer-dom-node-remove.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const selector = [
methodCallSelector({
method: 'removeChild',
argumentsLength: 1,
includeOptionalMember: true,
}),
notDomNodeSelector('callee.object'),
notDomNodeSelector('arguments.0'),
Expand Down Expand Up @@ -52,7 +53,7 @@ const create = context => {
return fixer.replaceText(node, `${childNodeText}.remove()`);
};

if (!hasSideEffect(parentNode, sourceCode) && isValueNotUsable(node)) {
if (!hasSideEffect(parentNode, sourceCode) && isValueNotUsable(node) && !node.callee.optional) {
problem.fix = fix;
} else {
problem.suggest = [
Expand Down
7 changes: 6 additions & 1 deletion rules/utils/is-value-not-usable.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
'use strict';

module.exports = ({parent}) => !parent || parent.type === 'ExpressionStatement';
module.exports = node =>
node.parent.type === 'ExpressionStatement'
|| (
node.parent.type === 'ChainExpression'
&& node.parent.parent.type === 'ExpressionStatement'
);
11 changes: 11 additions & 0 deletions test/prefer-dom-node-append.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ test({
'parent.appendChild(one, two);',
'parent.appendChild();',
'parent.appendChild(...argumentsArray)',
// Optional call
'parent.appendChild?.(child)',
// `callee.object` is not a DOM Node,
...notDomNodeTypes.map(data => `(${data}).appendChild(foo)`),
// First argument is not a DOM Node,
Expand Down Expand Up @@ -117,5 +119,14 @@ test({
code: 'foo(bar = node.appendChild(child))',
errors: [error],
},
{
code: 'node?.appendChild(child);',
output: 'node?.append(child);',
errors: [error],
},
{
code: '() => node?.appendChild(child)',
errors: [error],
},
],
});
7 changes: 7 additions & 0 deletions test/prefer-dom-node-remove.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ test({
'parentNode.removeChild(bar, extra);',
'parentNode.removeChild();',
'parentNode.removeChild(...argumentsArray)',
// Optional call
'parentNode.removeChild?.(foo)',

// `callee.object` is not a DOM Node,
...notDomNodeTypes.map(data => `(${data}).removeChild(foo)`),
Expand Down Expand Up @@ -245,5 +247,10 @@ test({
code: 'foo[doSomething()].removeChild(child)',
suggestionOutput: 'child.remove()',
},
// Optional parent
{
code: 'parentNode?.removeChild(foo)',
suggestionOutput: 'foo.remove()',
},
].map(options => invalidTestCase(options)),
});

0 comments on commit 443999b

Please sign in to comment.