Skip to content

Commit

Permalink
Add new rule no-at-ember-render-modifiers (#1902)
Browse files Browse the repository at this point in the history
* 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 <698306+bmish@users.noreply.github.com>

* Use more specific import path check

* Change category to Deprecations

* Add link to ember-template-lint

* Run update script

---------

Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>
  • Loading branch information
NullVoxPopuli and bmish committed Jul 7, 2023
1 parent 8d70560 commit 03afb9b
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ module.exports = {
| [closure-actions](docs/rules/closure-actions.md) | enforce usage of closure actions || | |
| [new-module-imports](docs/rules/new-module-imports.md) | enforce using "New Module Imports" from Ember RFC #176 || | |
| [no-array-prototype-extensions](docs/rules/no-array-prototype-extensions.md) | disallow usage of Ember's `Array` prototype extensions | | 🔧 | |
| [no-at-ember-render-modifiers](docs/rules/no-at-ember-render-modifiers.md) | disallow importing from @ember/render-modifiers | | | |
| [no-deprecated-router-transition-methods](docs/rules/no-deprecated-router-transition-methods.md) | enforce usage of router service transition methods | | 🔧 | |
| [no-function-prototype-extensions](docs/rules/no-function-prototype-extensions.md) | disallow usage of Ember's `function` prototype extensions || | |
| [no-implicit-injections](docs/rules/no-implicit-injections.md) | enforce usage of implicit service injections | | 🔧 | |
Expand Down
56 changes: 56 additions & 0 deletions docs/rules/no-at-ember-render-modifiers.md
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)
41 changes: 41 additions & 0 deletions lib/rules/no-at-ember-render-modifiers.js
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,
});
}
},
};
},
};
60 changes: 60 additions & 0 deletions tests/lib/rules/no-at-ember-render-modifiers.js
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' }],
},
],
});

0 comments on commit 03afb9b

Please sign in to comment.