Skip to content

Commit 2eca7f8

Browse files
NullVoxPopuliclaude
andcommitted
RFC#998 - {{fn}} as keyword
Make the `fn` helper a built-in keyword so it no longer needs to be explicitly imported in strict-mode templates (gjs/gts). Building on the infrastructure from RFC#997 ({{on}}), this adds `fn` to the keywords map and extends the auto-import-builtins AST plugin to handle SubExpression and MustacheStatement nodes. The existing `on` rewrite logic is extracted into a shared `rewriteKeyword` helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3b5f130 commit 2eca7f8

File tree

7 files changed

+297
-7
lines changed

7 files changed

+297
-7
lines changed

packages/@ember/template-compiler/lib/compile-options.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { fn } from '@ember/helper';
12
import { on } from '@ember/modifier';
23
import { assert } from '@ember/debug';
34
import {
@@ -24,6 +25,7 @@ function malformedComponentLookup(string: string) {
2425
export const RUNTIME_KEYWORDS_NAME = '__ember_keywords__';
2526

2627
export const keywords: Record<string, unknown> = {
28+
fn,
2729
on,
2830
};
2931

packages/@ember/template-compiler/lib/plugins/auto-import-builtins.ts

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,48 @@ export default function autoImportBuiltins(env: EmberASTPluginEnvironment): ASTP
2323
...visitor,
2424
ElementModifierStatement(node: AST.ElementModifierStatement) {
2525
if (isOn(node, hasLocal)) {
26-
if (env.meta?.jsutils) {
27-
node.path.original = env.meta.jsutils.bindImport('@ember/modifier', 'on', node, {
28-
name: 'on',
29-
});
30-
} else if (env.meta?.emberRuntime) {
31-
node.path.original = env.meta.emberRuntime.lookupKeyword('on');
32-
}
26+
rewriteKeyword(env, node, 'on', '@ember/modifier');
27+
}
28+
},
29+
SubExpression(node: AST.SubExpression) {
30+
if (isFn(node, hasLocal)) {
31+
rewriteKeyword(env, node, 'fn', '@ember/helper');
32+
}
33+
},
34+
MustacheStatement(node: AST.MustacheStatement) {
35+
if (isFn(node, hasLocal)) {
36+
rewriteKeyword(env, node, 'fn', '@ember/helper');
3337
}
3438
},
3539
},
3640
};
3741
}
3842

43+
function rewriteKeyword(
44+
env: EmberASTPluginEnvironment,
45+
node: { path: AST.PathExpression },
46+
name: string,
47+
moduleSpecifier: string
48+
) {
49+
if (env.meta?.jsutils) {
50+
node.path.original = env.meta.jsutils.bindImport(moduleSpecifier, name, node, {
51+
name,
52+
});
53+
} else if (env.meta?.emberRuntime) {
54+
node.path.original = env.meta.emberRuntime.lookupKeyword(name);
55+
}
56+
}
57+
3958
function isOn(
4059
node: AST.ElementModifierStatement | AST.MustacheStatement | AST.SubExpression,
4160
hasLocal: (k: string) => boolean
4261
): node is AST.ElementModifierStatement & { path: AST.PathExpression } {
4362
return isPath(node.path) && node.path.original === 'on' && !hasLocal('on');
4463
}
64+
65+
function isFn(
66+
node: AST.MustacheStatement | AST.SubExpression,
67+
hasLocal: (k: string) => boolean
68+
): node is (AST.MustacheStatement | AST.SubExpression) & { path: AST.PathExpression } {
69+
return isPath(node.path) && node.path.original === 'fn' && !hasLocal('fn');
70+
}
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { castToBrowser } from '@glimmer/debug-util';
2+
import {
3+
GlimmerishComponent,
4+
jitSuite,
5+
RenderTest,
6+
test,
7+
} from '@glimmer-workspace/integration-tests';
8+
9+
import { template } from '@ember/template-compiler/runtime';
10+
11+
class KeywordFn extends RenderTest {
12+
static suiteName = 'keyword helper: fn (runtime)';
13+
14+
@test
15+
'explicit scope'(assert: Assert) {
16+
let greet = (greeting: string) => {
17+
assert.step(greeting);
18+
};
19+
20+
const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
21+
strictMode: true,
22+
scope: () => ({
23+
greet,
24+
}),
25+
});
26+
27+
this.renderComponent(compiled);
28+
29+
castToBrowser(this.element, 'div').querySelector('button')!.click();
30+
assert.verifySteps(['hello']);
31+
}
32+
33+
@test
34+
'implicit scope'(assert: Assert) {
35+
let greet = (greeting: string) => {
36+
assert.step(greeting);
37+
};
38+
39+
hide(greet);
40+
41+
const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
42+
strictMode: true,
43+
eval() {
44+
return eval(arguments[0]);
45+
},
46+
});
47+
48+
this.renderComponent(compiled);
49+
50+
castToBrowser(this.element, 'div').querySelector('button')!.click();
51+
assert.verifySteps(['hello']);
52+
}
53+
54+
@test
55+
'no eval and no scope'(assert: Assert) {
56+
class Foo extends GlimmerishComponent {
57+
static {
58+
template('<button {{on "click" (fn this.greet "hello")}}>Click</button>', {
59+
strictMode: true,
60+
component: this,
61+
});
62+
}
63+
64+
greet = (greeting: string) => assert.step(greeting);
65+
}
66+
67+
this.renderComponent(Foo);
68+
69+
castToBrowser(this.element, 'div').querySelector('button')!.click();
70+
assert.verifySteps(['hello']);
71+
}
72+
}
73+
74+
jitSuite(KeywordFn);
75+
76+
/**
77+
* This function is used to hide a variable from the transpiler, so that it
78+
* doesn't get removed as "unused". It does not actually do anything with the
79+
* variable, it just makes it be part of an expression that the transpiler
80+
* won't remove.
81+
*
82+
* It's a bit of a hack, but it's necessary for testing.
83+
*
84+
* @param variable The variable to hide.
85+
*/
86+
const hide = (variable: unknown) => {
87+
new Function(`return (${JSON.stringify(variable)});`);
88+
};
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { castToBrowser } from '@glimmer/debug-util';
2+
import { jitSuite, RenderTest, test } from '@glimmer-workspace/integration-tests';
3+
import { helperCapabilities, setHelperManager } from '@glimmer/manager';
4+
5+
import { template } from '@ember/template-compiler/runtime';
6+
import { fn } from '@ember/helper';
7+
import { on } from '@ember/modifier';
8+
9+
class KeywordFn extends RenderTest {
10+
static suiteName = 'keyword helper: fn';
11+
12+
/**
13+
* We require the babel compiler to emit keywords, so this is actually no different than normal usage
14+
* prior to RFC 998.
15+
*
16+
* We are required to have the compiler that emits this low-level format to detect if fn is in scope and then
17+
* _not_ add the `fn` helper from `@ember/helper` import.
18+
*/
19+
@test
20+
'it works'(assert: Assert) {
21+
let greet = (greeting: string) => {
22+
assert.step(greeting);
23+
};
24+
25+
const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
26+
strictMode: true,
27+
scope: () => ({
28+
greet,
29+
fn,
30+
on,
31+
}),
32+
});
33+
34+
this.renderComponent(compiled);
35+
36+
castToBrowser(this.element, 'div').querySelector('button')!.click();
37+
assert.verifySteps(['hello']);
38+
}
39+
40+
@test
41+
'it works with the runtime compiler'(assert: Assert) {
42+
let greet = (greeting: string) => {
43+
assert.step(greeting);
44+
};
45+
46+
hide(greet);
47+
48+
const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
49+
strictMode: true,
50+
eval() {
51+
return eval(arguments[0]);
52+
},
53+
});
54+
55+
this.renderComponent(compiled);
56+
57+
castToBrowser(this.element, 'div').querySelector('button')!.click();
58+
assert.verifySteps(['hello']);
59+
}
60+
61+
@test
62+
'can be shadowed'(assert: Assert) {
63+
let fn = setHelperManager(
64+
() => ({
65+
capabilities: helperCapabilities('3.23', { hasValue: true }),
66+
createHelper() {
67+
assert.step('shadowed:success');
68+
return {};
69+
},
70+
getValue() {
71+
return () => {};
72+
},
73+
}),
74+
{}
75+
);
76+
77+
let greet = () => {};
78+
79+
const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
80+
strictMode: true,
81+
scope: () => ({ fn, greet, on }),
82+
});
83+
84+
this.renderComponent(compiled);
85+
assert.verifySteps(['shadowed:success']);
86+
}
87+
}
88+
89+
jitSuite(KeywordFn);
90+
91+
/**
92+
* This function is used to hide a variable from the transpiler, so that it
93+
* doesn't get removed as "unused". It does not actually do anything with the
94+
* variable, it just makes it be part of an expression that the transpiler
95+
* won't remove.
96+
*
97+
* It's a bit of a hack, but it's necessary for testing.
98+
*
99+
* @param variable The variable to hide.
100+
*/
101+
const hide = (variable: unknown) => {
102+
new Function(`return (${JSON.stringify(variable)});`);
103+
};

packages/@glimmer-workspace/integration-tests/test/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"@glimmer/util": "workspace:*",
2222
"@glimmer/validator": "workspace:*",
2323
"@glimmer/wire-format": "workspace:*",
24+
"@ember/helper": "workspace:*",
2425
"@ember/modifier": "workspace:*",
2526
"@ember/template-compiler": "workspace:*"
2627
}

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

smoke-tests/scenarios/basic-test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,73 @@ function basicTest(scenarios: Scenarios, appName: string) {
350350
});
351351
});
352352
`,
353+
'fn-as-keyword-test.gjs': `
354+
import { module, test } from 'qunit';
355+
import { setupRenderingTest } from 'ember-qunit';
356+
import { render, click } from '@ember/test-helpers';
357+
358+
import Component from '@glimmer/component';
359+
import { tracked } from '@glimmer/tracking';
360+
361+
class Demo extends Component {
362+
@tracked message = 'hello';
363+
setMessage = (msg) => this.message = msg;
364+
365+
<template>
366+
<button {{on 'click' (fn this.setMessage 'goodbye')}}>{{this.message}}</button>
367+
</template>
368+
}
369+
370+
module('{{fn}} as keyword', function(hooks) {
371+
setupRenderingTest(hooks);
372+
373+
test('it works', async function(assert) {
374+
await render(Demo);
375+
assert.dom('button').hasText('hello');
376+
await click('button');
377+
assert.dom('button').hasText('goodbye');
378+
});
379+
});
380+
`,
381+
'fn-as-keyword-but-its-shadowed-test.gjs': `
382+
import QUnit, { module, test } from 'qunit';
383+
import { setupRenderingTest } from 'ember-qunit';
384+
import { render, click } from '@ember/test-helpers';
385+
386+
import Component from '@glimmer/component';
387+
import { tracked } from '@glimmer/tracking';
388+
import { helper } from '@ember/component/helper';
389+
390+
module('{{fn}} as keyword (but it is shadowed)', function(hooks) {
391+
setupRenderingTest(hooks);
392+
393+
test('it works', async function(assert) {
394+
// shadows keyword!
395+
const fn = helper(() => {
396+
assert.step('shadowed:fn:create');
397+
return () => {};
398+
});
399+
400+
class Demo extends Component {
401+
@tracked message = 'hello';
402+
setMessage = (msg) => this.message = msg;
403+
404+
<template>
405+
<button {{on 'click' (fn this.setMessage 'goodbye')}}>{{this.message}}</button>
406+
</template>
407+
}
408+
409+
await render(Demo);
410+
assert.verifySteps(['shadowed:fn:create']);
411+
412+
assert.dom('button').hasText('hello');
413+
await click('button');
414+
assert.dom('button').hasText('hello', 'not changed because the shadowed fn returns a no-op');
415+
416+
assert.verifySteps([]);
417+
});
418+
});
419+
`,
353420
'on-as-keyword-but-its-shadowed-test.gjs': `
354421
import QUnit, { module, test } from 'qunit';
355422
import { setupRenderingTest } from 'ember-qunit';

0 commit comments

Comments
 (0)