diff --git a/README.md b/README.md index b5aa064bc8..536503d127 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ module.exports = { | [no-classic-classes](docs/rules/no-classic-classes.md) | disallow "classic" classes in favor of native JS classes | ✅ | | | | [no-ember-super-in-es-classes](docs/rules/no-ember-super-in-es-classes.md) | disallow use of `this._super` in ES class methods | ✅ | 🔧 | | | [no-empty-glimmer-component-classes](docs/rules/no-empty-glimmer-component-classes.md) | disallow empty backing classes for Glimmer components | ✅ | | | +| [no-tracked-properties-from-args](docs/rules/no-tracked-properties-from-args.md) | disallow creating @tracked properties from this.args | | | | ### jQuery diff --git a/docs/rules/no-tracked-properties-from-args.md b/docs/rules/no-tracked-properties-from-args.md new file mode 100644 index 0000000000..975233c292 --- /dev/null +++ b/docs/rules/no-tracked-properties-from-args.md @@ -0,0 +1,35 @@ +# ember/no-tracked-properties-from-args + + + +Disallow creation of @tracked properties from args. + +## Rule Details + +This rule disallows the creation of @tracked properties with values from `this.args`. The @tracked property will not be updated when the args change, which is almost never what you want. Instead, use a getter to derive the desired state. +If you need to modify a specific arg, consider having the parent provide a way for the child component to update it. This avoids having two sources of truth that you will need to keep in sync. + +## Examples + +Examples of **incorrect** code for this rule: + +```js +class Example { + @tracked someValue = this.args.someValue; +} +``` + +Examples of **correct** code for this rule: + +```js +class Example { + get someValue() { + return this.args.someValue; + } +} +``` + +## References + +- [Glimmer Components - args](https://guides.emberjs.com/release/upgrading/current-edition/glimmer-components/#toc_getting-used-to-glimmer-components) guide +- [tracked](https://guides.emberjs.com/release/in-depth-topics/autotracking-in-depth/) guide diff --git a/lib/rules/no-tracked-properties-from-args.js b/lib/rules/no-tracked-properties-from-args.js new file mode 100644 index 0000000000..86354f4078 --- /dev/null +++ b/lib/rules/no-tracked-properties-from-args.js @@ -0,0 +1,62 @@ +'use strict'; + +const { getImportIdentifier } = require('../utils/import'); +const { isClassPropertyOrPropertyDefinitionWithDecorator } = require('../utils/decorators'); + +const ERROR_MESSAGE = + 'Do not use this.args to create @tracked properties as this will not be updated if the args change. Instead use a getter.'; + +function visitClassPropertyOrPropertyDefinition(node, context, trackedImportName) { + const hasTrackedDecorator = + node.decorators?.length > 0 && + isClassPropertyOrPropertyDefinitionWithDecorator(node, trackedImportName); + + const hasThisArgsValue = + node.value.object?.type === 'MemberExpression' && + node.value.object?.object.type === 'ThisExpression' && + node.value.object?.property.name === 'args'; + + if (hasTrackedDecorator && hasThisArgsValue) { + context.report({ node, messageId: 'main' }); + } +} + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'disallow creating @tracked properties from this.args', + category: 'Ember Octane', + recommended: false, + url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-tracked-properties-from-args.md', + }, + fixable: null, + schema: [], + messages: { + main: ERROR_MESSAGE, + }, + }, + create(context) { + let trackedImportName; + + return { + ImportDeclaration(node) { + if (node.source.value === '@glimmer/tracking') { + trackedImportName = + trackedImportName || getImportIdentifier(node, '@glimmer/tracking', 'tracked'); + } + }, + ClassProperty(node) { + visitClassPropertyOrPropertyDefinition(node, context, trackedImportName); + }, + PropertyDefinition(node) { + visitClassPropertyOrPropertyDefinition(node, context, trackedImportName); + }, + }; + }, +}; diff --git a/tests/lib/rules/no-tracked-properties-from-args.js b/tests/lib/rules/no-tracked-properties-from-args.js new file mode 100644 index 0000000000..9d79379358 --- /dev/null +++ b/tests/lib/rules/no-tracked-properties-from-args.js @@ -0,0 +1,99 @@ +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-tracked-properties-from-args'); +const RuleTester = require('eslint').RuleTester; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parser: require.resolve('@babel/eslint-parser'), + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module', + }, +}); + +ruleTester.run('no-tracked-properties-from-args', rule, { + valid: [ + ` + import { tracked } from '@glimmer/tracking' + + class Test { + @someDecorator test = this.args.test + }`, + ` + import { tracked } from '@glimmer/tracking' + + class Test { + @tracked test = this.someValue + }`, + ` + import { tracked } from '@glimmer/tracking' + + class Test { + @tracked test = 7 + }`, + ` + import { tracked } from '@glimmer/tracking' + + class Test { + test = 7 + }`, + ` + import { tracked } from '@glimmer/tracking' + + class Test { + test = "test" + }`, + ` + import { tracked } from '@glimmer/tracking' + + class Test { + @tracked test = this.notArgs.args.test + }`, + ` + import { tracked as fooTracked } from '@glimmer/tracking'; + + class Test { + @fooTracked test = this.notArgs.args.test + } + `, + ], + invalid: [ + { + code: ` + import { tracked } from '@glimmer/tracking' + + class Test { + @tracked test = this.args.test; + }`, + output: null, + errors: [ + { + messageId: 'main', + // type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8) + }, + ], + }, + { + code: ` + import { tracked as fooTracked } from '@glimmer/tracking'; + + class Test { + @fooTracked test = this.args.test + } + `, + output: null, + errors: [ + { + messageId: 'main', + // type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8) + }, + ], + }, + ], +});