Merge pull request #2005 from embroider-build/bugfix-unique-id-versio…

…n-import

unique-id helper import based on ember-source version

packages/compat/src/compat-app-builder.ts

@@ -694,6 +694,14 @@ export class CompatAppBuilder {
     return this.activeAddonChildren(this.appPackageWithMovedDeps).find(a => a.name === 'ember-cli-fastboot');
   }
 
+  private emberVersion() {
+    let pkg = this.activeAddonChildren(this.appPackageWithMovedDeps).find(a => a.name === 'ember-source');
+    if (!pkg) {
+      throw new Error('no ember version!');
+    }
+    return pkg.version;
+  }
+
   @Memoize()
   private get fastbootConfig():
     | { packageJSON: PackageInfo; extraAppFiles: string[]; extraVendorFiles: string[] }
@@ -1021,6 +1029,7 @@ export class CompatAppBuilder {
     ) {
       let opts: ResolverTransformOptions = {
         appRoot: resolverConfig.appRoot,
+        emberVersion: this.emberVersion(),
       };
       transforms.push([require.resolve('./resolver-transform'), opts]);
     }

packages/compat/src/resolver-transform.ts

@@ -18,6 +18,7 @@ import { Resolver, locateEmbroiderWorkingDir } from '@embroider/core';
 import type CompatOptions from './options';
 import type { AuditMessage, Loc } from './audit';
 import { camelCase, mergeWith } from 'lodash';
+import { satisfies } from 'semver';
 
 type Env = WithJSUtils<ASTPluginEnvironment> & {
   filename: string;
@@ -41,6 +42,7 @@ export interface CompatResolverOptions extends CoreResolverOptions {
 
 export interface Options {
   appRoot: string;
+  emberVersion: string;
 }
 
 type BuiltIn = {
@@ -49,67 +51,73 @@ type BuiltIn = {
   importableModifier?: [string, string];
 };
 
-const builtInKeywords: Record<string, BuiltIn | undefined> = {
-  '-get-dynamic-var': {},
-  '-in-element': {},
-  '-with-dynamic-vars': {},
-  action: {},
-  array: {
-    importableHelper: ['array', '@ember/helper'],
-  },
-  component: {},
-  concat: {
-    importableHelper: ['concat', '@ember/helper'],
-  },
-  debugger: {},
-  'each-in': {},
-  each: {},
-  fn: {
-    importableHelper: ['fn', '@ember/helper'],
-  },
-  get: {
-    importableHelper: ['get', '@ember/helper'],
-  },
-  'has-block-params': {},
-  'has-block': {},
-  hasBlock: {},
-  hasBlockParams: {},
-  hash: {
-    importableHelper: ['hash', '@ember/helper'],
-  },
-  helper: {},
-  if: {},
-  'in-element': {},
-  input: {
-    importableComponent: ['Input', '@ember/component'],
-  },
-  let: {},
-  'link-to': {
-    importableComponent: ['LinkTo', '@ember/routing'],
-  },
-  loc: {},
-  log: {},
-  modifier: {},
-  mount: {},
-  mut: {},
-  on: {
-    importableModifier: ['on', '@ember/modifier'],
-  },
-  outlet: {},
-  partial: {},
-  'query-params': {},
-  readonly: {},
-  textarea: {
-    importableComponent: ['Textarea', '@ember/component'],
-  },
-  unbound: {},
-  'unique-id': {
-    importableHelper: ['uniqueId', '@ember/helper'],
-  },
-  unless: {},
-  with: {},
-  yield: {},
-};
+function builtInKeywords(emberVersion: string): Record<string, BuiltIn | undefined> {
+  const builtInKeywords: Record<string, BuiltIn | undefined> = {
+    '-get-dynamic-var': {},
+    '-in-element': {},
+    '-with-dynamic-vars': {},
+    action: {},
+    array: {
+      importableHelper: ['array', '@ember/helper'],
+    },
+    component: {},
+    concat: {
+      importableHelper: ['concat', '@ember/helper'],
+    },
+    debugger: {},
+    'each-in': {},
+    each: {},
+    fn: {
+      importableHelper: ['fn', '@ember/helper'],
+    },
+    get: {
+      importableHelper: ['get', '@ember/helper'],
+    },
+    'has-block-params': {},
+    'has-block': {},
+    hasBlock: {},
+    hasBlockParams: {},
+    hash: {
+      importableHelper: ['hash', '@ember/helper'],
+    },
+    helper: {},
+    if: {},
+    'in-element': {},
+    input: {
+      importableComponent: ['Input', '@ember/component'],
+    },
+    let: {},
+    'link-to': {
+      importableComponent: ['LinkTo', '@ember/routing'],
+    },
+    loc: {},
+    log: {},
+    modifier: {},
+    mount: {},
+    mut: {},
+    on: {
+      importableModifier: ['on', '@ember/modifier'],
+    },
+    outlet: {},
+    partial: {},
+    'query-params': {},
+    readonly: {},
+    textarea: {
+      importableComponent: ['Textarea', '@ember/component'],
+    },
+    unbound: {},
+    'unique-id': {},
+    unless: {},
+    with: {},
+    yield: {},
+  };
+  if (satisfies(emberVersion, '>=5.2')) {
+    builtInKeywords['unique-id'] = {
+      importableHelper: ['uniqueId', '@ember/helper'],
+    };
+  }
+  return builtInKeywords;
+}
 
 interface ComponentResolution {
   type: 'component';
@@ -167,7 +175,11 @@ class TemplateResolver implements ASTPlugin {
 
   private moduleResolver: Resolver;
 
-  constructor(private env: Env, private config: CompatResolverOptions) {
+  constructor(
+    private env: Env,
+    private config: CompatResolverOptions,
+    private builtInsForEmberVersion: ReturnType<typeof builtInKeywords>
+  ) {
     this.moduleResolver = new Resolver(config);
     if ((globalThis as any).embroider_audit) {
       this.auditHandler = (globalThis as any).embroider_audit;
@@ -399,7 +411,7 @@ class TemplateResolver implements ASTPlugin {
       return null;
     }
 
-    const builtIn = builtInKeywords[name];
+    const builtIn = this.builtInsForEmberVersion[name];
 
     if (builtIn?.importableComponent) {
       let [importedName, specifier] = builtIn.importableComponent;
@@ -482,7 +494,7 @@ class TemplateResolver implements ASTPlugin {
     // globally-named helpers. It throws an error. So it's fine for us to
     // prioritize the builtIns here without bothering to resolve a user helper
     // of the same name.
-    const builtIn = builtInKeywords[path];
+    const builtIn = this.builtInsForEmberVersion[path];
 
     if (builtIn?.importableHelper) {
       let [importedName, specifier] = builtIn.importableHelper;
@@ -567,7 +579,7 @@ class TemplateResolver implements ASTPlugin {
       return null;
     }
 
-    let builtIn = builtInKeywords[path];
+    let builtIn = this.builtInsForEmberVersion[path];
 
     if (builtIn?.importableComponent) {
       let [importedName, specifier] = builtIn.importableComponent;
@@ -656,7 +668,7 @@ class TemplateResolver implements ASTPlugin {
       return null;
     }
 
-    const builtIn = builtInKeywords[path];
+    const builtIn = this.builtInsForEmberVersion[path];
     if (builtIn?.importableModifier) {
       let [importedName, specifier] = builtIn.importableModifier;
       return {
@@ -959,7 +971,7 @@ class TemplateResolver implements ASTPlugin {
 }
 
 // This is the AST transform that resolves components, helpers and modifiers at build time
-export default function makeResolverTransform({ appRoot }: Options) {
+export default function makeResolverTransform({ appRoot, emberVersion }: Options) {
   let config: CompatResolverOptions = readJSONSync(join(locateEmbroiderWorkingDir(appRoot), 'resolver.json'));
   const resolverTransform: ASTPluginBuilder<Env> = env => {
     if (env.strictMode) {
@@ -968,7 +980,7 @@ export default function makeResolverTransform({ appRoot }: Options) {
         visitor: {},
       };
     }
-    return new TemplateResolver(env, config);
+    return new TemplateResolver(env, config, builtInKeywords(emberVersion));
   };
   (resolverTransform as any).parallelBabel = {
     requireFile: __filename,

packages/compat/tests/audit.test.ts

@@ -61,6 +61,7 @@ describe('audit', function () {
 
     let transformOpts: ResolverTransformOptions = {
       appRoot: resolverConfig.appRoot,
+      emberVersion: '*', // since no packages are declared ember version can be anything so * is valid
     };
     let transform: Transform = [require.resolve('../src/resolver-transform'), transformOpts];
 

tests/scenarios/compat-resolver-test.ts

@@ -41,7 +41,11 @@ Scenarios.fromProject(() => new Project())
     Qmodule(scenario.name, function (hooks) {
       let expectTranspiled: (file: string) => ReturnType<ReturnType<ExpectFile>['transform']>;
       let givenFiles: (files: Record<string, string>) => void;
-      let configure: (opts?: Partial<CompatResolverOptions['options']>, extraOpts?: ConfigureOpts) => Promise<void>;
+      let configure: (
+        opts?: Partial<CompatResolverOptions['options']>,
+        extraOpts?: ConfigureOpts,
+        emberVersion?: string
+      ) => Promise<void>;
 
       interface ConfigureOpts {
         appPackageRules?: Partial<PackageRules>;
@@ -59,13 +63,17 @@ Scenarios.fromProject(() => new Project())
             outputFileSync(resolve(app.dir, filename), contents, 'utf8');
           }
         };
-        configure = async function (opts?: Partial<CompatResolverOptions['options']>, extraOpts?: ConfigureOpts) {
+        configure = async function (
+          opts?: Partial<CompatResolverOptions['options']>,
+          extraOpts?: ConfigureOpts,
+          emberVersion = '4.6.0' //based on app-template package.json
+        ) {
           let etcOptions: EtcOptions = {
             compilerPath: require.resolve('ember-source-latest/dist/ember-template-compiler'),
             targetFormat: 'hbs',
             transforms: [
               ...(extraOpts?.astPlugins ?? []),
-              [require.resolve('@embroider/compat/src/resolver-transform'), { appRoot: app.dir }],
+              [require.resolve('@embroider/compat/src/resolver-transform'), { appRoot: app.dir, emberVersion }],
             ],
           };
 
@@ -1140,6 +1148,36 @@ Scenarios.fromProject(() => new Project())
       `);
       });
 
+      test('built-in helper unique-id is not imported when used with ember source version <5.2', async function () {
+        givenFiles({
+          'templates/application.hbs': `{{(unique-id)}}`,
+        });
+        await configure({ staticHelpers: true }, undefined, '4.6.0');
+        expectTranspiled('templates/application.hbs').equalsCode(`
+        import { precompileTemplate } from "@ember/template-compilation";
+        export default precompileTemplate("{{(unique-id)}}", {
+          moduleName: "my-app/templates/application.hbs",
+        });
+      `);
+      });
+
+      test('built-in helper unique-id is imported when used with ember source version >=5.2', async function () {
+        givenFiles({
+          'templates/application.hbs': `{{(unique-id)}}`,
+        });
+        await configure({ staticHelpers: true }, undefined, '5.2.0');
+        expectTranspiled('templates/application.hbs').equalsCode(`
+        import { precompileTemplate } from "@ember/template-compilation";
+        import { uniqueId } from "@ember/helper";
+        export default precompileTemplate("{{(uniqueId)}}", {
+          moduleName: "my-app/templates/application.hbs",
+          scope: () => ({
+            uniqueId,
+          }),
+        });
+      `);
+      });
+
       test('built-in modifiers are ignored when used with the modifier keyword', async function () {
         givenFiles({
           'templates/application.hbs': `{{modifier "on"}}{{modifier "action"}}`,