[BUGFIX beta] Allow passing a string literal to {{helper}} and {{modi…

…fier}}

As per [RFC #432](https://github.com/emberjs/rfcs/blob/master/text/0432-contextual-helpers.md#invoking-contextual-modifiers)

However, dynamic string values are currently disallowed per earlier
framework team weekly meeting discussion to not make it harder for
Embroider to heuristically analyze the dynamic usages.

Eventually we will want to do the same for components as well, and
this AST transform would work there too.

packages/@ember/-internals/glimmer/lib/helpers/-disallow-dynamic-resolution.ts

@@ -0,0 +1,57 @@
+/**
+@module ember
+*/
+import { assert } from '@ember/debug';
+import { CapturedArguments } from '@glimmer/interfaces';
+import { createComputeRef, valueForRef } from '@glimmer/reference';
+import { internalHelper } from './internal-helper';
+
+export default internalHelper(({ positional, named }: CapturedArguments) => {
+  assert(
+    `[BUG] wrong number of positional arguments, expecting 1, got ${positional.length}`,
+    positional.length === 1
+  );
+
+  let nameOrValueRef = positional[0];
+
+  assert(`[BUG] expecting \`type\` named argument`, 'type' in named);
+  assert(`[BUG] expecting \`loc\` named argument`, 'loc' in named);
+  assert(`[BUG] expecting \`original\` named argument`, 'original' in named);
+
+  let typeRef = named.type;
+  let locRef = named.loc;
+  let originalRef = named.original;
+
+  // Bug: why do these fail?
+  // assert('[BUG] expecting a string literal for the `type` argument', isConstRef(typeRef));
+  // assert('[BUG] expecting a string literal for the `loc` argument', isConstRef(locRef));
+  // assert('[BUG] expecting a string literal for the `original` argument', isConstRef(originalRef));
+
+  const type = valueForRef(typeRef);
+  const loc = valueForRef(locRef);
+  const original = valueForRef(originalRef);
+
+  assert('[BUG] expecting a string literal for the `type` argument', typeof type === 'string');
+  assert('[BUG] expecting a string literal for the `loc` argument', typeof loc === 'string');
+  assert(
+    '[BUG] expecting a string literal for the `original` argument',
+    typeof original === 'string'
+  );
+
+  return createComputeRef(() => {
+    let nameOrValue = valueForRef(nameOrValueRef);
+
+    assert(
+      `Passing a dynamic string to the \`(${type})\` keyword is disallowed. ` +
+        `(You specified \`(${type} ${original})\` and \`${original}\` evaluated into "${nameOrValue}".) ` +
+        `This ensures we can statically analyze the template and determine which ${type}s are used. ` +
+        `If the ${type} name is always the same, use a string literal instead, i.e. \`(${type} "${nameOrValue}")\`. ` +
+        `Otherwise, import the ${type}s into JavaScript and pass them to the ${type} keyword. ` +
+        'See https://github.com/emberjs/rfcs/blob/master/text/0496-handlebars-strict-mode.md#4-no-dynamic-resolution for details. ' +
+        loc,
+      typeof nameOrValue !== 'string'
+    );
+
+    return nameOrValue;
+  });
+});

packages/@ember/-internals/glimmer/lib/helpers/-resolve.ts

@@ -0,0 +1,38 @@
+/**
+@module ember
+*/
+import { Owner } from '@ember/-internals/owner';
+import { assert, runInDebug } from '@ember/debug';
+import { CapturedArguments } from '@glimmer/interfaces';
+import { createConstRef, isConstRef, valueForRef } from '@glimmer/reference';
+import { internalHelper } from './internal-helper';
+
+export default internalHelper(({ positional }: CapturedArguments, owner: Owner | undefined) => {
+  // why is this allowed to be undefined in the first place?
+  assert('[BUG] missing owner', owner);
+
+  assert(
+    `[BUG] wrong number of positional arguments, expecting 1, got ${positional.length}`,
+    positional.length === 1
+  );
+
+  let fullNameRef = positional[0];
+
+  assert('[BUG] expecting a string literal as argument', isConstRef(fullNameRef));
+
+  const fullName = valueForRef(fullNameRef);
+
+  assert('[BUG] expecting a string literal as argument', typeof fullName === 'string');
+  assert('[BUG] expecting a valid full name', fullName.split(':').length === 2);
+
+  runInDebug(() => {
+    let [type, name] = fullName.split(':');
+
+    assert(
+      `Attempted to invoke \`(-resolve "${fullName}")\`, but ${name} was not a valid ${type} name.`,
+      owner.hasRegistration(fullName)
+    );
+  });
+
+  return createConstRef(owner.factoryFor(fullName)?.class, `(-resolve "${fullName}")`);
+});

packages/@ember/-internals/glimmer/lib/resolver.ts

@@ -42,8 +42,10 @@ import {
   isClassicHelper,
   SimpleHelper,
 } from './helper';
+import { default as disallowDynamicResolution } from './helpers/-disallow-dynamic-resolution';
 import { default as inElementNullCheckHelper } from './helpers/-in-element-null-check';
 import { default as normalizeClassHelper } from './helpers/-normalize-class';
+import { default as resolve } from './helpers/-resolve';
 import { default as trackArray } from './helpers/-track-array';
 import { default as action } from './helpers/action';
 import { default as eachIn } from './helpers/each-in';
@@ -181,12 +183,17 @@ const BUILTIN_KEYWORD_HELPERS = {
   '-hash': hash,
   '-each-in': eachIn,
   '-normalize-class': normalizeClassHelper,
+  '-resolve': resolve,
   '-track-array': trackArray,
   '-mount': mountHelper,
   '-outlet': outletHelper,
   '-in-el-null': inElementNullCheckHelper,
 };
 
+if (DEBUG) {
+  BUILTIN_KEYWORD_HELPERS['-disallow-dynamic-resolution'] = disallowDynamicResolution;
+}
+
 const BUILTIN_HELPERS = {
   ...BUILTIN_KEYWORD_HELPERS,
   array,

packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js

@@ -581,6 +581,41 @@ moduleFor(
       assert.equal(updateCount, 1);
     }
 
+    '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can resolve a modifier'() {
+      this.registerModifier(
+        'replace',
+        defineSimpleModifier((element, [text]) => (element.innerHTML = text ?? 'Hello, world!'))
+      );
+
+      // BUG: this should work according to the RFC
+      // this.render(
+      //   '[<div {{modifier "replace"}}>Nope</div>][<div {{modifier (modifier "replace") "wow"}}>Nope</div>]'
+      // );
+      this.render(
+        '[<div {{(modifier "replace")}}>Nope</div>][<div {{(modifier "replace") "wow"}}>Nope</div>]'
+      );
+      this.assertText('[Hello, world!][wow]');
+      this.assertStableRerender();
+    }
+
+    '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Cannot dynamically resolve a modifier'() {
+      this.registerModifier(
+        'replace',
+        defineSimpleModifier((element) => (element.innerHTML = 'Hello, world!'))
+      );
+
+      expectAssertion(
+        () =>
+          this.render(
+            // BUG: this should work according to the RFC
+            // '<div {{modifier this.name}}>Nope</div>',
+            '<div {{(modifier this.name)}}>Nope</div>',
+            { name: 'replace' }
+          ),
+        /Passing a dynamic string to the `\(modifier\)` keyword is disallowed\./
+      );
+    }
+
     '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can be curried'() {
       let val = defineSimpleModifier((element, [text]) => (element.innerHTML = text));
 

packages/@ember/-internals/glimmer/tests/integration/helpers/custom-helper-test.js

@@ -780,6 +780,23 @@ moduleFor(
       }, expectedMessage);
     }
 
+    '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can resolve a helper'() {
+      this.registerHelper('hello-world', ([text]) => text ?? 'Hello, world!');
+
+      this.render('[{{helper "hello-world"}}][{{helper (helper "hello-world") "wow"}}]');
+      this.assertText('[Hello, world!][wow]');
+      this.assertStableRerender();
+    }
+
+    '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Cannot dynamically resolve a helper'() {
+      this.registerHelper('hello-world', () => 'Hello, world!');
+
+      expectAssertion(
+        () => this.render('{{helper this.name}}', { name: 'hello-world' }),
+        /Passing a dynamic string to the `\(helper\)` keyword is disallowed\./
+      );
+    }
+
     '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can use a curried dynamic helper'() {
       let val = defineSimpleHelper((value) => value);
 

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

@@ -14,6 +14,7 @@ import TransformInElement from './transform-in-element';
 import TransformLinkTo from './transform-link-to';
 import TransformOldClassBindingSyntax from './transform-old-class-binding-syntax';
 import TransformQuotedBindingsIntoJustBindings from './transform-quoted-bindings-into-just-bindings';
+import TransformResolutions from './transform-resolutions';
 import TransformWrapMountAndOutlet from './transform-wrap-mount-and-outlet';
 
 import { EMBER_DYNAMIC_HELPERS_AND_MODIFIERS, EMBER_NAMED_BLOCKS } from '@ember/canary-features';
@@ -38,7 +39,9 @@ export const RESOLUTION_MODE_TRANSFORMS = Object.freeze(
     DeprecateWith,
     SEND_ACTION ? DeprecateSendAction : null,
     !EMBER_NAMED_BLOCKS ? AssertAgainstNamedBlocks : null,
-    !EMBER_DYNAMIC_HELPERS_AND_MODIFIERS ? AssertAgainstDynamicHelpersModifiers : null,
+    EMBER_DYNAMIC_HELPERS_AND_MODIFIERS
+      ? TransformResolutions
+      : AssertAgainstDynamicHelpersModifiers,
   ].filter(notNull)
 );