Repository: alangpierce/sucrase
Pull request #790 (2023-03-27 02:49:26)

Fix dynamic import with enableLegacyTypeScriptModuleInterop
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.

codecov[bot]: ## [Codecov](https://codecov.io/gh/alangpierce/sucrase/pull/790?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) Report
> Merging [#790](https://codecov.io/gh/alangpierce/sucrase/pull/790?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) (f61126c) into [main](https://codecov.io/gh/alangpierce/sucrase/commit/9081e43cc94470550570019bd434586e7331f0b9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) (9081e43) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##             main     #790   +/-   ##
=======================================
  Coverage   88.32%   88.32%           
=======================================
  Files          55       55           
  Lines        5955     5955           
  Branches     1413     1415    +2     
=======================================
  Hits         5260     5260           
  Misses        430      430           
  Partials      265      265           
```


| [Impacted Files](https://codecov.io/gh/alangpierce/sucrase/pull/790?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) | Coverage Δ | |
|---|---|---|
| [src/transformers/RootTransformer.ts](https://codecov.io/gh/alangpierce/sucrase/pull/790?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce#diff-c3JjL3RyYW5zZm9ybWVycy9Sb290VHJhbnNmb3JtZXIudHM=) | `93.68% <ø> (ø)` | |
| [src/transformers/CJSImportTransformer.ts](https://codecov.io/gh/alangpierce/sucrase/pull/790?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce#diff-c3JjL3RyYW5zZm9ybWVycy9DSlNJbXBvcnRUcmFuc2Zvcm1lci50cw==) | `89.59% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce)

github-actions[bot]: ## Benchmark results
**Before this PR:** 382.7 thousand lines per second
**After this PR:**  384.4 thousand lines per second

**Measured change:** 0.44% faster (1.04% slower to 1.27% faster)
**Summary:** Likely no significant difference

Patch:
diff --git a/src/transformers/CJSImportTransformer.ts b/src/transformers/CJSImportTransformer.ts
index afbbdc9..1d14d47 100644
--- a/src/transformers/CJSImportTransformer.ts
+++ b/src/transformers/CJSImportTransformer.ts
@@ -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,
   ) {
@@ -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.");
@@ -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;
     }
 
diff --git a/src/transformers/RootTransformer.ts b/src/transformers/RootTransformer.ts
index 850cd36..460eb07 100644
--- a/src/transformers/RootTransformer.ts
+++ b/src/transformers/RootTransformer.ts
@@ -86,6 +86,7 @@ export default class RootTransformer {
           this.helperManager,
           reactHotLoaderTransformer,
           enableLegacyBabel5ModuleInterop,
+          Boolean(options.enableLegacyTypeScriptModuleInterop),
           transforms.includes("typescript"),
           Boolean(options.preserveDynamicImport),
         ),


Test patch:
diff --git a/test/imports-test.ts b/test/imports-test.ts
index e1d76a8..f2f351d 100644
--- a/test/imports-test.ts
+++ b/test/imports-test.ts
@@ -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(
       `