-
-
Notifications
You must be signed in to change notification settings - Fork 204
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new rule
no-at-ember-render-modifiers
(#1902)
* Implementation -- needs tests * Rename lint * Add tests, run update commands * Add docs * Lint docs * Update docs/rules/no-at-ember-render-modifiers.md Co-authored-by: Bryan Mishkin <[email protected]> * Use more specific import path check * Change category to Deprecations * Add link to ember-template-lint * Run update script --------- Co-authored-by: Bryan Mishkin <[email protected]>
- Loading branch information
1 parent
8d70560
commit 03afb9b
Showing
4 changed files
with
158 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
# ember/no-at-ember-render-modifiers | ||
|
||
<!-- end auto-generated rule header --> | ||
|
||
What's wrong with `{{did-insert}}`, `{{did-update}}`, and `{{will-destroy}}`? | ||
|
||
These modifiers were meant for temporary migration purposes for quickly migrating `@ember/component` from before Octane to the `@glimmer/component` we have today. Since `@ember/component` implicitly had a wrapping `div` around each component and `@glimmer/component`s have "outer HTML" semantics, an automated migration could have ended up looking something like: | ||
|
||
```hbs | ||
<div | ||
{{did-insert this.doInsertBehavior}} | ||
{{did-update this.doUpdateBehavior @arg1 @arg2}} | ||
{{will-destroy this.doDestroyBehavior}} | ||
> | ||
... | ||
</div> | ||
``` | ||
|
||
It was intended that this would be a temporary step to help get folks off of `@ember/components` quickly in early 2020 when folks migrated to the Octane editon, but some folks continued using these modifiers. | ||
|
||
Additionally, this style of mananging data flow has some flaws: | ||
|
||
- an element is required | ||
- this can be mitigated by using helpers, but they have the same problems mentioned below | ||
- the behavior that is used with these modifiers can cause extra renders and infinite rendering loops | ||
- this is the nature of "effect"-driven development / data-flow, every time an effect runs, rendering must re-occur. | ||
- behavior that needs to be co-located is spread out, making maintenance and debugging harder | ||
- each part of the responsibility of a "behavior" or "feature" is spread out, making it harder to find and comprehend the full picture of that behavior or feature. | ||
|
||
## Examples | ||
|
||
This rule **forbids** the following: | ||
|
||
```js | ||
import didInsert from '@ember/render-modifiers/modifiers/did-insert'; | ||
``` | ||
|
||
```js | ||
import didUpdate from '@ember/render-modifiers/modifiers/did-update'; | ||
``` | ||
|
||
```js | ||
import willDestroy from '@ember/render-modifiers/modifiers/will-destroy'; | ||
``` | ||
|
||
For more examples, see [the docs on ember-template-lint](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-at-ember-render-modifiers.md). | ||
|
||
## References | ||
|
||
- [Editions](https://emberjs.com/editions/) | ||
- [Octane Upgrade Guide](https://guides.emberjs.com/release/upgrading/current-edition/) | ||
- [Component Documentation](https://guides.emberjs.com/release/components/) | ||
- [Avoiding Lifecycle in Component](https://nullvoxpopuli.com/avoiding-lifecycle) | ||
- [The `ember-template-lint` version of this rule](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-at-ember-render-modifiers.md) | ||
- [`ember-modifier`](https://github.com/ember-modifier/ember-modifier) | ||
- [`@ember/render-modifiers`](https://github.com/emberjs/ember-render-modifiers) (deprecated) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
'use strict'; | ||
|
||
const ERROR_MESSAGE = | ||
'Do not use @ember/render-modifiers. Instead, use derived data patterns, and/or co-locate destruction via @ember/destroyable'; | ||
//------------------------------------------------------------------------------ | ||
// Rule Definition | ||
//------------------------------------------------------------------------------ | ||
|
||
function importDeclarationIsPackageName(node, path) { | ||
return node.source.value === path || node.source.value.startsWith(`${path}/`); | ||
} | ||
|
||
/** @type {import('eslint').Rule.RuleModule} */ | ||
module.exports = { | ||
meta: { | ||
type: 'suggestion', | ||
docs: { | ||
description: 'disallow importing from @ember/render-modifiers', | ||
category: 'Deprecations', | ||
recommended: false, | ||
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-at-ember-render-modifiers.md', | ||
}, | ||
fixable: null, | ||
schema: [], | ||
}, | ||
|
||
ERROR_MESSAGE, | ||
|
||
create(context) { | ||
return { | ||
ImportDeclaration(node) { | ||
if (importDeclarationIsPackageName(node, '@ember/render-modifiers')) { | ||
context.report({ | ||
node, | ||
message: ERROR_MESSAGE, | ||
}); | ||
} | ||
}, | ||
}; | ||
}, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
//------------------------------------------------------------------------------ | ||
// Requirements | ||
//------------------------------------------------------------------------------ | ||
|
||
const rule = require('../../../lib/rules/no-at-ember-render-modifiers'); | ||
const RuleTester = require('eslint').RuleTester; | ||
|
||
const { ERROR_MESSAGE } = rule; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Tests | ||
//------------------------------------------------------------------------------ | ||
|
||
const ruleTester = new RuleTester({ | ||
parserOptions: { | ||
ecmaVersion: 2020, | ||
sourceType: 'module', | ||
}, | ||
}); | ||
ruleTester.run('no-at-ember-render-modifiers', rule, { | ||
valid: [ | ||
'import x from "x"', | ||
'', | ||
"import { x } from 'foo';", | ||
"import x from '@ember/foo';", | ||
"import x from '@ember/render-modifiers-foo';", | ||
], | ||
invalid: [ | ||
{ | ||
code: 'import "@ember/render-modifiers";', | ||
output: null, | ||
errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }], | ||
}, | ||
{ | ||
code: 'import x from "@ember/render-modifiers";', | ||
output: null, | ||
errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }], | ||
}, | ||
{ | ||
code: 'import { x } from "@ember/render-modifiers";', | ||
output: null, | ||
errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }], | ||
}, | ||
{ | ||
code: 'import didInsert from "@ember/render-modifiers/modifiers/did-insert";', | ||
output: null, | ||
errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }], | ||
}, | ||
{ | ||
code: 'import didUpdate from "@ember/render-modifiers/modifiers/did-update";', | ||
output: null, | ||
errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }], | ||
}, | ||
{ | ||
code: 'import willDestroy from "@ember/render-modifiers/modifiers/will-destroy";', | ||
output: null, | ||
errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }], | ||
}, | ||
], | ||
}); |