workaround for ember helper invocation bug

I thought we could take first-class helper support for granted since it was supposed to be introduced before 3.28 (our oldest supported ember), but due to: emberjs/ember.js#19878 we can't rely on it until ember 4.2.
embroider-build · Nov 16, 2022 · 83a17e3 · 83a17e3
1 parent 5a26466
commit 83a17e3
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 23 deletions.
diff --git a/packages/compat/src/compat-app.ts b/packages/compat/src/compat-app.ts
@@ -325,6 +325,7 @@ class CompatAppAdapter implements AppAdapter<TreeNames> {
   @Memoize()
   templateResolver(): Resolver {
     return new CompatResolver({
+      emberVersion: this.activeAddonChildren().find(a => a.name === 'ember-source')!.packageJSON.version,
       root: this.root,
       modulePrefix: this.modulePrefix(),
       podModulePrefix: this.podModulePrefix(),
@@ -381,6 +382,7 @@ class CompatAppAdapter implements AppAdapter<TreeNames> {
   @Memoize()
   private internalTemplateResolver(): CompatResolver {
     return new CompatResolver({
+      emberVersion: this.activeAddonChildren().find(a => a.name === 'ember-source')!.packageJSON.version,
       root: this.root,
       modulePrefix: this.modulePrefix(),
       options: this.options,

diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts
@@ -2,11 +2,17 @@ import { default as Resolver, ComponentResolution, ComponentLocator, ResolutionF
 import type { ASTv1, ASTPluginBuilder, ASTPluginEnvironment, WalkerPath } from '@glimmer/syntax';
 import type { WithJSUtils } from 'babel-plugin-ember-template-compilation';
 import assertNever from 'assert-never';
+import { ResolvedDep } from '@embroider/core/src/resolver';
 
 type Env = WithJSUtils<ASTPluginEnvironment> & { filename: string; contents: string };
 
+export interface Options {
+  resolver: Resolver;
+  patchHelpersBug: boolean;
+}
+
 // This is the AST transform that resolves components, helpers and modifiers at build time
-export default function makeResolverTransform(resolver: Resolver) {
+export default function makeResolverTransform({ resolver, patchHelpersBug }: Options) {
   const resolverTransform: ASTPluginBuilder<Env> = ({
     filename,
     contents,
@@ -20,17 +26,15 @@ export default function makeResolverTransform(resolver: Resolver) {
       resolver.reportError(err, filename, contents);
     }
 
-    function emitAMD(resolution: ComponentResolution) {
-      for (let m of [resolution.hbsModule, resolution.jsModule]) {
-        if (m && !emittedAMDDeps.has(m.runtimeName)) {
-          let parts = m.runtimeName.split('/');
-          let { path, runtimeName } = m;
-          jsutils.emitExpression(context => {
-            let identifier = context.import(path, 'default', parts[parts.length - 1]);
-            return `window.define("${runtimeName}", () => ${identifier})`;
-          });
-          emittedAMDDeps.add(m.runtimeName);
-        }
+    function emitAMD(dep: ResolvedDep | null) {
+      if (dep && !emittedAMDDeps.has(dep.runtimeName)) {
+        let parts = dep.runtimeName.split('/');
+        let { path, runtimeName } = dep;
+        jsutils.emitExpression(context => {
+          let identifier = context.import(path, 'default', parts[parts.length - 1]);
+          return `window.define("${runtimeName}", () => ${identifier})`;
+        });
+        emittedAMDDeps.add(dep.runtimeName);
       }
     }
 
@@ -44,6 +48,18 @@ export default function makeResolverTransform(resolver: Resolver) {
           reportError(resolution);
           return;
         case 'helper':
+          if (patchHelpersBug) {
+            // lexical invocation of helpers was not reliable before Ember 4.2 due to https://github.com/emberjs/ember.js/pull/19878
+            emitAMD(resolution.module);
+          } else {
+            setter(
+              parentPath.node,
+              builders.path(
+                jsutils.bindImport(resolution.module.path, 'default', parentPath, { nameHint: resolution.nameHint })
+              )
+            );
+          }
+          return;
         case 'modifier':
           setter(
             parentPath.node,
@@ -71,7 +87,8 @@ export default function makeResolverTransform(resolver: Resolver) {
               )
             );
           } else {
-            emitAMD(resolution);
+            emitAMD(resolution.jsModule);
+            emitAMD(resolution.hbsModule);
           }
         case undefined:
           return;

diff --git a/packages/compat/src/resolver.ts b/packages/compat/src/resolver.ts
@@ -16,6 +16,8 @@ import { ResolvedDep } from '@embroider/core/src/resolver';
 import { dasherize, snippetToDasherizedName } from './dasherize-component-name';
 import { pathExistsSync } from 'fs-extra';
 import resolve from 'resolve';
+import semver from 'semver';
+import { Options as ResolverTransformOptions } from './resolver-transform';
 
 export interface ComponentResolution {
   type: 'component';
@@ -130,6 +132,7 @@ interface RehydrationParamsBase {
   modulePrefix: string;
   podModulePrefix?: string;
   options: ResolverOptions;
+  emberVersion: string;
   activePackageRules: ActivePackageRules[];
 }
 
@@ -305,7 +308,14 @@ export default class CompatResolver implements Resolver {
 
   astTransformer(): undefined | string | [string, unknown] {
     if (this.staticComponentsEnabled || this.staticHelpersEnabled || this.staticModifiersEnabled) {
-      return [require.resolve('./resolver-transform'), this];
+      let opts: ResolverTransformOptions = {
+        resolver: this,
+        // lexical invocation of helpers was not reliable before Ember 4.2 due to https://github.com/emberjs/ember.js/pull/19878
+        patchHelpersBug: semver.satisfies(this.params.emberVersion, '<4.2.0-beta.0', {
+          includePrerelease: true,
+        }),
+      };
+      return [require.resolve('./resolver-transform'), opts];
     }
   }
 

diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts
@@ -1,4 +1,4 @@
-import { emberTemplateCompilerPath } from '@embroider/test-support';
+import { emberTemplateCompiler } from '@embroider/test-support';
 import { Project } from 'scenario-tester';
 import { AppMeta, throwOnWarnings } from '@embroider/core';
 import merge from 'lodash/merge';
@@ -26,6 +26,7 @@ describe('audit', function () {
     const resolvableExtensions = ['.js', '.hbs'];
 
     let resolver = new CompatResolver({
+      emberVersion: emberTemplateCompiler().version,
       root: app.baseDir,
       modulePrefix: 'audit-this-app',
       options: {
@@ -58,7 +59,7 @@ describe('audit', function () {
     }
 
     let etcOptions: InlinePrecompileOptions = {
-      compilerPath: emberTemplateCompilerPath(),
+      compilerPath: emberTemplateCompiler().path,
       transforms: [transform],
       enableLegacyModules: ['ember-cli-htmlbars'],
     };

diff --git a/packages/compat/tests/resolver.test.ts b/packages/compat/tests/resolver.test.ts
@@ -3,7 +3,7 @@ import { join, dirname } from 'path';
 import Options, { optionsWithDefaults } from '../src/options';
 import { hbsToJS, tmpdir } from '@embroider/shared-internals';
 import { throwOnWarnings } from '@embroider/core';
-import { emberTemplateCompilerPath } from '@embroider/test-support';
+import { emberTemplateCompiler } from '@embroider/test-support';
 import { Options as AdjustImportsOptions } from '@embroider/core/src/babel-plugin-adjust-imports';
 import Resolver from '../src/resolver';
 import { PackageRules } from '../src';
@@ -26,6 +26,7 @@ describe('compat-resolver', function () {
     appDir = realpathSync(mkdtempSync(join(tmpdir, 'embroider-compat-tests-')));
     writeJSONSync(join(appDir, 'package.json'), { name: 'the-app' });
     let resolver = new Resolver({
+      emberVersion: emberTemplateCompiler().version,
       root: appDir,
       modulePrefix: 'the-app',
       podModulePrefix: otherOptions.podModulePrefix,
@@ -43,7 +44,6 @@ describe('compat-resolver', function () {
           activeAddons: {},
           relocatedFiles: {},
           resolvableExtensions: ['.js', '.hbs'],
-          emberNeedsModulesPolyfill: false,
           appRoot: appDir,
         },
         otherOptions.adjustImportsImports
@@ -56,7 +56,7 @@ describe('compat-resolver', function () {
       transforms.push(resolverTransform);
     }
     let etcOptions: EtcOptions = {
-      compilerPath: emberTemplateCompilerPath(),
+      compilerPath: emberTemplateCompiler().path,
       transforms,
       targetFormat: 'hbs',
     };

diff --git a/packages/macros/tests/glimmer/helpers.ts b/packages/macros/tests/glimmer/helpers.ts
@@ -1,12 +1,12 @@
-import { emberTemplateCompilerPath } from '@embroider/test-support';
+import { emberTemplateCompiler } from '@embroider/test-support';
 import { Project } from 'scenario-tester';
 import { MacrosConfig } from '../../src/node';
 import { join } from 'path';
 import { hbsToJS } from '@embroider/shared-internals';
 import { transformSync } from '@babel/core';
 import { Options as EtcOptions, Transform } from 'babel-plugin-ember-template-compilation';
 
-const compilerPath = emberTemplateCompilerPath();
+const compilerPath = emberTemplateCompiler().path;
 
 export { Project };
 

diff --git a/test-packages/support/index.ts b/test-packages/support/index.ts
@@ -95,8 +95,11 @@ export function allBabelVersions(params: {
   }
 }
 
-export function emberTemplateCompilerPath() {
-  return join(__dirname, 'vendor', 'ember-template-compiler.js');
+export function emberTemplateCompiler() {
+  return {
+    path: join(__dirname, 'vendor', 'ember-template-compiler.js'),
+    version: '4.1.0',
+  };
 }
 
 export function definesPattern(runtimeName: string, buildTimeName: string): RegExp {