Skip to content

Commit 875c471

Browse files
fiskerfisker
authored
Refactor prefer-modern-dom-apis (#565)
1 parent 9bc218e commit 875c471

File tree

2 files changed

+73
-81
lines changed

2 files changed

+73
-81
lines changed

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

Lines changed: 70 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -3,77 +3,71 @@ const getDocumentationUrl = require('./utils/get-documentation-url');
33
const isValueNotUsable = require('./utils/is-value-not-usable');
44
const methodSelector = require('./utils/method-selector');
55

6-
const selector = methodSelector({
7-
length: 2
8-
});
9-
10-
const getArgumentNameForReplaceChildOrInsertBefore = nodeArguments => {
11-
if (nodeArguments.type === 'Identifier') {
12-
return nodeArguments.name;
13-
}
6+
const messages = {
7+
replaceChildOrInsertBefore:
8+
'Prefer `{{oldChildNode}}.{{preferredMethod}}({{newChildNode}})` over `{{parentNode}}.{{method}}({{newChildNode}}, {{oldChildNode}})`.',
9+
insertAdjacentTextOrInsertAdjacentElement:
10+
'Prefer `{{reference}}.{{preferredMethod}}({{content}})` over `{{reference}}.{{method}}({{position}}, {{content}})`.'
1411
};
1512

16-
const forbiddenIdentifierNames = new Map([
13+
const replaceChildOrInsertBeforeSelector = [
14+
methodSelector({
15+
names: ['replaceChild', 'insertBefore'],
16+
length: 2
17+
}),
18+
// We only allow Identifier for now
19+
'[arguments.0.type="Identifier"]',
20+
'[arguments.0.name!="undefined"]',
21+
'[arguments.1.type="Identifier"]',
22+
'[arguments.1.name!="undefined"]',
23+
// 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
24+
'[callee.object.type="Identifier"]'
25+
].join('');
26+
27+
const forbiddenMethods = new Map([
1728
['replaceChild', 'replaceWith'],
1829
['insertBefore', 'before']
1930
]);
2031

2132
const checkForReplaceChildOrInsertBefore = (context, node) => {
22-
const identifierName = node.callee.property.name;
23-
24-
// Return early when specified methods don't exist in forbiddenIdentifierNames
25-
if (!forbiddenIdentifierNames.has(identifierName)) {
26-
return;
27-
}
28-
29-
const nodeArguments = node.arguments;
30-
const newChildNodeArgument = getArgumentNameForReplaceChildOrInsertBefore(
31-
nodeArguments[0]
32-
);
33-
const oldChildNodeArgument = getArgumentNameForReplaceChildOrInsertBefore(
34-
nodeArguments[1]
35-
);
36-
37-
// Return early in case that one of the provided arguments is not a node
38-
if (!newChildNodeArgument || !oldChildNodeArgument) {
39-
return;
40-
}
41-
33+
const method = node.callee.property.name;
4234
const parentNode = node.callee.object.name;
43-
// 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
44-
if (!parentNode) {
45-
return;
46-
}
47-
48-
const preferredSelector = forbiddenIdentifierNames.get(identifierName);
35+
const [newChildNode, oldChildNode] = node.arguments.map(({name}) => name);
36+
const preferredMethod = forbiddenMethods.get(method);
4937

5038
const fix = isValueNotUsable(node) ?
51-
// Report error when the method is part of a variable assignment
52-
// but don't offer to autofix `.replaceWith()` and `.before()`
53-
// which don't have a return value.
5439
fixer => fixer.replaceText(
5540
node,
56-
`${oldChildNodeArgument}.${preferredSelector}(${newChildNodeArgument})`
41+
`${oldChildNode}.${preferredMethod}(${newChildNode})`
5742
) :
5843
undefined;
5944

6045
return context.report({
6146
node,
62-
message: `Prefer \`${oldChildNodeArgument}.${preferredSelector}(${newChildNodeArgument})\` over \`${parentNode}.${identifierName}(${newChildNodeArgument}, ${oldChildNodeArgument})\`.`,
47+
messageId: 'replaceChildOrInsertBefore',
48+
data: {
49+
parentNode,
50+
method,
51+
preferredMethod,
52+
newChildNode,
53+
oldChildNode
54+
},
6355
fix
6456
});
6557
};
6658

67-
// Handle both `Identifier` and `Literal` because the preferred selectors support nodes and DOMString.
68-
const getArgumentNameForInsertAdjacentMethods = nodeArguments => {
69-
if (nodeArguments.type === 'Identifier') {
70-
return nodeArguments.name;
71-
}
72-
73-
if (nodeArguments.type === 'Literal') {
74-
return nodeArguments.raw;
75-
}
76-
};
59+
const insertAdjacentTextOrInsertAdjacentElementSelector = [
60+
methodSelector({
61+
names: ['insertAdjacentText', 'insertAdjacentElement'],
62+
length: 2
63+
}),
64+
// Position argument should be `string`
65+
'[arguments.0.type="Literal"]',
66+
// TODO: remove this limits on second argument
67+
':matches([arguments.1.type="Literal"], [arguments.1.type="Identifier"])',
68+
// TODO: remove this limits on callee
69+
'[callee.object.type="Identifier"]'
70+
].join('');
7771

7872
const positionReplacers = new Map([
7973
['beforebegin', 'before'],
@@ -83,53 +77,47 @@ const positionReplacers = new Map([
8377
]);
8478

8579
const checkForInsertAdjacentTextOrInsertAdjacentElement = (context, node) => {
86-
const identifierName = node.callee.property.name;
87-
88-
// Return early when method name is not one of the targeted ones.
89-
if (
90-
identifierName !== 'insertAdjacentText' &&
91-
identifierName !== 'insertAdjacentElement'
92-
) {
93-
return;
94-
}
95-
96-
const nodeArguments = node.arguments;
97-
const positionArgument = getArgumentNameForInsertAdjacentMethods(nodeArguments[0]);
98-
const positionAsValue = nodeArguments[0].value;
80+
const method = node.callee.property.name;
81+
const [positionNode, contentNode] = node.arguments;
9982

83+
const position = positionNode.value;
10084
// Return early when specified position value of first argument is not a recognized value.
101-
if (!positionReplacers.has(positionAsValue)) {
85+
if (!positionReplacers.has(position)) {
10286
return;
10387
}
10488

105-
const referenceNode = node.callee.object.name;
106-
const preferredSelector = positionReplacers.get(positionAsValue);
107-
const insertedTextArgument = getArgumentNameForInsertAdjacentMethods(
108-
nodeArguments[1]
109-
);
89+
const preferredMethod = positionReplacers.get(position);
90+
const content = context.getSource(contentNode);
91+
const reference = context.getSource(node.callee.object);
11092

111-
const fix = identifierName === 'insertAdjacentElement' && !isValueNotUsable(node) ?
112-
// Report error when the method is part of a variable assignment
113-
// but don't offer to autofix `.insertAdjacentElement()`
114-
// which doesn't have a return value.
93+
const fix = method === 'insertAdjacentElement' && !isValueNotUsable(node) ?
11594
undefined :
116-
fixer =>
117-
fixer.replaceText(
118-
node,
119-
`${referenceNode}.${preferredSelector}(${insertedTextArgument})`
120-
);
95+
// TODO: make a better fix, don't touch reference
96+
fixer => fixer.replaceText(
97+
node,
98+
`${reference}.${preferredMethod}(${content})`
99+
);
121100

122101
return context.report({
123102
node,
124-
message: `Prefer \`${referenceNode}.${preferredSelector}(${insertedTextArgument})\` over \`${referenceNode}.${identifierName}(${positionArgument}, ${insertedTextArgument})\`.`,
103+
messageId: 'insertAdjacentTextOrInsertAdjacentElement',
104+
data: {
105+
reference,
106+
method,
107+
preferredMethod,
108+
position: context.getSource(positionNode),
109+
content
110+
},
125111
fix
126112
});
127113
};
128114

129115
const create = context => {
130116
return {
131-
[selector](node) {
117+
[replaceChildOrInsertBeforeSelector](node) {
132118
checkForReplaceChildOrInsertBefore(context, node);
119+
},
120+
[insertAdjacentTextOrInsertAdjacentElementSelector](node) {
133121
checkForInsertAdjacentTextOrInsertAdjacentElement(context, node);
134122
}
135123
};
@@ -142,6 +130,7 @@ module.exports = {
142130
docs: {
143131
url: getDocumentationUrl(__filename)
144132
},
145-
fixable: 'code'
133+
fixable: 'code',
134+
messages
146135
}
147136
};

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ ruleTester.run('prefer-modern-dom-apis', rule, {
2020
'referenceNode.append("text");',
2121
'referenceNode.after(newNode);',
2222
'referenceNode.after("text");',
23+
// Argument is `Identifier` but is `undefined`
24+
'oldChildNode.replaceWith(undefined, oldNode);',
25+
'oldChildNode.replaceWith(newNode, undefined);',
2326
// Not `CallExpression`
2427
'new parentNode.replaceChild(newNode, oldNode);',
2528
'new parentNode.insertBefore(newNode, referenceNode);',

0 commit comments

Comments
 (0)