Skip to content

Commit a41d6e6

Browse files
NullVoxPopuliNullVoxPopuli
authored
Merge pull request #2552 from ember-cli/copilot/add-lint-rule-ember-modifiers
Add `no-modifier-argument-destructuring` rule
2 parents c9be61b + 9ab0e0b commit a41d6e6

4 files changed

Lines changed: 299 additions & 9 deletions

File tree

‎README.md‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -316,15 +316,16 @@ rules in templates can be disabled with eslint directives with mustache or html
316316

317317
### Ember Object
318318

319-
| Name                                 | Description | 💼 | 🔧 | 💡 |
320-
| :----------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------- | :- | :- | :- |
321-
| [avoid-leaking-state-in-ember-objects](docs/rules/avoid-leaking-state-in-ember-objects.md) | disallow state leakage | ✅ | | |
322-
| [no-get](docs/rules/no-get.md) | require using ES5 getters instead of Ember's `get` / `getProperties` functions | ✅ | 🔧 | |
323-
| [no-get-with-default](docs/rules/no-get-with-default.md) | disallow usage of the Ember's `getWithDefault` function | ✅ | 🔧 | |
324-
| [no-proxies](docs/rules/no-proxies.md) | disallow using array or object proxies | | | |
325-
| [no-try-invoke](docs/rules/no-try-invoke.md) | disallow usage of the Ember's `tryInvoke` util | ✅ | | |
326-
| [require-super-in-lifecycle-hooks](docs/rules/require-super-in-lifecycle-hooks.md) | require super to be called in lifecycle hooks | ✅ | 🔧 | |
327-
| [use-ember-get-and-set](docs/rules/use-ember-get-and-set.md) | enforce usage of `Ember.get` and `Ember.set` | | 🔧 | |
319+
| Name                                 | Description | 💼 | 🔧 | 💡 |
320+
| :----------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------- | :- | :- | :- |
321+
| [avoid-leaking-state-in-ember-objects](docs/rules/avoid-leaking-state-in-ember-objects.md) | disallow state leakage | ✅ | | |
322+
| [no-get](docs/rules/no-get.md) | require using ES5 getters instead of Ember's `get` / `getProperties` functions | ✅ | 🔧 | |
323+
| [no-get-with-default](docs/rules/no-get-with-default.md) | disallow usage of the Ember's `getWithDefault` function | ✅ | 🔧 | |
324+
| [no-modifier-argument-destructuring](docs/rules/no-modifier-argument-destructuring.md) | disallow destructuring of modifier arguments to avoid consuming tracked data too early | | | |
325+
| [no-proxies](docs/rules/no-proxies.md) | disallow using array or object proxies | | | |
326+
| [no-try-invoke](docs/rules/no-try-invoke.md) | disallow usage of the Ember's `tryInvoke` util | ✅ | | |
327+
| [require-super-in-lifecycle-hooks](docs/rules/require-super-in-lifecycle-hooks.md) | require super to be called in lifecycle hooks | ✅ | 🔧 | |
328+
| [use-ember-get-and-set](docs/rules/use-ember-get-and-set.md) | enforce usage of `Ember.get` and `Ember.set` | | 🔧 | |
328329

329330
### Ember Octane
330331

‎docs/rules/no-modifier-argument-destructuring.md‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# ember/no-modifier-argument-destructuring
2+
3+
<!-- end auto-generated rule header -->
4+
5+
Disallow destructuring of `modifier` arguments to avoid consuming tracked data too early.
6+
7+
## Rule Details
8+
9+
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).
10+
11+
Instead, access the arguments by index (for positional) or property (for named) inside the callback body where the data is actually needed.
12+
13+
## Examples
14+
15+
Examples of **incorrect** code for this rule:
16+
17+
```js
18+
import { modifier } from 'ember-modifier';
19+
20+
// Destructuring positional args
21+
modifier((element, [text]) => {
22+
element.addEventListener('hover', () => console.log(text));
23+
});
24+
```
25+
26+
```js
27+
import { modifier } from 'ember-modifier';
28+
29+
// Destructuring named args
30+
modifier((element, positional, { title }) => {
31+
element.addEventListener('hover', () => console.log(title));
32+
});
33+
```
34+
35+
Examples of **correct** code for this rule:
36+
37+
```js
38+
import { modifier } from 'ember-modifier';
39+
40+
// Access positional args by index
41+
modifier((element, positional) => {
42+
element.addEventListener('hover', () => console.log(positional[0]));
43+
});
44+
```
45+
46+
```js
47+
import { modifier } from 'ember-modifier';
48+
49+
// Access named args by property
50+
modifier((element, positional, named) => {
51+
element.addEventListener('hover', () => console.log(named.title));
52+
});
53+
```
54+
55+
## References
56+
57+
- [ember-modifier](https://github.com/ember-modifier/ember-modifier)
58+
- [Ember Autotracking](https://guides.emberjs.com/release/in-depth-topics/autotracking-in-depth/)

‎lib/rules/no-modifier-argument-destructuring.js‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
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;
30+
31+
return {
32+
ImportDeclaration(node) {
33+
if (node.source.value === 'ember-modifier') {
34+
modifierImportedName ??= getImportIdentifier(node, 'ember-modifier', 'modifier');
35+
}
36+
},
37+
38+
CallExpression(node) {
39+
if (!modifierImportedName) {
40+
return;
41+
}
42+
43+
if (node.callee.type !== 'Identifier' || node.callee.name !== modifierImportedName) {
44+
return;
45+
}
46+
47+
const callback = node.arguments[0];
48+
if (!callback) {
49+
return;
50+
}
51+
52+
// Support arrow functions and function expressions
53+
if (callback.type !== 'ArrowFunctionExpression' && callback.type !== 'FunctionExpression') {
54+
return;
55+
}
56+
57+
// Check 2nd parameter (positional args) - index 1
58+
const positionalParam = callback.params[1];
59+
if (positionalParam && positionalParam.type === 'ArrayPattern') {
60+
context.report({
61+
node: positionalParam,
62+
messageId: 'main',
63+
});
64+
}
65+
66+
// Check 3rd parameter (named args) - index 2
67+
const namedParam = callback.params[2];
68+
if (namedParam && namedParam.type === 'ObjectPattern') {
69+
context.report({
70+
node: namedParam,
71+
messageId: 'main',
72+
});
73+
}
74+
},
75+
};
76+
},
77+
};

‎tests/lib/rules/no-modifier-argument-destructuring.js‎

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)