Skip to content

Commit 096fead

Browse files
brettz9brettz9
authored
prefer-modern-dom-apis: Only fix when expression is not used (#503)
1 parent fb0268b commit 096fead

File tree

6 files changed

+101
-62
lines changed

6 files changed

+101
-62
lines changed

‎rules/prefer-modern-dom-apis.js‎

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22
const getDocumentationUrl = require('./utils/get-documentation-url');
3+
const isValueNotUsable = require('./utils/is-value-not-usable');
34

45
const getArgumentNameForReplaceChildOrInsertBefore = nodeArguments => {
56
if (nodeArguments.type === 'Identifier') {
@@ -12,14 +13,6 @@ const forbiddenIdentifierNames = new Map([
1213
['insertBefore', 'before']
1314
]);
1415

15-
const isPartOfVariableAssignment = nodeParentType => {
16-
if (nodeParentType === 'VariableDeclarator' || nodeParentType === 'AssignmentExpression') {
17-
return true;
18-
}
19-
20-
return false;
21-
};
22-
2316
const checkForReplaceChildOrInsertBefore = (context, node) => {
2417
const identifierName = node.callee.property.name;
2518

@@ -42,24 +35,22 @@ const checkForReplaceChildOrInsertBefore = (context, node) => {
4235
}
4336

4437
const parentNode = node.callee.object.name;
45-
// This check makes sure that only the first method of chained methods with same identifier name e.g: parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta); gets transformed
38+
// This check makes sure that only the first method of chained methods with same identifier name e.g: parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta); gets reported
4639
if (!parentNode) {
4740
return;
4841
}
4942

5043
const preferredSelector = forbiddenIdentifierNames.get(identifierName);
5144

52-
let fix = fixer => fixer.replaceText(
53-
node,
54-
`${oldChildNodeArgument}.${preferredSelector}(${newChildNodeArgument})`
55-
);
56-
57-
// Report error when the method is part of a variable assignment
58-
// but don't offer to autofix `.replaceWith()` and `.before()`
59-
// which don't have a return value.
60-
if (isPartOfVariableAssignment(node.parent.type)) {
61-
fix = undefined;
62-
}
45+
const fix = isValueNotUsable(node) ?
46+
// Report error when the method is part of a variable assignment
47+
// but don't offer to autofix `.replaceWith()` and `.before()`
48+
// which don't have a return value.
49+
fixer => fixer.replaceText(
50+
node,
51+
`${oldChildNodeArgument}.${preferredSelector}(${newChildNodeArgument})`
52+
) :
53+
undefined;
6354

6455
return context.report({
6556
node,
@@ -112,18 +103,16 @@ const checkForInsertAdjacentTextOrInsertAdjacentElement = (context, node) => {
112103
nodeArguments[1]
113104
);
114105

115-
let fix = fixer =>
116-
fixer.replaceText(
117-
node,
118-
`${referenceNode}.${preferredSelector}(${insertedTextArgument})`
119-
);
120-
121-
// Report error when the method is part of a variable assignment
122-
// but don't offer to autofix `.insertAdjacentElement()`
123-
// which don't have a return value.
124-
if (identifierName === 'insertAdjacentElement' && isPartOfVariableAssignment(node.parent.type)) {
125-
fix = undefined;
126-
}
106+
const fix = identifierName === 'insertAdjacentElement' && !isValueNotUsable(node) ?
107+
// Report error when the method is part of a variable assignment
108+
// but don't offer to autofix `.insertAdjacentElement()`
109+
// which doesn't have a return value.
110+
undefined :
111+
fixer =>
112+
fixer.replaceText(
113+
node,
114+
`${referenceNode}.${preferredSelector}(${insertedTextArgument})`
115+
);
127116

128117
return context.report({
129118
node,

‎rules/prefer-node-append.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22
const getDocumentationUrl = require('./utils/get-documentation-url');
3-
const isValueUsed = require('./utils/is-value-used');
3+
const isValueNotUsable = require('./utils/is-value-not-usable');
44

55
const getMethodName = memberExpression => memberExpression.property.name;
66

@@ -10,7 +10,7 @@ const create = context => {
1010
const {callee} = node;
1111

1212
if (callee.type === 'MemberExpression' && getMethodName(callee) === 'appendChild') {
13-
const fix = isValueUsed(node) ? undefined : fixer => fixer.replaceText(callee.property, 'append');
13+
const fix = isValueNotUsable(node) ? fixer => fixer.replaceText(callee.property, 'append') : undefined;
1414

1515
context.report({
1616
node,

‎rules/prefer-node-remove.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22
const getDocumentationUrl = require('./utils/get-documentation-url');
3-
const isValueUsed = require('./utils/is-value-used');
3+
const isValueNotUsable = require('./utils/is-value-not-usable');
44

55
const getMethodName = callee => {
66
const {property} = callee;
@@ -66,7 +66,7 @@ const create = context => {
6666
const argumentName = getArgumentName(node.arguments);
6767

6868
if (argumentName) {
69-
const fix = isValueUsed(node) ? undefined : fixer => fixer.replaceText(node, `${argumentName}.remove()`);
69+
const fix = isValueNotUsable(node) ? fixer => fixer.replaceText(node, `${argumentName}.remove()`) : undefined;
7070

7171
context.report({
7272
node,

‎rules/utils/is-value-not-usable.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
'use strict';
2+
3+
module.exports = ({parent}) => !parent || parent.type === 'ExpressionStatement';

‎rules/utils/is-value-used.js‎

Lines changed: 0 additions & 25 deletions
This file was deleted.

‎test/prefer-modern-dom-apis.js‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ ruleTester.run('prefer-modern-dom-apis', rule, {
102102
'Prefer `beta.before(alfa)` over `parentNode.insertBefore(alfa, beta)`.'
103103
}
104104
],
105-
output: 'beta.before(alfa).insertBefore(charlie, delta);'
105+
output: 'parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta);'
106106
},
107107
{
108108
code: 'const foo = parentNode.insertBefore(alfa, beta);',
@@ -124,6 +124,28 @@ ruleTester.run('prefer-modern-dom-apis', rule, {
124124
],
125125
output: 'foo = parentNode.insertBefore(alfa, beta);'
126126
},
127+
{
128+
code: 'new Dom(parentNode.insertBefore(alfa, beta))',
129+
errors: [
130+
{
131+
message:
132+
'Prefer `beta.before(alfa)` over `parentNode.insertBefore(alfa, beta)`.'
133+
}
134+
],
135+
output: 'new Dom(parentNode.insertBefore(alfa, beta))'
136+
},
137+
{
138+
/* eslint-disable no-template-curly-in-string */
139+
code: '`${parentNode.insertBefore(alfa, beta)}`',
140+
errors: [
141+
{
142+
message:
143+
'Prefer `beta.before(alfa)` over `parentNode.insertBefore(alfa, beta)`.'
144+
}
145+
],
146+
output: '`${parentNode.insertBefore(alfa, beta)}`'
147+
/* eslint-enable no-template-curly-in-string */
148+
},
127149
// Tests for .insertAdjacentText()
128150
{
129151
code: 'referenceNode.insertAdjacentText("beforebegin", "text");',
@@ -275,6 +297,56 @@ ruleTester.run('prefer-modern-dom-apis', rule, {
275297
}
276298
],
277299
output: 'foo = referenceNode.insertAdjacentElement("beforebegin", newNode);'
300+
},
301+
{
302+
code: 'const foo = [referenceNode.insertAdjacentElement("beforebegin", newNode)]',
303+
errors: [
304+
{
305+
message:
306+
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
307+
}
308+
],
309+
output: 'const foo = [referenceNode.insertAdjacentElement("beforebegin", newNode)]'
310+
},
311+
{
312+
code: 'foo(bar = referenceNode.insertAdjacentElement("beforebegin", newNode))',
313+
errors: [
314+
{
315+
message:
316+
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
317+
}
318+
],
319+
output: 'foo(bar = referenceNode.insertAdjacentElement("beforebegin", newNode))'
320+
},
321+
{
322+
code: 'const foo = () => { return referenceNode.insertAdjacentElement("beforebegin", newNode); }',
323+
errors: [
324+
{
325+
message:
326+
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
327+
}
328+
],
329+
output: 'const foo = () => { return referenceNode.insertAdjacentElement("beforebegin", newNode); }'
330+
},
331+
{
332+
code: 'if (referenceNode.insertAdjacentElement("beforebegin", newNode)) {}',
333+
errors: [
334+
{
335+
message:
336+
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
337+
}
338+
],
339+
output: 'if (referenceNode.insertAdjacentElement("beforebegin", newNode)) {}'
340+
},
341+
{
342+
code: 'const foo = { bar: referenceNode.insertAdjacentElement("beforebegin", newNode) }',
343+
errors: [
344+
{
345+
message:
346+
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
347+
}
348+
],
349+
output: 'const foo = { bar: referenceNode.insertAdjacentElement("beforebegin", newNode) }'
278350
}
279351
]
280352
});

0 commit comments

Comments
 (0)