Skip to content

Commit 066f96b

Browse files
fiskerfisker
authored
prefer-classlist-toggle: Omit Element#classList.contains() call when fixing (#2732)
1 parent 5774925 commit 066f96b

File tree

5 files changed

+1159
-68
lines changed

5 files changed

+1159
-68
lines changed

‎docs/rules/prefer-classlist-toggle.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,26 @@ Prefer using [`Element#classList.toggle()`](https://developer.mozilla.org/en-US/
1111

1212
## Examples
1313

14+
```js
15+
// ❌
16+
if (element.classList.contains('className')) {
17+
element.classList.remove('className');
18+
} else {
19+
element.classList.add('className');
20+
}
21+
22+
// ❌
23+
element.classList.contains('className')
24+
? element.classList.remove('className')
25+
: element.classList.add('className');
26+
27+
// ❌
28+
element.classList[element.classList.contains('className') ? 'remove' : 'add']('className')
29+
30+
// ✅
31+
element.classList.toggle('className');
32+
```
33+
1434
```js
1535
// ❌
1636
if (condition) {

‎rules/prefer-classlist-toggle.js‎

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,40 @@ const getConditionText = (node, sourceCode, isNegative) => {
7878
return text;
7979
};
8080

81+
const isClassListMethodCall = (node, methods) =>
82+
isMethodCall(node, {
83+
methods,
84+
argumentsLength: 1,
85+
optionalCall: false,
86+
optionalMember: false,
87+
})
88+
&& isClassList(node.callee.object);
89+
90+
const isSameElementAndClassName = (callExpressionA, callExpressionB) =>
91+
isSameReference(callExpressionA.callee.object, callExpressionB.callee.object)
92+
&& isSameReference(callExpressionA.arguments[0], callExpressionB.arguments[0]);
93+
94+
const getClassListContainsCall = (conditionNode, isNegative, addOrRemoveCall) => {
95+
if (!isNegative) {
96+
if (!(conditionNode.type === 'UnaryExpression' && conditionNode.operator === '!' && conditionNode.prefix)) {
97+
return;
98+
}
99+
100+
return getClassListContainsCall(conditionNode.argument, !isNegative, addOrRemoveCall);
101+
}
102+
103+
if (conditionNode.type === 'ChainExpression') {
104+
conditionNode = conditionNode.expression;
105+
}
106+
107+
if (
108+
isClassListMethodCall(conditionNode, ['contains'])
109+
&& isSameElementAndClassName(conditionNode, addOrRemoveCall)
110+
) {
111+
return conditionNode;
112+
}
113+
};
114+
81115
/** @param {import('eslint').Rule.RuleContext} context */
82116
const create = context => {
83117
const {sourceCode} = context;
@@ -121,37 +155,30 @@ const create = context => {
121155

122156
// `element.classList.add('className');`
123157
// `element.classList.remove('className');`
124-
if (!clauses.every(node =>
125-
isMethodCall(node, {
126-
methods: ['add', 'remove'],
127-
argumentsLength: 1,
128-
optionalCall: false,
129-
optionalMember: false,
130-
})
131-
&& isClassList(node.callee.object),
132-
)) {
158+
if (!clauses.every(node => isClassListMethodCall(node, ['add', 'remove']))) {
133159
return;
134160
}
135161

136162
const [consequent, alternate] = clauses;
137163
if (
138164
(consequent.callee.property.name === alternate.callee.property.name)
139-
|| !isSameReference(consequent.callee.object, alternate.callee.object)
140-
|| !isSameReference(consequent.arguments[0], alternate.arguments[0])
165+
|| !isSameElementAndClassName(consequent, alternate)
141166
) {
142167
return;
143168
}
144169

145170
/** @param {import('eslint').Rule.RuleFixer} fixer */
146171
function * fix(fixer) {
147-
const isOptional = consequent.callee.object.optional || alternate.callee.object.optional;
148172
const elementText = getParenthesizedText(consequent.callee.object.object, sourceCode);
149173
const classNameText = getParenthesizedText(consequent.arguments[0], sourceCode);
150174
const isExpression = node.type === 'ConditionalExpression';
151175
const isNegative = consequent.callee.property.name === 'remove';
152-
const conditionText = getConditionText(node.test, sourceCode, isNegative);
176+
const conditionNode = node.test;
177+
const classListContainsCall = getClassListContainsCall(conditionNode, isNegative, consequent);
178+
const conditionText = classListContainsCall ? '' : getConditionText(conditionNode, sourceCode, isNegative);
179+
const isOptional = consequent.callee.object.optional || alternate.callee.object.optional || classListContainsCall?.callee.object.optional;
153180

154-
let text = `${elementText}${isOptional ? '?' : ''}.classList.toggle(${classNameText}, ${conditionText})`;
181+
let text = `${elementText}${isOptional ? '?' : ''}.classList.toggle(${classNameText}${conditionText ? `, ${conditionText}` : ''})`;
155182

156183
if (!isExpression) {
157184
text = `${text};`;
@@ -195,9 +222,14 @@ const create = context => {
195222
/** @param {import('eslint').Rule.RuleFixer} fixer */
196223
function * fix(fixer) {
197224
const isNegative = conditionalExpression.consequent.value === 'remove';
198-
const conditionText = getConditionText(conditionalExpression.test, sourceCode, isNegative);
225+
const conditionNode = conditionalExpression.test;
226+
const classListContainsCall = getClassListContainsCall(conditionNode, isNegative, callExpression);
227+
const conditionText = classListContainsCall ? '' : getConditionText(conditionNode, sourceCode, isNegative);
228+
229+
if (conditionText) {
230+
yield fixer.insertTextAfter(callExpression.arguments[0], `, ${conditionText}`);
231+
}
199232

200-
yield fixer.insertTextAfter(callExpression.arguments[0], `, ${conditionText}`);
201233
yield replaceMemberExpressionProperty(fixer, classListMethod, sourceCode, '.toggle');
202234
}
203235

‎test/prefer-classlist-toggle.js‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,84 @@ test.snapshot({
212212
213213
[].forEach(foo);
214214
`,
215+
// Checking class name existence
216+
outdent`
217+
if (element.classList.contains('className')) {
218+
element.classList.remove('className');
219+
} else {
220+
element.classList.add('className');
221+
}
222+
`,
223+
outdent`
224+
if (element?.classList.contains('className')) {
225+
element.classList.remove('className');
226+
} else {
227+
element.classList.add('className');
228+
}
229+
`,
230+
outdent`
231+
if (element.classList.contains?.('className')) {
232+
element.classList.remove('className');
233+
} else {
234+
element.classList.add('className');
235+
}
236+
`,
237+
outdent`
238+
if (element.classList?.contains('className')) {
239+
element.classList.remove('className');
240+
} else {
241+
element.classList.add('className');
242+
}
243+
`,
244+
outdent`
245+
if (element.classList.notContains('className')) {
246+
element.classList.remove('className');
247+
} else {
248+
element.classList.add('className');
249+
}
250+
`,
251+
outdent`
252+
if (element.classList.contains('not-same-class-name')) {
253+
element.classList.remove('className');
254+
} else {
255+
element.classList.add('className');
256+
}
257+
`,
258+
outdent`
259+
if (element.notClassList.contains('className')) {
260+
element.classList.remove('className');
261+
} else {
262+
element.classList.add('className');
263+
}
264+
`,
265+
outdent`
266+
if (contains('className')) {
267+
element.classList.remove('className');
268+
} else {
269+
element.classList.add('className');
270+
}
271+
`,
272+
outdent`
273+
if (notSameElement.classList.contains('className')) {
274+
element.classList.remove('className');
275+
} else {
276+
element.classList.add('className');
277+
}
278+
`,
279+
outdent`
280+
if (element.classList.contains('className')) {
281+
element.classList.add('className');
282+
} else {
283+
element.classList.remove('className');
284+
}
285+
`,
286+
outdent`
287+
if (!element.classList.contains('className')) {
288+
element.classList.add('className');
289+
} else {
290+
element.classList.remove('className');
291+
}
292+
`,
215293
],
216294
});
217295

@@ -242,6 +320,62 @@ test.snapshot({
242320
243321
condition ? (( element )).classList.add(className) : element.classList.remove(className);
244322
`,
323+
// Checking class name existence
324+
outdent`
325+
element.classList.contains('className')
326+
? element.classList.remove('className')
327+
: element.classList.add('className')
328+
`,
329+
outdent`
330+
element?.classList.contains('className')
331+
? element.classList.remove('className')
332+
: element.classList.add('className')
333+
`,
334+
outdent`
335+
element.classList.contains?.('className')
336+
? element.classList.remove('className')
337+
: element.classList.add('className')
338+
`,
339+
outdent`
340+
element.classList?.contains('className')
341+
? element.classList.remove('className')
342+
: element.classList.add('className')
343+
`,
344+
outdent`
345+
element.classList.notContains('className')
346+
? element.classList.remove('className')
347+
: element.classList.add('className')
348+
`,
349+
outdent`
350+
element.classList.contains('not-same-class-name')
351+
? element.classList.remove('className')
352+
: element.classList.add('className')
353+
`,
354+
outdent`
355+
element.notClassList.contains('className')
356+
? element.classList.remove('className')
357+
: element.classList.add('className')
358+
`,
359+
outdent`
360+
contains('className')
361+
? element.classList.remove('className')
362+
: element.classList.add('className')
363+
`,
364+
outdent`
365+
notSameElement.classList.contains('className')
366+
? element.classList.remove('className')
367+
: element.classList.add('className')
368+
`,
369+
outdent`
370+
element.classList.contains('className')
371+
? element.classList.add('className')
372+
: element.classList.remove('className')
373+
`,
374+
outdent`
375+
!element.classList.contains('className')
376+
? element.classList.add('className')
377+
: element.classList.remove('className')
378+
`,
245379
],
246380
});
247381

@@ -268,5 +402,19 @@ test.snapshot({
268402
'element.classList[index % 2 ? "remove" : "add"](className)',
269403
'element.classList[(index % 2) ? "remove" : "add"](className)',
270404
'element.classList[(0, condition) ? "add" : "remove"](className)',
405+
// Checking class name existence
406+
...[
407+
'element.classList.contains("className") ? "remove" : "add"',
408+
'element?.classList.contains("className") ? "remove" : "add"',
409+
'element.classList.contains?.("className") ? "remove" : "add"',
410+
'element.classList?.contains("className") ? "remove" : "add"',
411+
'element.classList.notContains("className") ? "remove" : "add"',
412+
'element.classList.contains("not-same-class-name") ? "remove" : "add"',
413+
'element.notClassList.contains("className") ? "remove" : "add"',
414+
'contains("className") ? "remove" : "add"',
415+
'notSameElement.classList.contains("className") ? "remove" : "add"',
416+
'element.classList.contains("className") ? "add": "remove"',
417+
'!element.classList.contains("className") ? "add": "remove"',
418+
].map(condition => `element.classList[${condition}]("className")`),
271419
],
272420
});

0 commit comments

Comments
 (0)