Skip to content

Commit 9f4ab6e

Browse files
Add no-modifier-argument-destructuring rule
New rule that disallows destructuring positional or named arguments in ember-modifier callbacks. Destructuring eagerly consumes tracked data, which can lead to backtracking re-render assertions. Instead, arguments should be accessed by index or property inside the callback body. Co-authored-by: NullVoxPopuli <[email protected]>
1 parent 7d80beb commit 9f4ab6e

3 files changed

Lines changed: 295 additions & 0 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# no-modifier-argument-destructuring
2+
3+
Disallow destructuring of `modifier` arguments to avoid consuming tracked data too early.
4+
5+
## Rule Details
6+
7+
When using `ember-modifier`, destructuring the positional or named arguments of the modifier callback eagerly consumes tracked data. This can lead to backtracking re-render assertions when the tracked data is not actually needed until later (e.g., inside an event listener).
8+
9+
Instead, access the arguments by index (for positional) or property (for named) inside the callback body where the data is actually needed.
10+
11+
## Examples
12+
13+
Examples of **incorrect** code for this rule:
14+
15+
```js
16+
import { modifier } from 'ember-modifier';
17+
18+
// Destructuring positional args
19+
modifier((element, [text]) => {
20+
element.addEventListener('hover', () => console.log(text));
21+
});
22+
```
23+
24+
```js
25+
import { modifier } from 'ember-modifier';
26+
27+
// Destructuring named args
28+
modifier((element, positional, { title }) => {
29+
element.addEventListener('hover', () => console.log(title));
30+
});
31+
```
32+
33+
Examples of **correct** code for this rule:
34+
35+
```js
36+
import { modifier } from 'ember-modifier';
37+
38+
// Access positional args by index
39+
modifier((element, positional) => {
40+
element.addEventListener('hover', () => console.log(positional[0]));
41+
});
42+
```
43+
44+
```js
45+
import { modifier } from 'ember-modifier';
46+
47+
// Access named args by property
48+
modifier((element, positional, named) => {
49+
element.addEventListener('hover', () => console.log(named.title));
50+
});
51+
```
52+
53+
## References
54+
55+
- [ember-modifier](https://github.com/ember-modifier/ember-modifier)
56+
- [Ember Autotracking](https://guides.emberjs.com/release/in-depth-topics/autotracking-in-depth/)
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
'use strict';
2+
3+
const { getImportIdentifier } = require('../utils/import');
4+
5+
const ERROR_MESSAGE =
6+
'Do not destructure the positional or named arguments of a modifier. Instead, access them by index or property to avoid eagerly consuming tracked data.';
7+
8+
/** @type {import('eslint').Rule.RuleModule} */
9+
module.exports = {
10+
meta: {
11+
type: 'suggestion',
12+
docs: {
13+
description:
14+
'disallow destructuring of modifier arguments to avoid consuming tracked data too early',
15+
category: 'Ember Object',
16+
recommended: false,
17+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-modifier-argument-destructuring.md',
18+
},
19+
fixable: null,
20+
schema: [],
21+
messages: {
22+
main: ERROR_MESSAGE,
23+
},
24+
},
25+
26+
ERROR_MESSAGE,
27+
28+
create(context) {
29+
let modifierImportedName = undefined;
30+
31+
return {
32+
ImportDeclaration(node) {
33+
if (node.source.value === 'ember-modifier') {
34+
modifierImportedName =
35+
getImportIdentifier(node, 'ember-modifier', 'modifier') ??
36+
modifierImportedName;
37+
}
38+
},
39+
40+
CallExpression(node) {
41+
if (!modifierImportedName) {
42+
return;
43+
}
44+
45+
if (
46+
node.callee.type !== 'Identifier' ||
47+
node.callee.name !== modifierImportedName
48+
) {
49+
return;
50+
}
51+
52+
const callback = node.arguments[0];
53+
if (!callback) {
54+
return;
55+
}
56+
57+
// Support arrow functions and function expressions
58+
if (
59+
callback.type !== 'ArrowFunctionExpression' &&
60+
callback.type !== 'FunctionExpression'
61+
) {
62+
return;
63+
}
64+
65+
// Check 2nd parameter (positional args) - index 1
66+
const positionalParam = callback.params[1];
67+
if (positionalParam && positionalParam.type === 'ArrayPattern') {
68+
context.report({
69+
node: positionalParam,
70+
messageId: 'main',
71+
});
72+
}
73+
74+
// Check 3rd parameter (named args) - index 2
75+
const namedParam = callback.params[2];
76+
if (namedParam && namedParam.type === 'ObjectPattern') {
77+
context.report({
78+
node: namedParam,
79+
messageId: 'main',
80+
});
81+
}
82+
},
83+
};
84+
},
85+
};
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
const rule = require('../../../lib/rules/no-modifier-argument-destructuring');
2+
const RuleTester = require('eslint').RuleTester;
3+
4+
const { ERROR_MESSAGE } = rule;
5+
6+
const ruleTester = new RuleTester({
7+
parserOptions: {
8+
ecmaVersion: 2022,
9+
sourceType: 'module',
10+
},
11+
});
12+
13+
ruleTester.run('no-modifier-argument-destructuring', rule, {
14+
valid: [
15+
// Not importing from ember-modifier
16+
`
17+
function modifier(fn) { return fn; }
18+
modifier((element, [text]) => {});
19+
`,
20+
21+
// No destructuring of positional args
22+
`
23+
import { modifier } from 'ember-modifier';
24+
modifier((element, positional) => {
25+
element.addEventListener('hover', () => console.log(positional[0]));
26+
});
27+
`,
28+
29+
// No destructuring of named args
30+
`
31+
import { modifier } from 'ember-modifier';
32+
modifier((element, positional, named) => {
33+
element.addEventListener('hover', () => console.log(named.text));
34+
});
35+
`,
36+
37+
// Only element parameter
38+
`
39+
import { modifier } from 'ember-modifier';
40+
modifier((element) => {
41+
element.addEventListener('click', () => {});
42+
});
43+
`,
44+
45+
// No arguments at all
46+
`
47+
import { modifier } from 'ember-modifier';
48+
modifier(() => {});
49+
`,
50+
51+
// Using a non-modifier function from ember-modifier
52+
`
53+
import { something } from 'ember-modifier';
54+
something((element, [text]) => {});
55+
`,
56+
57+
// Modifier called with non-function argument
58+
`
59+
import { modifier } from 'ember-modifier';
60+
modifier(myCallback);
61+
`,
62+
63+
// Destructuring element (first param) is fine as it's not tracked
64+
`
65+
import { modifier } from 'ember-modifier';
66+
modifier(({ tagName }) => {});
67+
`,
68+
69+
// Function expression (not arrow) without destructuring
70+
`
71+
import { modifier } from 'ember-modifier';
72+
modifier(function(element, positional) {
73+
console.log(positional[0]);
74+
});
75+
`,
76+
],
77+
78+
invalid: [
79+
// Destructuring positional args (arrow function)
80+
{
81+
code: `
82+
import { modifier } from 'ember-modifier';
83+
modifier((element, [text]) => {
84+
element.addEventListener('hover', () => console.log(text));
85+
});
86+
`,
87+
output: null,
88+
errors: [{ message: ERROR_MESSAGE, type: 'ArrayPattern' }],
89+
},
90+
91+
// Destructuring positional args (function expression)
92+
{
93+
code: `
94+
import { modifier } from 'ember-modifier';
95+
modifier(function(element, [text]) {
96+
element.addEventListener('hover', () => console.log(text));
97+
});
98+
`,
99+
output: null,
100+
errors: [{ message: ERROR_MESSAGE, type: 'ArrayPattern' }],
101+
},
102+
103+
// Destructuring named args
104+
{
105+
code: `
106+
import { modifier } from 'ember-modifier';
107+
modifier((element, positional, { text }) => {
108+
element.addEventListener('hover', () => console.log(text));
109+
});
110+
`,
111+
output: null,
112+
errors: [{ message: ERROR_MESSAGE, type: 'ObjectPattern' }],
113+
},
114+
115+
// Both positional and named args destructured
116+
{
117+
code: `
118+
import { modifier } from 'ember-modifier';
119+
modifier((element, [text], { title }) => {
120+
element.addEventListener('hover', () => console.log(text, title));
121+
});
122+
`,
123+
output: null,
124+
errors: [
125+
{ message: ERROR_MESSAGE, type: 'ArrayPattern' },
126+
{ message: ERROR_MESSAGE, type: 'ObjectPattern' },
127+
],
128+
},
129+
130+
// Renamed import
131+
{
132+
code: `
133+
import { modifier as createModifier } from 'ember-modifier';
134+
createModifier((element, [text]) => {
135+
element.addEventListener('hover', () => console.log(text));
136+
});
137+
`,
138+
output: null,
139+
errors: [{ message: ERROR_MESSAGE, type: 'ArrayPattern' }],
140+
},
141+
142+
// Multiple positional args destructured
143+
{
144+
code: `
145+
import { modifier } from 'ember-modifier';
146+
modifier((element, [text, count]) => {
147+
element.addEventListener('hover', () => console.log(text, count));
148+
});
149+
`,
150+
output: null,
151+
errors: [{ message: ERROR_MESSAGE, type: 'ArrayPattern' }],
152+
},
153+
],
154+
});

0 commit comments

Comments
 (0)