Skip to content

Commit

Permalink
Add new rule no-tracked-properties-from-args (#1702)
Browse files Browse the repository at this point in the history
Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>
Co-authored-by: Joan Cejudo <joan@joans-mbp.myfiosgateway.com>
Co-authored-by: Joan Cejudo <joan@Joans-MacBook-Pro.local>
Fixes #1701
  • Loading branch information
joancc committed Dec 19, 2022
1 parent ba7eab2 commit a2e2586
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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

Expand Down
35 changes: 35 additions & 0 deletions docs/rules/no-tracked-properties-from-args.md
@@ -0,0 +1,35 @@
# ember/no-tracked-properties-from-args

<!-- end auto-generated rule header -->

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
62 changes: 62 additions & 0 deletions 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);
},
};
},
};
99 changes: 99 additions & 0 deletions 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)
},
],
},
],
});

0 comments on commit a2e2586

Please sign in to comment.