Skip to content

Commit d77c61c

Browse files
committed
Fix template-no-action false positive in GJS/GTS
`action` is an ambient strict-mode keyword in Ember (registered in STRICT_MODE_KEYWORDS at @ember/template-compiler/lib/plugins/index.ts), so `{{action this.x}}` works in .gjs/.gts templates without an import. The rule should still flag those uses — its purpose is to discourage the deprecated keyword everywhere — but skip cases where `action` resolves to a JS-scope binding or a template block param: - `import action from './my-action-helper'; {{action this.x}}` (user import) - `const action = (h) => () => h(); {{action this.x}}` (local declaration) - `{{#each items as |action|}}{{action this.x}}{{/each}}` (block-param shadow) - `<Foo as |action|>{{action this.x}}</Foo>` (element-block-param shadow) Add the same JS-scope-walk + block-param tracking pattern used by template-no-log, template-no-unbound, etc.: walk sourceCode.getScope().variables for the JS side, and push/pop GlimmerBlockStatement.program.blockParams and GlimmerElementNode.blockParams for template-side scopes. The `@action` and `this.action` head-type checks are unchanged (those are JS-side namespaces, not the template keyword). Tests: 22 (was 8) — adds 9 new valid cases (5 JS-scope, 4 block-param) and 5 new invalid cases (3 ambient-keyword in .gjs/.gts, 1 unrelated import does not mask, 1 ambient-after-block-exit). Docs updated to document the strict-mode behavior.
1 parent b705850 commit d77c61c

3 files changed

Lines changed: 130 additions & 25 deletions

File tree

docs/rules/template-no-action.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,25 @@ Examples of **correct** code for this rule:
4242
</template>
4343
```
4444

45+
```gjs
46+
import action from './my-action-helper';
47+
<template>
48+
{{action this.handleClick}}
49+
</template>
50+
```
51+
52+
```gjs
53+
<template>
54+
{{#each items as |action|}}
55+
<button {{action this.handleClick}}>x</button>
56+
{{/each}}
57+
</template>
58+
```
59+
60+
## Strict-mode behavior
61+
62+
`action` is an ambient strict-mode keyword in Ember (registered in `STRICT_MODE_KEYWORDS`), so `{{action this.x}}` works in `.gjs`/`.gts` templates without an explicit import. The rule still flags those uses to discourage the deprecated keyword — but skips reports when `action` resolves to a JS-scope binding (an import or local declaration) or a template block param.
63+
4564
## Migration
4665

4766
- Replace `(action "methodName")` with method references or `(fn this.methodName)`

lib/rules/template-no-action.js

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
1-
function isActionHelper(node) {
1+
function isActionHelperPath(node) {
22
if (!node.path || node.path.type !== 'GlimmerPathExpression') {
33
return false;
44
}
55

66
// Check if it's the action helper (not this.action or @action)
77
const path = node.path;
8-
if (path.original === 'action') {
9-
// Check head.type to avoid deprecated data/this properties
10-
const head = path.head;
11-
if (head && (head.type === 'AtHead' || head.type === 'ThisHead')) {
12-
return false;
13-
}
14-
return true;
8+
if (path.original !== 'action') {
9+
return false;
1510
}
16-
17-
return false;
11+
// Avoid deprecated data/this properties — those are not the helper.
12+
const head = path.head;
13+
if (head && (head.type === 'AtHead' || head.type === 'ThisHead')) {
14+
return false;
15+
}
16+
return true;
1817
}
1918

2019
/** @type {import('eslint').Rule.RuleModule} */
@@ -39,31 +38,48 @@ module.exports = {
3938
},
4039

4140
create(context) {
41+
// `action` is an ambient strict-mode keyword in Ember (registered in
42+
// STRICT_MODE_KEYWORDS), so `{{action this.x}}` works in .gjs/.gts without
43+
// an import. Still flag the ambient keyword everywhere — but skip when
44+
// `action` resolves to a binding (JS import/const, or template block param).
45+
// ember-eslint-parser already registers template block params in scope, so
46+
// a single getScope walk covers both.
47+
const sourceCode = context.sourceCode;
48+
49+
function isInScope(node) {
50+
if (!sourceCode) return false;
51+
try {
52+
let scope = sourceCode.getScope(node);
53+
while (scope) {
54+
if (scope.variables.some((v) => v.name === 'action')) return true;
55+
scope = scope.upper;
56+
}
57+
} catch {
58+
// getScope not available in .hbs-only mode
59+
}
60+
return false;
61+
}
62+
63+
function shouldFlag(node) {
64+
return isActionHelperPath(node) && !isInScope(node);
65+
}
66+
4267
return {
4368
GlimmerSubExpression(node) {
44-
if (isActionHelper(node)) {
45-
context.report({
46-
node,
47-
messageId: 'subExpression',
48-
});
69+
if (shouldFlag(node)) {
70+
context.report({ node, messageId: 'subExpression' });
4971
}
5072
},
5173

5274
GlimmerMustacheStatement(node) {
53-
if (isActionHelper(node)) {
54-
context.report({
55-
node,
56-
messageId: 'mustache',
57-
});
75+
if (shouldFlag(node)) {
76+
context.report({ node, messageId: 'mustache' });
5877
}
5978
},
6079

6180
GlimmerElementModifierStatement(node) {
62-
if (isActionHelper(node)) {
63-
context.report({
64-
node,
65-
messageId: 'modifier',
66-
});
81+
if (shouldFlag(node)) {
82+
context.report({ node, messageId: 'modifier' });
6783
}
6884
},
6985
};

tests/lib/rules/template-no-action.js

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,37 @@ ruleTester.run('template-no-action', rule, {
2828
`<template>
2929
{{@action}}
3030
</template>`,
31+
32+
// GJS/GTS: a JS-scope binding shadows the ambient `action` keyword. The
33+
// user has imported (or declared) their own `action`, so flagging would
34+
// corrupt their reference.
35+
{
36+
filename: 'test.gjs',
37+
code: "import action from './my-action';\n<template>{{action this.handleClick}}</template>",
38+
},
39+
{
40+
filename: 'test.gjs',
41+
code: "import action from './my-action';\n<template>{{(action this.handleClick)}}</template>",
42+
},
43+
{
44+
filename: 'test.gjs',
45+
code: "import action from './my-action';\n<template><button {{action this.handleClick}}>x</button></template>",
46+
},
47+
{
48+
filename: 'test.gts',
49+
code: "import action from './my-action';\n<template>{{action this.handleClick}}</template>",
50+
},
51+
{
52+
filename: 'test.gjs',
53+
code: 'const action = (h) => () => h();\n<template>{{action this.handleClick}}</template>',
54+
},
55+
56+
// Template block-param shadowing — `action` is the iterator/let-bound
57+
// value, not the ambient keyword.
58+
'<template>{{#each items as |action|}}{{action this.x}}{{/each}}</template>',
59+
'<template>{{#let (component "x") as |action|}}{{action this.x}}{{/let}}</template>',
60+
'<template>{{#each items as |action|}}<button {{action this.x}}>x</button>{{/each}}</template>',
61+
'<template><Foo as |action|>{{action this.x}}</Foo></template>',
3162
],
3263

3364
invalid: [
@@ -83,5 +114,44 @@ ruleTester.run('template-no-action', rule, {
83114
},
84115
],
85116
},
117+
118+
// GJS/GTS: ambient `action` keyword usage with no shadowing import or
119+
// block param. Still flagged.
120+
{
121+
filename: 'test.gjs',
122+
code: '<template>{{action "save"}}</template>',
123+
output: null,
124+
errors: [{ messageId: 'mustache', type: 'GlimmerMustacheStatement' }],
125+
},
126+
{
127+
filename: 'test.gjs',
128+
code: '<template><button {{on "click" (action "save")}}>x</button></template>',
129+
output: null,
130+
errors: [{ messageId: 'subExpression', type: 'GlimmerSubExpression' }],
131+
},
132+
{
133+
filename: 'test.gts',
134+
code: '<template><button {{action "submit"}}>x</button></template>',
135+
output: null,
136+
errors: [{ messageId: 'modifier', type: 'GlimmerElementModifierStatement' }],
137+
},
138+
139+
// Unrelated JS bindings do NOT mask the rule. Only a binding named
140+
// `action` should be respected.
141+
{
142+
filename: 'test.gjs',
143+
code: "import handler from './handler';\n<template>{{action this.x}}</template>",
144+
output: null,
145+
errors: [{ messageId: 'mustache' }],
146+
},
147+
148+
// Ambient `action` outside a block-param scope is still flagged, even
149+
// when an inner block legitimately shadows it.
150+
{
151+
filename: 'test.gjs',
152+
code: '<template>{{#each items as |action|}}{{action this.x}}{{/each}}{{action this.y}}</template>',
153+
output: null,
154+
errors: [{ messageId: 'mustache' }],
155+
},
86156
],
87157
});

0 commit comments

Comments
 (0)