Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ lib/recommended-rules-gts.js

# Contains <template> in js markdown
docs/rules/no-empty-glimmer-component-classes.md
docs/rules/template-no-let-reference.md
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ rules in templates can be disabled with eslint directives with mustache or html
| [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 | ✅ | | |
| [template-no-let-reference](docs/rules/template-no-let-reference.md) | disallow referencing let variables in \<template\> | | | |

### jQuery

Expand Down
58 changes: 58 additions & 0 deletions docs/rules/template-no-let-reference.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# ember/template-no-let-reference

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

Disallows referencing let/var variables in templates.

```js
// app/components/foo.gjs
let foo = 1;

function increment() {
foo++;
}

export default <template>{{foo}}</template>;
```

This does not "work" – it doesn't error, but it will essentially capture and compile in the value of the foo variable at some arbitrary point and never update again. Even if the component is torn down and a new instance is created/rendered, it will probably still hold on to the old value when the template was initially compiled.

So, generally speaking, one should avoid referencing let variables from within &lt;template&gt; and instead prefer to use const bindings.

## Rule Detail

Use `const` variables instead of `let`.

## Examples

Examples of **incorrect** code for this rule:

```js
let x = 1;
<template>
{{x}}
</template>
```

```js
let Comp = x; // SomeComponent
<template>
<Comp />
</template>
```

Examples of **correct** code for this rule:

```js
const x = 1;
<template>
{{x}}
</template>
```

```js
const Comp = x; // SomeComponent
<template>
<Comp />
</template>
```
28 changes: 14 additions & 14 deletions lib/parsers/gjs-gts-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,20 +377,6 @@ function convertAst(result, preprocessedResult, visitorKeys) {
Object.assign(node, ast);
}

if ('blockParams' in node) {
const upperScope = findParentScope(result.scopeManager, path);
const scope = result.isTypescript
? new TypescriptScope.BlockScope(result.scopeManager, upperScope, node)
: new Scope(result.scopeManager, 'block', upperScope, node);
for (const [i, b] of node.params.entries()) {
const v = new Variable(b.name, scope);
v.identifiers.push(b);
v.defs.push(new Definition('Parameter', b, node, node, i, 'Block Param'));
scope.variables.push(v);
scope.set.set(b.name, v);
}
}

if (node.type === 'GlimmerPathExpression' && node.head.type === 'VarHead') {
const name = node.head.name;
if (glimmer.isKeyword(name)) {
Expand All @@ -409,6 +395,20 @@ function convertAst(result, preprocessedResult, visitorKeys) {
registerNodeInScope(node, scope, variable);
}
}

if ('blockParams' in node) {
const upperScope = findParentScope(result.scopeManager, path);
const scope = result.isTypescript
? new TypescriptScope.BlockScope(result.scopeManager, upperScope, node)
: new Scope(result.scopeManager, 'block', upperScope, node);
for (const [i, b] of node.params.entries()) {
const v = new Variable(b.name, scope);
v.identifiers.push(b);
v.defs.push(new Definition('Parameter', b, node, node, i, 'Block Param'));
scope.variables.push(v);
scope.set.set(b.name, v);
}
}
return null;
});

Expand Down
44 changes: 44 additions & 0 deletions lib/rules/template-no-let-reference.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
const ERROR_MESSAGE =
'update-able variables are not supported in templates, reference a const variable';

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
ERROR_MESSAGE,
meta: {
type: 'suggestion',
docs: {
description: 'disallow referencing let variables in \\<template\\>',
category: 'Ember Octane',
recommended: false,
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-let-reference.md',
},
fixable: null,
schema: [],
},

create: (context) => {
const sourceCode = context.sourceCode;

function checkIfWritableReferences(node, scope) {
const ref = scope.references.find((v) => v.identifier === node);
if (!ref) {
return;
}
if (ref.resolved?.identifiers.some((i) => ['let', 'var'].includes(i.parent.parent.kind))) {
context.report({ node, message: ERROR_MESSAGE });
}
}

return {
GlimmerPathExpression(node) {
checkIfWritableReferences(node.head, sourceCode.getScope(node));
},

GlimmerElementNode(node) {
// glimmer element is in its own scope, need tp get upper scope
const scope = sourceCode.getScope(node.parent);
checkIfWritableReferences(node, scope);
},
};
},
};
64 changes: 64 additions & 0 deletions tests/lib/rules/template-no-let-reference.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require('../../../lib/rules/template-no-let-reference');
const RuleTester = require('eslint').RuleTester;

const { ERROR_MESSAGE } = rule;

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({
parser: require.resolve('../../../lib/parsers/gjs-gts-parser.js'),
parserOptions: { ecmaVersion: 2020, sourceType: 'module' },
});
ruleTester.run('template-no-let-reference', rule, {
valid: [
`
const a = '';
<template>
{{a}}
</template>
`,
`
const a = '';
<template>
<a></a>
</template>
`,
// make sure rule does not error out on missing references
`
const a = '';
<template>
{{ab}}
<ab></ab>
</template>
`,
],

invalid: [
{
code: `
let a = '';
<template>
{{a}}
</template>
`,
output: null,
errors: [{ type: 'VarHead', message: ERROR_MESSAGE }],
},
{
code: `
var a = '';
<template>
<a></a>
</template>
`,
output: null,
errors: [{ type: 'GlimmerElementNode', message: ERROR_MESSAGE }],
},
],
});