Skip to content

Commit

Permalink
Fix dynamic import with enableLegacyTypeScriptModuleInterop (#790)
Browse files Browse the repository at this point in the history
In #789, I changed dynamic `import()` to use `_interopRequireWildcard`. However,
after some more testing, I realized that `enableLegacyTypeScriptModuleInterop`
mode should *not* do this, since it's an older interpretation of interop that
generally doesn't use these helpers. This PR adds that as a new special case.
  • Loading branch information
alangpierce committed Mar 27, 2023
1 parent 9081e43 commit c81e393
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
11 changes: 6 additions & 5 deletions src/transformers/CJSImportTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default class CJSImportTransformer extends Transformer {
readonly helperManager: HelperManager,
readonly reactHotLoaderTransformer: ReactHotLoaderTransformer | null,
readonly enableLegacyBabel5ModuleInterop: boolean,
readonly enableLegacyTypeScriptModuleInterop: boolean,
readonly isTypeScriptTransformEnabled: boolean,
readonly preserveDynamicImport: boolean,
) {
Expand Down Expand Up @@ -124,10 +125,10 @@ export default class CJSImportTransformer extends Transformer {
this.tokens.copyToken();
return;
}
const interopRequireWildcardName = this.helperManager.getHelperName("interopRequireWildcard");
this.tokens.replaceToken(
`Promise.resolve().then(() => ${interopRequireWildcardName}(require`,
);
const requireWrapper = this.enableLegacyTypeScriptModuleInterop
? ""
: `${this.helperManager.getHelperName("interopRequireWildcard")}(`;
this.tokens.replaceToken(`Promise.resolve().then(() => ${requireWrapper}require`);
const contextId = this.tokens.currentToken().contextId;
if (contextId == null) {
throw new Error("Expected context ID on dynamic import invocation.");
Expand All @@ -136,7 +137,7 @@ export default class CJSImportTransformer extends Transformer {
while (!this.tokens.matchesContextIdAndLabel(tt.parenR, contextId)) {
this.rootTransformer.processToken();
}
this.tokens.replaceToken(")))");
this.tokens.replaceToken(requireWrapper ? ")))" : "))");
return;
}

Expand Down
1 change: 1 addition & 0 deletions src/transformers/RootTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export default class RootTransformer {
this.helperManager,
reactHotLoaderTransformer,
enableLegacyBabel5ModuleInterop,
Boolean(options.enableLegacyTypeScriptModuleInterop),
transforms.includes("typescript"),
Boolean(options.preserveDynamicImport),
),
Expand Down
16 changes: 16 additions & 0 deletions test/imports-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,22 @@ module.exports = exports.default;
);
});

it("uses plain require for dynamic import with enableLegacyTypeScriptModuleInterop", () => {
assertResult(
`
async function loadThing() {
const foo = await import('foo');
}
`,
`"use strict";
async function loadThing() {
const foo = await Promise.resolve().then(() => require('foo'));
}
`,
{transforms: ["imports"], enableLegacyTypeScriptModuleInterop: true},
);
});

it("preserves dynamic import when configured to do so", () => {
assertResult(
`
Expand Down

0 comments on commit c81e393

Please sign in to comment.