[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.

(cherry picked from commit 397d6b9)

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,39 @@
+/**
+@module ember
+*/
+import { Owner } from '@ember/-internals/owner';
+import { assert } from '@ember/debug';
+import { DEBUG } from '@glimmer/env';
+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));
+
+  let 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);
+
+  if (DEBUG) {
+    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,27 @@ 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;
+} else {
+  // Bug: this may be a quirk of our test setup?
+  // In prod builds, this is a no-op helper and is unused in practice. We shouldn't need
+  // to add it at all, but the current test build doesn't produce a "prod compiler", so
+  // we ended up running the debug-build for the template compliler in prod tests. Once
+  // that is fixed, this can be removed. For now, this allows the test to work and does
+  // not really harm anything, since it's just a no-op pass-through helper and the bytes
+  // has to be included anyway. In the future, perhaps we can avoid the latter by using
+  // `import(...)`?
+  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,45 @@ 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'(assert) {
+      this.registerModifier(
+        'replace',
+        defineSimpleModifier((element) => (element.innerHTML = 'Hello, world!'))
+      );
+
+      if (DEBUG) {
+        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\./
+        );
+      } else {
+        assert.expect(0);
+      }
+    }
+
     '@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,27 @@ 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'(assert) {
+      this.registerHelper('hello-world', () => 'Hello, world!');
+
+      if (DEBUG) {
+        expectAssertion(
+          () => this.render('{{helper this.name}}', { name: 'hello-world' }),
+          /Passing a dynamic string to the `\(helper\)` keyword is disallowed\./
+        );
+      } else {
+        assert.expect(0);
+      }
+    }
+
     '@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)
 );