Merge pull request #1692 from embroider-build/force-eager-import-sync

force importSync to always be eager

packages/compat/src/compat-adapters/ember-fetch.ts

@@ -0,0 +1,16 @@
+import V1Addon from '../v1-addon';
+import type { AddonMeta } from '@embroider/core';
+
+export default class extends V1Addon {
+  get packageMeta(): Partial<AddonMeta> {
+    let meta = super.packageMeta;
+
+    // this file is not accessible from the outside of ember-fetch and is not being used inside ember-fetch so it's dead code
+    // but it is importing `@ember/polyfills` which casues ember-source@5 to crash because it has been removed
+    if (meta['implicit-modules']) {
+      meta['implicit-modules'] = meta['implicit-modules'].filter(mod => mod !== './utils/mung-options-for-fetch');
+    }
+
+    return meta;
+  }
+}

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

@@ -1409,9 +1409,17 @@ let d = w.define;
 
 {{#if fastbootOnlyAmdModules}}
   if (macroCondition(getGlobalConfig().fastboot?.isRunning)) {
+    let fastbootModules = {};
+
     {{#each fastbootOnlyAmdModules as |amdModule| ~}}
-      d("{{js-string-escape amdModule.runtime}}", function(){ return i("{{js-string-escape amdModule.buildtime}}");});
+      fastbootModules["{{js-string-escape amdModule.runtime}}"] = import("{{js-string-escape amdModule.buildtime}}");
     {{/each}}
+
+    const resolvedValues = await Promise.all(Object.values(fastbootModules));
+
+    Object.keys(fastbootModules).forEach((k, i) => {
+      d(k, function(){ return resolvedValues[i];});
+    })
   }
 {{/if}}
 

packages/macros/src/babel/each.ts

@@ -4,6 +4,7 @@ import type { types as t } from '@babel/core';
 import error from './error';
 import type State from './state';
 import type * as Babel from '@babel/core';
+import cloneDeep from 'lodash/cloneDeep';
 
 type CallEachExpression = NodePath<t.CallExpression> & {
   get(callee: 'callee'): NodePath<t.Identifier>;
@@ -58,7 +59,7 @@ export function insertEach(path: EachPath, state: State, context: typeof Babel)
       for (let target of nameRefs) {
         target.replaceWith(literalElement);
       }
-      path.insertBefore(state.cloneDeep(path.get('body').node));
+      path.insertBefore(cloneDeep(path.get('body').node));
     }
     path.remove();
   }

packages/macros/src/babel/macros-babel-plugin.ts

@@ -121,25 +121,16 @@ export default function main(context: typeof Babel): unknown {
         // For example ember-auto-import needs to do some custom transforms to enable use of dynamic template strings,
         // so its babel plugin needs to see and handle the importSync call first!
         if (callee.referencesImport('@embroider/macros', 'importSync')) {
-          if (state.opts.importSyncImplementation === 'eager') {
-            let specifier = path.node.arguments[0];
-            if (specifier?.type !== 'StringLiteral') {
-              throw new Error(`importSync eager mode doesn't implement non string literal arguments yet`);
-            }
-            path.replaceWith(state.importUtil.import(path, specifier.value, '*'));
-            state.calledIdentifiers.add(callee.node);
-          } else {
-            if (path.scope.hasBinding('require')) {
-              path.scope.rename('require');
-            }
-            let r = t.identifier('require');
-            state.generatedRequires.add(r);
-            path.replaceWith(
-              t.callExpression(state.importUtil.import(path, state.pathToOurAddon('es-compat2'), 'default', 'esc'), [
-                t.callExpression(r, path.node.arguments),
-              ])
-            );
+          let specifier = path.node.arguments[0];
+          if (specifier?.type !== 'StringLiteral') {
+            throw new Error(`importSync eager mode doesn't implement non string literal arguments yet`);
           }
+          path.replaceWith(
+            t.callExpression(state.importUtil.import(path, state.pathToOurAddon('es-compat2'), 'default', 'esc'), [
+              state.importUtil.import(path, specifier.value, '*'),
+            ])
+          );
+          state.calledIdentifiers.add(callee.node);
           return;
         }
       },
@@ -187,9 +178,8 @@ export default function main(context: typeof Babel): unknown {
       }
 
       if (
-        state.opts.importSyncImplementation === 'cjs' &&
+        state.opts.hideRequires &&
         path.node.name === 'require' &&
-        !state.generatedRequires.has(path.node) &&
         !path.scope.hasBinding('require') &&
         state.owningPackage().isEmberPackage()
       ) {

packages/macros/src/babel/state.ts

@@ -1,6 +1,4 @@
 import type { NodePath, Node } from '@babel/traverse';
-import cloneDeepWith from 'lodash/cloneDeepWith';
-import lodashCloneDeep from 'lodash/cloneDeep';
 import { join, dirname, resolve } from 'path';
 import type { Package } from '@embroider/shared-internals';
 import { cleanUrl, explicitRelative, RewrittenPackageCache } from '@embroider/shared-internals';
@@ -9,7 +7,6 @@ import type * as Babel from '@babel/core';
 
 export default interface State {
   importUtil: ImportUtil;
-  generatedRequires: Set<Node>;
   removed: Set<Node>;
   calledIdentifiers: Set<Node>;
   jobs: (() => void)[];
@@ -18,7 +15,6 @@ export default interface State {
   pathToOurAddon(moduleName: string): string;
   owningPackage(): Package;
   originalOwningPackage(): Package;
-  cloneDeep(node: Node): Node;
 
   opts: {
     userConfigs: {
@@ -45,15 +41,14 @@ export default interface State {
 
     embroiderMacrosConfigMarker: true;
 
-    mode: 'compile-time' | 'run-time';
+    hideRequires: boolean;
 
-    importSyncImplementation: 'cjs' | 'eager';
+    mode: 'compile-time' | 'run-time';
   };
 }
 
 export function initState(t: typeof Babel.types, path: NodePath<Babel.types.Program>, state: State) {
   state.importUtil = new ImportUtil(t, path);
-  state.generatedRequires = new Set();
   state.jobs = [];
   state.removed = new Set();
   state.calledIdentifiers = new Set();
@@ -62,7 +57,6 @@ export function initState(t: typeof Babel.types, path: NodePath<Babel.types.Prog
   state.pathToOurAddon = pathToAddon;
   state.owningPackage = owningPackage;
   state.originalOwningPackage = originalOwningPackage;
-  state.cloneDeep = cloneDeep;
 }
 
 const runtimeAddonPath = resolve(join(__dirname, '..', 'addon'));
@@ -96,14 +90,3 @@ function originalOwningPackage(this: State): Package {
   let pkg = this.owningPackage();
   return this.packageCache.original(pkg);
 }
-
-function cloneDeep(this: State, node: Node): Node {
-  let state = this;
-  return cloneDeepWith(node, function (value: any) {
-    if (state.generatedRequires.has(value)) {
-      let cloned = lodashCloneDeep(value);
-      state.generatedRequires.add(cloned);
-      return cloned;
-    }
-  });
-}

packages/macros/src/macros-config.ts

@@ -135,21 +135,6 @@ export default class MacrosConfig {
     }
   }
 
-  private _importSyncImplementation: 'cjs' | 'eager' = 'cjs';
-
-  get importSyncImplementation() {
-    return this._importSyncImplementation;
-  }
-
-  set importSyncImplementation(value: 'cjs' | 'eager') {
-    if (!this._configWritable) {
-      throw new Error(
-        `[Embroider:MacrosConfig] attempted to set importSyncImplementation after configs have been finalized`
-      );
-    }
-    this._importSyncImplementation = value;
-  }
-
   private constructor(private origAppRoot: string) {
     // this uses globalConfig because these things truly are global. Even if a
     // package doesn't have a dep or peerDep on @embroider/macros, it's legit
@@ -343,7 +328,7 @@ export default class MacrosConfig {
         return self.mode;
       },
 
-      importSyncImplementation: this.importSyncImplementation,
+      hideRequires: true,
     };
 
     let lockFilePath = findUp.sync(['yarn.lock', 'package-lock.json', 'pnpm-lock.yaml'], { cwd: self.appRoot });

packages/macros/tests/babel/import-sync.test.ts

@@ -6,13 +6,13 @@ describe('importSync', function () {
     config.setOwnConfig(__filename, { target: 'my-plugin' });
     config.finalize();
 
-    test('importSync becomes esc(require())', () => {
+    test('importSync becomes import * as _something', () => {
       let code = transform(`
       import { importSync } from '@embroider/macros';
       importSync('foo');
       `);
-      expect(code).toMatch(/import esc from "\.\.\/\.\.\/src\/addon\/es-compat2"/);
-      expect(code).toMatch(/esc\(require\(['"]foo['"]\)\)/);
+      expect(code).toMatch(/import \* as _importSync\d from "foo"/);
+      expect(code).toMatch(/esc\(_importSync\d\);/);
       expect(code).not.toMatch(/window/);
     });
     test('importSync leaves existing binding for require alone', () => {
@@ -22,16 +22,16 @@ describe('importSync', function () {
       importSync('foo');
       require('x');
       `);
-      expect(code).toMatch(/esc\(require\(['"]foo['"]\)\)/);
-      expect(code).toMatch(/import _require from 'require'/);
-      expect(code).toMatch(/_require\(['"]x['"]\)/);
+      expect(code).toMatch(/import \* as _importSync\d from "foo"/);
+      expect(code).toMatch(/import require from 'require'/);
+      expect(code).toMatch(/require\(['"]x['"]\)/);
     });
-    test('aliased importSync becomes require', () => {
+    test('aliased importSync becomes aliased variable', () => {
       let code = transform(`
       import { importSync as i } from '@embroider/macros';
       i('foo');
       `);
-      expect(code).toMatch(/require\(['"]foo['"]\)/);
+      expect(code).toMatch(/import \* as _i\d from "foo"/);
       expect(code).not.toMatch(/window/);
     });
     test('import of importSync itself gets removed', () => {
@@ -51,7 +51,7 @@ describe('importSync', function () {
       import { importSync, getOwnConfig } from '@embroider/macros';
       importSync(getOwnConfig().target);
       `);
-      expect(code).toMatch(/require\(['"]my-plugin['"]\)/);
+      expect(code).toMatch(/import \* as _importSync\d from "my-plugin"/);
     });
   });
 });

packages/webpack/src/ember-webpack.ts

@@ -285,6 +285,10 @@ const Webpack: PackagerConstructor<Options> = class Webpack implements Packager
           'style-loader': require.resolve('style-loader'),
         },
       },
+      experiments: {
+        // this is needed because fasboot-only modules need to use await import()
+        topLevelAwait: true,
+      },
     };
   }