Merge pull request #21068 from emberjs/nvp/on-as-keyword-tremplate-the-transform-way

RFC#997 - {{on}} as keyword

Author: NullVoxPopuli

package.json

@@ -286,6 +286,7 @@
       "@ember/template-compiler/lib/plugins/assert-against-named-outlets.js": "ember-source/@ember/template-compiler/lib/plugins/assert-against-named-outlets.js",
       "@ember/template-compiler/lib/plugins/assert-input-helper-without-block.js": "ember-source/@ember/template-compiler/lib/plugins/assert-input-helper-without-block.js",
       "@ember/template-compiler/lib/plugins/assert-reserved-named-arguments.js": "ember-source/@ember/template-compiler/lib/plugins/assert-reserved-named-arguments.js",
+      "@ember/template-compiler/lib/plugins/auto-import-builtins.js": "ember-source/@ember/template-compiler/lib/plugins/auto-import-builtins.js",
       "@ember/template-compiler/lib/plugins/index.js": "ember-source/@ember/template-compiler/lib/plugins/index.js",
       "@ember/template-compiler/lib/plugins/transform-action-syntax.js": "ember-source/@ember/template-compiler/lib/plugins/transform-action-syntax.js",
       "@ember/template-compiler/lib/plugins/transform-each-in-into-each.js": "ember-source/@ember/template-compiler/lib/plugins/transform-each-in-into-each.js",

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

@@ -1,3 +1,4 @@
+import { on } from '@ember/modifier';
 import { assert } from '@ember/debug';
 import {
   RESOLUTION_MODE_TRANSFORMS,
@@ -14,11 +15,22 @@ function malformedComponentLookup(string: string) {
   return string.indexOf('::') === -1 && string.indexOf(':') > -1;
 }
 
+/**
+ * The variable name used to inject the keywords object into the
+ * template's evaluation scope. auto-import-builtins rewrites bare
+ * keyword references (e.g. `on`) to property accesses on this
+ * variable (e.g. `__ember_keywords__.on`).
+ */
+export const RUNTIME_KEYWORDS_NAME = '__ember_keywords__';
+
+export const keywords: Record<string, unknown> = {
+  on,
+};
+
 function buildCompileOptions(_options: EmberPrecompileOptions): EmberPrecompileOptions {
   let moduleName = _options.moduleName;
 
-  let options: EmberPrecompileOptions & Partial<EmberPrecompileOptions> = {
-    meta: {},
+  let options = {
     isProduction: false,
     plugins: { ast: [] },
     ..._options,
@@ -35,11 +47,29 @@ function buildCompileOptions(_options: EmberPrecompileOptions): EmberPrecompileO
     },
   };
 
-  if ('eval' in options) {
-    const localScopeEvaluator = options.eval as (value: string) => unknown;
+  options.meta ||= {};
+  options.meta.emberRuntime ||= {
+    lookupKeyword(name: string): string {
+      assert(
+        `${name} is not a known keyword. Available keywords: ${Object.keys(keywords).join(', ')}`,
+        name in keywords
+      );
+
+      return `${RUNTIME_KEYWORDS_NAME}.${name}`;
+    },
+  };
+
+  if ('eval' in options && options.eval) {
+    const localScopeEvaluator = options.eval;
     const globalScopeEvaluator = (value: string) => new Function(`return ${value};`)();
 
     options.lexicalScope = (variable: string) => {
+      // The keywords container variable is always "in scope" —
+      // we inject it via the evaluator in template.ts.
+      if (variable === RUNTIME_KEYWORDS_NAME) {
+        return true;
+      }
+
       if (ALLOWED_GLOBALS.has(variable)) {
         return variable in globalThis;
       }
@@ -57,13 +87,18 @@ function buildCompileOptions(_options: EmberPrecompileOptions): EmberPrecompileO
   if ('scope' in options) {
     const scope = (options.scope as () => Record<string, unknown>)();
 
-    options.lexicalScope = (variable: string) => {
-      return variable in scope;
-    };
+    options.lexicalScope = (variable: string) =>
+      variable in scope || variable === RUNTIME_KEYWORDS_NAME;
 
     delete options.scope;
   }
 
+  // When neither eval nor scope is provided, the keywords container
+  // still needs to be visible to the compiler.
+  if (!options.lexicalScope) {
+    options.lexicalScope = (variable: string) => variable === RUNTIME_KEYWORDS_NAME;
+  }
+
   if ('locals' in options && !options.locals) {
     // Glimmer's precompile options declare `locals` like:
     //    locals?: string[]

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

@@ -0,0 +1,44 @@
+import type { AST, ASTPlugin } from '@glimmer/syntax';
+import type { EmberASTPluginEnvironment } from '../types';
+import { isPath, trackLocals } from './utils';
+
+/**
+ @module ember
+*/
+
+/**
+  A Glimmer2 AST transformation that makes importable keywords work
+
+  @private
+  @class TransformActionSyntax
+*/
+
+export default function autoImportBuiltins(env: EmberASTPluginEnvironment): ASTPlugin {
+  let { hasLocal, visitor } = trackLocals(env);
+
+  return {
+    name: 'auto-import-built-ins',
+
+    visitor: {
+      ...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');
+          }
+        }
+      },
+    },
+  };
+}
+
+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');
+}

packages/@ember/template-compiler/lib/plugins/index.ts

@@ -9,8 +9,10 @@ import TransformInElement from './transform-in-element';
 import TransformQuotedBindingsIntoJustBindings from './transform-quoted-bindings-into-just-bindings';
 import TransformResolutions from './transform-resolutions';
 import TransformWrapMountAndOutlet from './transform-wrap-mount-and-outlet';
+import AutoImportBuiltins from './auto-import-builtins';
 
 export const INTERNAL_PLUGINS = {
+  AutoImportBuiltins,
   AssertAgainstAttrs,
   AssertAgainstNamedOutlets,
   AssertInputHelperWithoutBlock,
@@ -40,6 +42,7 @@ export const RESOLUTION_MODE_TRANSFORMS = Object.freeze([
 ]);
 
 export const STRICT_MODE_TRANSFORMS = Object.freeze([
+  AutoImportBuiltins,
   TransformQuotedBindingsIntoJustBindings,
   AssertReservedNamedArguments,
   TransformActionSyntax,

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

@@ -3,7 +3,7 @@ import { precompile as glimmerPrecompile } from '@glimmer/compiler';
 import type { SerializedTemplateWithLazyBlock } from '@glimmer/interfaces';
 import { setComponentTemplate } from '@glimmer/manager';
 import { templateFactory } from '@glimmer/opcode-compiler';
-import compileOptions from './compile-options';
+import compileOptions, { keywords, RUNTIME_KEYWORDS_NAME } from './compile-options';
 import type { EmberPrecompileOptions } from './types';
 
 type ComponentClass = abstract new (...args: any[]) => object;
@@ -237,38 +237,54 @@ export function template(
   templateString: string,
   providedOptions?: BaseTemplateOptions | BaseClassTemplateOptions<any>
 ): object {
-  const options: EmberPrecompileOptions = { strictMode: true, ...providedOptions };
-  const evaluate = buildEvaluator(options);
+  const options = { strictMode: true, ...providedOptions };
 
+  const evaluate = buildEvaluator(options);
   const normalizedOptions = compileOptions(options);
   const component = normalizedOptions.component ?? templateOnly();
 
   const source = glimmerPrecompile(templateString, normalizedOptions);
-  const template = templateFactory(evaluate(`(${source})`) as SerializedTemplateWithLazyBlock);
+  const wire = evaluate(`(${source})`) as SerializedTemplateWithLazyBlock;
+
+  const template = templateFactory(wire);
 
   setComponentTemplate(template, component);
 
   return component;
 }
 
-const evaluator = (source: string) => {
-  return new Function(`return  ${source}`)();
-};
+/**
+ * Builds the source wireformat JSON block
+ *
+ * @param options
+ * @returns
+ */
+function buildEvaluator(options: Partial<EmberPrecompileOptions>) {
+  if (options.eval) {
+    const userEval = options.eval;
 
-function buildEvaluator(options: Partial<EmberPrecompileOptions> | undefined) {
-  if (options === undefined) {
-    return evaluator;
-  }
+    // Wrap the compiled source in a function that receives the keywords
+    // container as a parameter. The user's eval evaluates this in the
+    // caller's scope, so local variables (like `handleClick`) are captured
+    // via closure, while `__keywords__` comes from the function parameter.
+    return (source: string) => {
+      let wrapperFn = userEval(`(function(${RUNTIME_KEYWORDS_NAME}){ return (${source}); })`) as (
+        ...args: unknown[]
+      ) => unknown;
 
-  if (options.eval) {
-    return options.eval;
+      return wrapperFn(keywords);
+    };
   } else {
-    const scope = options.scope?.();
+    let scope = options.scope?.();
 
     if (!scope) {
-      return evaluator;
+      return (source: string) => {
+        return new Function(RUNTIME_KEYWORDS_NAME, `return (${source})`)(keywords);
+      };
     }
 
+    scope = Object.assign({ [RUNTIME_KEYWORDS_NAME]: keywords }, scope);
+
     return (source: string) => {
       let hasThis = Object.prototype.hasOwnProperty.call(scope, 'this');
       let thisValue = hasThis ? (scope as { this?: unknown }).this : undefined;

packages/@ember/template-compiler/lib/types.ts

@@ -1,4 +1,5 @@
 import type {
+  ASTPluginBuilder,
   ASTPluginEnvironment,
   builders,
   PrecompileOptions,
@@ -13,21 +14,39 @@ export type Builders = typeof builders;
  * typing. Here export the interface subclass with no modification.
  */
 
-export type PluginFunc = NonNullable<
-  NonNullable<PrecompileOptionsWithLexicalScope['plugins']>['ast']
->[number];
+export type PluginFunc = ASTPluginBuilder<EmberASTPluginEnvironment>;
 
 export type LexicalScope = NonNullable<PrecompileOptionsWithLexicalScope['lexicalScope']>;
 
 interface Plugins {
   ast: PluginFunc[];
 }
 
-export interface EmberPrecompileOptions extends PrecompileOptions {
+export interface EmberPrecompileOptions extends Omit<PrecompileOptions, 'meta'> {
   isProduction?: boolean;
   moduleName?: string;
   plugins?: Plugins;
   lexicalScope?: LexicalScope;
+  meta?: {
+    /**
+     * Exists for historical reasons, should not be in new code, as
+     * the module name does not correspond to anything meaningful at runtime.
+     */
+    moduleName?: string | undefined;
+
+    /**
+     * Not available at runtime
+     */
+    jsutils?: { bindImport: (...args: unknown[]) => string };
+
+    /**
+     * Utils unique to the runtime compiler
+     */
+    emberRuntime?: {
+      lookupKeyword(name: string): string;
+    };
+  };
+
   /**
    * This supports template blocks defined in class bodies.
    *

packages/@ember/template-compiler/package.json

@@ -12,6 +12,8 @@
     "@ember/-internals": "workspace:*",
     "@ember/component": "workspace:*",
     "@ember/debug": "workspace:*",
+    "@ember/modifier": "workspace:*",
+    "@ember/helper": "workspace:*",
     "@glimmer/compiler": "workspace:*",
     "@glimmer/env": "workspace:*",
     "@glimmer/interfaces": "workspace:*",

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

@@ -0,0 +1,88 @@
+import { castToBrowser } from '@glimmer/debug-util';
+import {
+  GlimmerishComponent,
+  jitSuite,
+  RenderTest,
+  test,
+} from '@glimmer-workspace/integration-tests';
+
+import { template } from '@ember/template-compiler/runtime';
+
+class KeywordOn extends RenderTest {
+  static suiteName = 'keyword modifier: on (runtime)';
+
+  @test
+  'explicit scope'(assert: Assert) {
+    let handleClick = () => {
+      assert.step('success');
+    };
+
+    const compiled = template('<button {{on "click" handleClick}}>Click</button>', {
+      strictMode: true,
+      scope: () => ({
+        handleClick,
+      }),
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['success']);
+  }
+
+  @test
+  'implicit scope'(assert: Assert) {
+    let handleClick = () => {
+      assert.step('success');
+    };
+
+    hide(handleClick);
+
+    const compiled = template('<button {{on "click" handleClick}}>Click</button>', {
+      strictMode: true,
+      eval() {
+        return eval(arguments[0]);
+      },
+    });
+
+    this.renderComponent(compiled);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['success']);
+  }
+
+  @test
+  'no eval and no scope'(assert: Assert) {
+    class Foo extends GlimmerishComponent {
+      static {
+        template('<button {{on "click" this.handleClick}}>Click</button>', {
+          strictMode: true,
+          component: this,
+        });
+      }
+
+      handleClick = () => assert.step('success');
+    }
+
+    this.renderComponent(Foo);
+
+    castToBrowser(this.element, 'div').querySelector('button')!.click();
+    assert.verifySteps(['success']);
+  }
+}
+
+jitSuite(KeywordOn);
+
+/**
+ * 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)});`);
+};