Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

force importSync to always be eager #1692

Merged
merged 13 commits into from
Dec 11, 2023
16 changes: 16 additions & 0 deletions packages/compat/src/compat-adapters/ember-fetch.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
10 changes: 9 additions & 1 deletion packages/compat/src/compat-app-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}}

Expand Down
3 changes: 2 additions & 1 deletion packages/macros/src/babel/each.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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();
}
Expand Down
30 changes: 10 additions & 20 deletions packages/macros/src/babel/macros-babel-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
},
Expand Down Expand Up @@ -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()
) {
Expand Down
21 changes: 2 additions & 19 deletions packages/macros/src/babel/state.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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)[];
Expand All @@ -18,7 +15,6 @@ export default interface State {
pathToOurAddon(moduleName: string): string;
owningPackage(): Package;
originalOwningPackage(): Package;
cloneDeep(node: Node): Node;

opts: {
userConfigs: {
Expand All @@ -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();
Expand All @@ -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'));
Expand Down Expand Up @@ -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;
}
});
}
17 changes: 1 addition & 16 deletions packages/macros/src/macros-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 });
Expand Down
18 changes: 9 additions & 9 deletions packages/macros/tests/babel/import-sync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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"/);
});
});
});
4 changes: 4 additions & 0 deletions packages/webpack/src/ember-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
}

Expand Down
Loading