Merge pull request #21299 from NullVoxPopuli-ai-agent/nvp/fn-as-keyword

RFC#998 - {{fn}} as keyword

Author: NullVoxPopuli

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

@@ -1,3 +1,4 @@
+import { fn } from '@ember/helper';
 import { on } from '@ember/modifier';
 import { assert } from '@ember/debug';
 import {
@@ -24,6 +25,7 @@ function malformedComponentLookup(string: string) {
 export const RUNTIME_KEYWORDS_NAME = '__ember_keywords__';
 
 export const keywords: Record<string, unknown> = {
+  fn,
   on,
 };
 

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

@@ -23,22 +23,48 @@ export default function autoImportBuiltins(env: EmberASTPluginEnvironment): ASTP
       ...visitor,
       ElementModifierStatement(node: AST.ElementModifierStatement) {
         if (isOn(node, hasLocal)) {
-          if (env.meta?.jsutils) {
-            node.path.original = env.meta.jsutils.bindImport('@ember/modifier', 'on', node, {
-              name: 'on',
-            });
-          } else if (env.meta?.emberRuntime) {
-            node.path.original = env.meta.emberRuntime.lookupKeyword('on');
-          }
+          rewriteKeyword(env, node, 'on', '@ember/modifier');
+        }
+      },
+      SubExpression(node: AST.SubExpression) {
+        if (isFn(node, hasLocal)) {
+          rewriteKeyword(env, node, 'fn', '@ember/helper');
+        }
+      },
+      MustacheStatement(node: AST.MustacheStatement) {
+        if (isFn(node, hasLocal)) {
+          rewriteKeyword(env, node, 'fn', '@ember/helper');
         }
       },
     },
   };
 }
 
+function rewriteKeyword(
+  env: EmberASTPluginEnvironment,
+  node: { path: AST.PathExpression },
+  name: string,
+  moduleSpecifier: string
+) {
+  if (env.meta?.jsutils) {
+    node.path.original = env.meta.jsutils.bindImport(moduleSpecifier, name, node, {
+      name,
+    });
+  } else if (env.meta?.emberRuntime) {
+    node.path.original = env.meta.emberRuntime.lookupKeyword(name);
+  }
+}
+
 function isOn(
   node: AST.ElementModifierStatement | AST.MustacheStatement | AST.SubExpression,
   hasLocal: (k: string) => boolean
 ): node is AST.ElementModifierStatement & { path: AST.PathExpression } {
   return isPath(node.path) && node.path.original === 'on' && !hasLocal('on');
 }
+
+function isFn(
+  node: AST.MustacheStatement | AST.SubExpression,
+  hasLocal: (k: string) => boolean
+): node is (AST.MustacheStatement | AST.SubExpression) & { path: AST.PathExpression } {
+  return isPath(node.path) && node.path.original === 'fn' && !hasLocal('fn');
+}

packages/@glimmer-workspace/integration-tests/test/keywords/fn-runtime-test.ts

@@ -0,0 +1,113 @@
+import { castToBrowser } from '@glimmer/debug-util';
+import {
+  GlimmerishComponent,
+  jitSuite,
+  RenderTest,
+  test,
+} from '@glimmer-workspace/integration-tests';
+
+import { template } from '@ember/template-compiler/runtime';
+
+class KeywordFn extends RenderTest {
+  static suiteName = 'keyword helper: fn (runtime)';
+
+  @test
+  'explicit scope'(assert: Assert) {
+    let greet = (greeting: string) => {
+      assert.step(greeting);
+    };
+
+    const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
+      strictMode: true,
+      scope: () => ({
+        greet,
+      }),
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['hello']);
+  }
+
+  @test
+  'implicit scope'(assert: Assert) {
+    let greet = (greeting: string) => {
+      assert.step(greeting);
+    };
+
+    hide(greet);
+
+    const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
+      strictMode: true,
+      eval() {
+        return eval(arguments[0]);
+      },
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['hello']);
+  }
+
+  @test
+  'MustacheStatement with explicit scope'(assert: Assert) {
+    let greet = (greeting: string) => {
+      assert.step(greeting);
+    };
+
+    const Child = template('<button {{on "click" @callback}}>Click</button>', {
+      strictMode: true,
+      scope: () => ({}),
+    });
+
+    const compiled = template('<Child @callback={{fn greet "hello"}} />', {
+      strictMode: true,
+      scope: () => ({
+        greet,
+        Child,
+      }),
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['hello']);
+  }
+
+  @test
+  'no eval and no scope'(assert: Assert) {
+    class Foo extends GlimmerishComponent {
+      static {
+        template('<button {{on "click" (fn this.greet "hello")}}>Click</button>', {
+          strictMode: true,
+          component: this,
+        });
+      }
+
+      greet = (greeting: string) => assert.step(greeting);
+    }
+
+    this.renderComponent(Foo);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['hello']);
+  }
+}
+
+jitSuite(KeywordFn);
+
+/**
+ * This function is used to hide a variable from the transpiler, so that it
+ * doesn't get removed as "unused". It does not actually do anything with the
+ * variable, it just makes it be part of an expression that the transpiler
+ * won't remove.
+ *
+ * It's a bit of a hack, but it's necessary for testing.
+ *
+ * @param variable The variable to hide.
+ */
+const hide = (variable: unknown) => {
+  new Function(`return (${JSON.stringify(variable)});`);
+};

packages/@glimmer-workspace/integration-tests/test/keywords/fn-test.ts

@@ -0,0 +1,119 @@
+import { castToBrowser } from '@glimmer/debug-util';
+import { jitSuite, RenderTest, test } from '@glimmer-workspace/integration-tests';
+
+import { template } from '@ember/template-compiler/runtime';
+import { fn } from '@ember/helper';
+import { on } from '@ember/modifier';
+
+class KeywordFn extends RenderTest {
+  static suiteName = 'keyword helper: fn';
+
+  /**
+   * We require the babel compiler to emit keywords, so this is actually no different than normal usage
+   * prior to RFC 998.
+   *
+   * We are required to have the compiler that emits this low-level format to detect if fn is in scope and then
+   * _not_ add the `fn` helper from `@ember/helper` import.
+   */
+  @test
+  'it works'(assert: Assert) {
+    let greet = (greeting: string) => {
+      assert.step(greeting);
+    };
+
+    const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
+      strictMode: true,
+      scope: () => ({
+        greet,
+        fn,
+        on,
+      }),
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['hello']);
+  }
+
+  @test
+  'it works with the runtime compiler'(assert: Assert) {
+    let greet = (greeting: string) => {
+      assert.step(greeting);
+    };
+
+    hide(greet);
+
+    const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
+      strictMode: true,
+      eval() {
+        return eval(arguments[0]);
+      },
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['hello']);
+  }
+
+  @test
+  'it works as a MustacheStatement'(assert: Assert) {
+    let greet = (greeting: string) => {
+      assert.step(greeting);
+    };
+
+    const Child = template('<button {{on "click" @callback}}>Click</button>', {
+      strictMode: true,
+      scope: () => ({ on }),
+    });
+
+    const compiled = template('<Child @callback={{fn greet "hello"}} />', {
+      strictMode: true,
+      scope: () => ({
+        greet,
+        fn,
+        Child,
+      }),
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['hello']);
+  }
+
+  @test
+  'can be shadowed'(assert: Assert) {
+    let fn = () => {
+      assert.step('shadowed:success');
+      return () => {};
+    };
+
+    let greet = () => {};
+
+    const compiled = template('<button {{on "click" (fn greet "hello")}}>Click</button>', {
+      strictMode: true,
+      scope: () => ({ fn, greet, on }),
+    });
+
+    this.renderComponent(compiled);
+    assert.verifySteps(['shadowed:success']);
+  }
+}
+
+jitSuite(KeywordFn);
+
+/**
+ * This function is used to hide a variable from the transpiler, so that it
+ * doesn't get removed as "unused". It does not actually do anything with the
+ * variable, it just makes it be part of an expression that the transpiler
+ * won't remove.
+ *
+ * It's a bit of a hack, but it's necessary for testing.
+ *
+ * @param variable The variable to hide.
+ */
+const hide = (variable: unknown) => {
+  new Function(`return (${JSON.stringify(variable)});`);
+};

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

@@ -21,6 +21,7 @@
     "@glimmer/util": "workspace:*",
     "@glimmer/validator": "workspace:*",
     "@glimmer/wire-format": "workspace:*",
+    "@ember/helper": "workspace:*",
     "@ember/modifier": "workspace:*",
     "@ember/template-compiler": "workspace:*"
   }

pnpm-lock.yaml

@@ -350,6 +350,72 @@ function basicTest(scenarios: Scenarios, appName: string) {
                 });
               });
             `,
+            'fn-as-keyword-test.gjs': `
+              import { module, test } from 'qunit';
+              import { setupRenderingTest } from 'ember-qunit';
+              import { render, click } from '@ember/test-helpers';
+
+              import Component from '@glimmer/component';
+              import { tracked } from '@glimmer/tracking';
+
+              class Demo extends Component {
+                @tracked message = 'hello';
+                setMessage = (msg) => this.message = msg;
+
+                <template>
+                  <button {{on 'click' (fn this.setMessage 'goodbye')}}>{{this.message}}</button>
+                </template>
+              }
+
+              module('{{fn}} as keyword', function(hooks) {
+                setupRenderingTest(hooks);
+
+                test('it works', async function(assert) {
+                  await render(Demo);
+                  assert.dom('button').hasText('hello');
+                  await click('button');
+                  assert.dom('button').hasText('goodbye');
+                });
+              });
+            `,
+            'fn-as-keyword-but-its-shadowed-test.gjs': `
+              import QUnit, { module, test } from 'qunit';
+              import { setupRenderingTest } from 'ember-qunit';
+              import { render, click } from '@ember/test-helpers';
+
+              import Component from '@glimmer/component';
+              import { tracked } from '@glimmer/tracking';
+
+              module('{{fn}} as keyword (but it is shadowed)', function(hooks) {
+                setupRenderingTest(hooks);
+
+                test('it works', async function(assert) {
+                  // shadows keyword!
+                  const fn = () => {
+                    assert.step('shadowed:fn:invoke');
+                    return () => {};
+                  };
+
+                  class Demo extends Component {
+                    @tracked message = 'hello';
+                    setMessage = (msg) => this.message = msg;
+
+                    <template>
+                      <button {{on 'click' (fn this.setMessage 'goodbye')}}>{{this.message}}</button>
+                    </template>
+                  }
+
+                  await render(Demo);
+                  assert.verifySteps(['shadowed:fn:invoke']);
+
+                  assert.dom('button').hasText('hello');
+                  await click('button');
+                  assert.dom('button').hasText('hello', 'not changed because the shadowed fn returns a no-op');
+
+                  assert.verifySteps([]);
+                });
+              });
+            `,
             'on-as-keyword-but-its-shadowed-test.gjs': `
               import QUnit, { module, test } from 'qunit';
               import { setupRenderingTest } from 'ember-qunit';