Skip to content

Commit a5203e4

Browse files
bmishbmish
authored
Merge pull request #595 from jbandura/rule-no-component-lifecycle-hooks
Add new rule `no-component-lifecycle-hooks` (included in `octane` config)
2 parents 9dec68f + 88ae1d5 commit a5203e4

File tree

9 files changed

+378
-0
lines changed

9 files changed

+378
-0
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ Each rule has emojis denoting what configuration it belongs to and/or a :wrench:
124124
| :car: | [no-actions-hash](./docs/rules/no-actions-hash.md) | disallow the actions hash in components, controllers, and routes |
125125
| :car: | [no-classic-classes](./docs/rules/no-classic-classes.md) | disallow "classic" classes in favor of native JS classes |
126126
| :car: | [no-classic-components](./docs/rules/no-classic-components.md) | enforce using Glimmer components |
127+
| :car: | [no-component-lifecycle-hooks](./docs/rules/no-component-lifecycle-hooks.md) | disallow usage of "classic" ember component lifecycle hooks. Render modifiers or custom functional modifiers should be used instead. |
127128
| :car: | [no-computed-properties-in-native-classes](./docs/rules/no-computed-properties-in-native-classes.md) | disallow using computed properties in native classes |
128129
| :car: | [require-tagless-components](./docs/rules/require-tagless-components.md) | disallow using the wrapper element of a component |
129130

‎docs/rules/no-component-lifecycle-hooks.md‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# no-component-lifecycle-hooks
2+
3+
Disallow usage of "classic" ember component lifecycle hooks.
4+
5+
## Rule Details
6+
7+
As most component lifecycle hooks are gone in glimmer components, this rule aims to:
8+
9+
- remind the developer that those hooks no longer exist in glimmer components
10+
- encourage migrating away from those hooks in classic ember components
11+
12+
Effectively, this rule disallows following lifecycle hooks in components:
13+
14+
- `didDestroyElement`
15+
- `didInsertElement`
16+
- `didReceiveAttrs`
17+
- `didRender`
18+
- `didUpdate`
19+
- `didUpdateAttrs`
20+
- `willClearRender`
21+
- `willDestroyElement`
22+
- `willRender`
23+
24+
Custom functional modifiers or @ember/render-modifiers should be used instead.
25+
26+
## Examples
27+
28+
Examples of **incorrect** code for this rule:
29+
30+
```js
31+
export default class MyComponent extends Component {
32+
didDestroyElement() {}
33+
didInsertElement() {}
34+
didReceiveAttrs() {}
35+
didRender() {}
36+
didUpdate() {}
37+
didUpdateAttrs() {}
38+
willClearRender() {}
39+
willDestroyElement() {}
40+
willRender() {}
41+
}
42+
```
43+
44+
```js
45+
export default Component.extend({
46+
didDestroyElement() {},
47+
didInsertElement() {},
48+
didReceiveAttrs() {},
49+
didRender() {},
50+
didUpdate() {},
51+
didUpdateAttrs() {},
52+
willClearRender() {},
53+
willDestroyElement() {},
54+
willRender() {}
55+
});
56+
```
57+
58+
Examples of **correct** code for this rule:
59+
60+
```js
61+
export default class MyComponent extends Component {
62+
init() {}
63+
willDestroy() {}
64+
}
65+
```
66+
67+
```js
68+
export default Component.extend({
69+
init() {},
70+
willDestroy() {}
71+
});
72+
```
73+
74+
## Further Reading
75+
76+
- [`@ember/render-modifiers`](https://github.com/emberjs/ember-render-modifiers)
77+
- [Blog post about modifiers](https://blog.emberjs.com/2019/03/06/coming-soon-in-ember-octane-part-4.html)

‎lib/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ module.exports = {
1919
'no-capital-letters-in-routes': require('./rules/no-capital-letters-in-routes'),
2020
'no-classic-classes': require('./rules/no-classic-classes'),
2121
'no-classic-components': require('./rules/no-classic-components'),
22+
'no-component-lifecycle-hooks': require('./rules/no-component-lifecycle-hooks'),
2223
'no-computed-properties-in-native-classes': require('./rules/no-computed-properties-in-native-classes'),
2324
'no-controllers': require('./rules/no-controllers'),
2425
'no-deeply-nested-dependent-keys-with-each': require('./rules/no-deeply-nested-dependent-keys-with-each'),

‎lib/octane-rules.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ module.exports = {
1010
"ember/no-actions-hash": "error",
1111
"ember/no-classic-classes": "error",
1212
"ember/no-classic-components": "error",
13+
"ember/no-component-lifecycle-hooks": "error",
1314
"ember/no-computed-properties-in-native-classes": "error",
1415
"ember/no-get-with-default": "error",
1516
"ember/no-get": "error",

‎lib/recommended-rules.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ module.exports = {
2222
"ember/no-capital-letters-in-routes": "error",
2323
"ember/no-classic-classes": "off",
2424
"ember/no-classic-components": "off",
25+
"ember/no-component-lifecycle-hooks": "off",
2526
"ember/no-computed-properties-in-native-classes": "off",
2627
"ember/no-controllers": "off",
2728
"ember/no-deeply-nested-dependent-keys-with-each": "error",

‎lib/rules/no-component-lifecycle-hooks.js‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
'use strict';
2+
3+
const emberUtils = require('../utils/ember');
4+
5+
const { isComponentLifecycleHook, isEmberComponent, isGlimmerComponent } = emberUtils;
6+
7+
const ERROR_MESSAGE_NO_COMPONENT_LIFECYCLE_HOOKS =
8+
'Do not use classic ember components lifecycle hooks. Prefer using "@ember/render-modifiers" or custom functional modifiers.';
9+
10+
const report = (context, node) => {
11+
context.report(node, ERROR_MESSAGE_NO_COMPONENT_LIFECYCLE_HOOKS);
12+
};
13+
14+
module.exports = {
15+
ERROR_MESSAGE_NO_COMPONENT_LIFECYCLE_HOOKS,
16+
meta: {
17+
type: 'suggestion',
18+
docs: {
19+
description:
20+
'disallow usage of "classic" ember component lifecycle hooks. Render modifiers or custom functional modifiers should be used instead.',
21+
category: 'Ember Octane',
22+
recommended: false,
23+
octane: true,
24+
url:
25+
'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-component-lifecycle-hooks.md',
26+
},
27+
fixable: null, // or "code" or "whitespace"
28+
schema: [],
29+
},
30+
31+
create(context) {
32+
let isInsideEmberComponent = false;
33+
let currentEmberComponent = null;
34+
35+
const isAClassicLifecycleHook = node => {
36+
return isComponentLifecycleHook(node) && isInsideEmberComponent;
37+
};
38+
39+
return {
40+
ClassDeclaration(node) {
41+
if (isEmberComponent(context, node) || isGlimmerComponent(context, node)) {
42+
currentEmberComponent = node;
43+
isInsideEmberComponent = true;
44+
}
45+
},
46+
47+
CallExpression(node) {
48+
if (isEmberComponent(context, node)) {
49+
currentEmberComponent = node;
50+
isInsideEmberComponent = true;
51+
}
52+
},
53+
54+
'ClassDeclaration:exit'(node) {
55+
if (currentEmberComponent === node) {
56+
currentEmberComponent = null;
57+
isInsideEmberComponent = false;
58+
}
59+
},
60+
61+
'CallExpression:exit'(node) {
62+
if (currentEmberComponent === node) {
63+
currentEmberComponent = null;
64+
isInsideEmberComponent = false;
65+
}
66+
},
67+
68+
MethodDefinition(node) {
69+
if (isAClassicLifecycleHook(node)) {
70+
report(context, node);
71+
}
72+
},
73+
74+
Property(node) {
75+
if (isAClassicLifecycleHook(node)) {
76+
report(context, node.key);
77+
}
78+
},
79+
};
80+
},
81+
};

‎lib/utils/ember.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ module.exports = {
1313

1414
isEmberCoreModule,
1515
isEmberComponent,
16+
isGlimmerComponent,
1617
isEmberController,
1718
isEmberMixin,
1819
isEmberRoute,
@@ -63,6 +64,7 @@ module.exports = {
6364
// Private
6465
const CORE_MODULE_IMPORT_PATHS = {
6566
Component: '@ember/component',
67+
GlimmerComponent: '@glimmer/component',
6668
Controller: '@ember/controller',
6769
Mixin: '@ember/object/mixin',
6870
Route: '@ember/routing/route',
@@ -199,6 +201,10 @@ function isEmberComponent(context, node) {
199201
return isEmberCoreModule(context, node, 'Component');
200202
}
201203

204+
function isGlimmerComponent(context, node) {
205+
return isEmberCoreModule(context, node, 'GlimmerComponent');
206+
}
207+
202208
function isEmberController(context, node) {
203209
return isEmberCoreModule(context, node, 'Controller');
204210
}

‎tests/lib/rules/no-component-lifecycle-hooks.js‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
'use strict';
2+
3+
const rule = require('../../../lib/rules/no-component-lifecycle-hooks');
4+
const RuleTester = require('eslint').RuleTester;
5+
6+
const { ERROR_MESSAGE_NO_COMPONENT_LIFECYCLE_HOOKS: ERROR_MESSAGE } = rule;
7+
8+
const ruleTester = new RuleTester({
9+
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
10+
});
11+
12+
ruleTester.run('no-component-lifecycle-hooks', rule, {
13+
valid: [
14+
`
15+
import Component from '@glimmer/component';
16+
export default class MyComponent extends Component {
17+
willDestroy() {}
18+
}
19+
`,
20+
`
21+
import Component from '@glimmer/component';
22+
export default class MyClass {
23+
didUpdate() {}
24+
}
25+
`,
26+
`
27+
export default {
28+
didUpdate() {},
29+
}
30+
`,
31+
`
32+
import Component from '@ember/component';
33+
export default Component.extend({
34+
willDestroy() {},
35+
});
36+
`,
37+
`
38+
export default EmberObject.extend({
39+
didUpdate() {},
40+
});
41+
`,
42+
`
43+
import Component from '@ember/component';
44+
export const Component1 = Component.extend({
45+
willDestroy() {},
46+
});
47+
export const Component2 = Component.extend({
48+
willDestroy() {},
49+
});
50+
`,
51+
`
52+
import Component from '@ember/component';
53+
export default class MyComponent extends Component {}
54+
55+
const someRandomClassOrObject = { didDestroyElement() { } };
56+
`,
57+
`
58+
import Component from '@ember/component';
59+
export default Component.extend({});
60+
61+
const someRandomClassOrObject = { didDestroyElement() { } };
62+
`,
63+
],
64+
65+
invalid: [
66+
{
67+
code: `
68+
import Component from '@glimmer/component';
69+
export default class MyComponent extends Component {
70+
didDestroyElement() {}
71+
}
72+
`,
73+
output: null,
74+
errors: [
75+
{
76+
message: ERROR_MESSAGE,
77+
type: 'MethodDefinition',
78+
},
79+
],
80+
},
81+
{
82+
code: `
83+
import Component from '@ember/component';
84+
export default class MyComponent extends Component {
85+
didDestroyElement() {}
86+
}
87+
`,
88+
output: null,
89+
errors: [
90+
{
91+
message: ERROR_MESSAGE,
92+
type: 'MethodDefinition',
93+
},
94+
],
95+
},
96+
{
97+
code: `
98+
import Component from '@ember/component';
99+
export default Component.extend({
100+
didDestroyElement() {}
101+
})
102+
`,
103+
output: null,
104+
errors: [
105+
{
106+
message: ERROR_MESSAGE,
107+
type: 'Identifier',
108+
},
109+
],
110+
},
111+
{
112+
code: `
113+
import Component from '@ember/component';
114+
115+
export const Component2 = Component.extend({
116+
didDestroyElement() {},
117+
});
118+
119+
export const Component1 = Component.extend({
120+
willDestroy() {},
121+
});
122+
`,
123+
output: null,
124+
errors: [
125+
{
126+
message: ERROR_MESSAGE,
127+
type: 'Identifier',
128+
},
129+
],
130+
},
131+
{
132+
code: `
133+
import Component from "@ember/component";
134+
135+
export const Component1 = Component.extend({
136+
test: computed('', function () {}),
137+
didDestroyElement() {},
138+
});
139+
`,
140+
output: null,
141+
errors: [
142+
{
143+
message: ERROR_MESSAGE,
144+
type: 'Identifier',
145+
},
146+
],
147+
},
148+
{
149+
code: `
150+
import Component from "@glimmer/component";
151+
152+
export default class MyComponent extends Component {
153+
myMethod() {
154+
class FooBarClass {}
155+
}
156+
didDestroyElement() {}
157+
}
158+
`,
159+
output: null,
160+
errors: [
161+
{
162+
message: ERROR_MESSAGE,
163+
type: 'MethodDefinition',
164+
},
165+
],
166+
},
167+
],
168+
});

0 commit comments

Comments
 (0)