Repository: alangpierce/sucrase
Pull request #727 (2022-07-25 07:02:05)

Add flag to preserve dynamic import
Progress toward #726

This PR adds a new `preserveDynamicImport` flag that, when enabled, changes the
`imports` transform so that only static `import`s are transformed, while dynamic
`import()` expressions are passed through as-is.

This behavior matches what TypeScript does when handling a `.cjs` file when
`module: nodenext` is specified, and ts-node expects transpilers to handle this
mode, so this PR is a necessary step toward Sucrase as a ts-node plugin.

This mode also seems like the best interpretation of `import()` in new code
these days; if `import()` is transpiled, then it's impossible to access ESM
code from CJS in Node, so leaving it alone allows a mechanism to import
ESM-only libraries. My tentative plan is to change this mode to be the default
behavior in an upcoming semver-major release.

codecov[bot]: # [Codecov](https://codecov.io/gh/alangpierce/sucrase/pull/727?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) Report
> Merging [#727](https://codecov.io/gh/alangpierce/sucrase/pull/727?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) (c9b3fba) into [main](https://codecov.io/gh/alangpierce/sucrase/commit/5268e6cbcdff13ae2c6219658981c03ac8e7fae6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) (5268e6c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##             main     #727   +/-   ##
=======================================
  Coverage   87.15%   87.16%           
=======================================
  Files          54       54           
  Lines        5708     5711    +3     
  Branches     1345     1346    +1     
=======================================
+ Hits         4975     4978    +3     
  Misses        466      466           
  Partials      267      267           
```


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

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?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:** 324.1 thousand lines per second
**After this PR:**  313.9 thousand lines per second

**Measured change:** 3.14% slower (7.16% slower to 0.31% slower)
**Summary:** Probably slower

Patch:
diff --git a/README.md b/README.md
index 4b3ae33..8dfef42 100644
--- a/README.md
+++ b/README.md
@@ -62,7 +62,11 @@ transforms are available:
 * **flow**:  Removes Flow type annotations. Does not check types.
 * **imports**: Transforms ES Modules (`import`/`export`) to CommonJS
   (`require`/`module.exports`) using the same approach as Babel and TypeScript
-  with `--esModuleInterop`. Also includes dynamic `import`.
+  with `--esModuleInterop`. If `preserveDynamicImport` is specified in the Sucrase
+  options, then dynamic `import` expressions are left alone, which is particularly
+  useful in Node to load ESM-only libraries. If `preserveDynamicImport` is not
+  specified, `import` expressions are transformed into a promise-wrapped call to
+  `require`.
 * **react-hot-loader**: Performs the equivalent of the `react-hot-loader/babel`
   transform in the [react-hot-loader](https://github.com/gaearon/react-hot-loader)
   project. This enables advanced hot reloading use cases such as editing of
diff --git a/src/Options-gen-types.ts b/src/Options-gen-types.ts
index 8ea2283..26b886e 100644
--- a/src/Options-gen-types.ts
+++ b/src/Options-gen-types.ts
@@ -27,6 +27,7 @@ export const Options = t.iface([], {
   filePath: t.opt("string"),
   production: t.opt("boolean"),
   disableESTransforms: t.opt("boolean"),
+  preserveDynamicImport: t.opt("boolean"),
 });
 
 const exportedTypeSuite: t.ITypeSuite = {
diff --git a/src/Options.ts b/src/Options.ts
index 2616734..a007e3b 100644
--- a/src/Options.ts
+++ b/src/Options.ts
@@ -51,6 +51,11 @@ export interface Options {
    * separators, etc.
    */
   disableESTransforms?: boolean;
+  /**
+   * If specified, the imports transform does not attempt to change dynamic import()
+   * expressions into require() calls.
+   */
+  preserveDynamicImport?: boolean;
 }
 
 export function validateOptions(options: Options): void {
diff --git a/src/transformers/CJSImportTransformer.ts b/src/transformers/CJSImportTransformer.ts
index aa05d56..7b29047 100644
--- a/src/transformers/CJSImportTransformer.ts
+++ b/src/transformers/CJSImportTransformer.ts
@@ -32,6 +32,7 @@ export default class CJSImportTransformer extends Transformer {
     readonly reactHotLoaderTransformer: ReactHotLoaderTransformer | null,
     readonly enableLegacyBabel5ModuleInterop: boolean,
     readonly isTypeScriptTransformEnabled: boolean,
+    readonly preserveDynamicImport: boolean,
   ) {
     super();
     this.declarationInfo = isTypeScriptTransformEnabled
@@ -115,6 +116,11 @@ export default class CJSImportTransformer extends Transformer {
    */
   private processImport(): void {
     if (this.tokens.matches2(tt._import, tt.parenL)) {
+      if (this.preserveDynamicImport) {
+        // Bail out, only making progress for this one token.
+        this.tokens.copyToken();
+        return;
+      }
       this.tokens.replaceToken("Promise.resolve().then(() => require");
       const contextId = this.tokens.currentToken().contextId;
       if (contextId == null) {
diff --git a/src/transformers/RootTransformer.ts b/src/transformers/RootTransformer.ts
index 1e167a3..9b15a80 100644
--- a/src/transformers/RootTransformer.ts
+++ b/src/transformers/RootTransformer.ts
@@ -84,6 +84,7 @@ export default class RootTransformer {
           reactHotLoaderTransformer,
           enableLegacyBabel5ModuleInterop,
           transforms.includes("typescript"),
+          Boolean(options.preserveDynamicImport),
         ),
       );
     } else {


Test patch:
diff --git a/test/imports-test.ts b/test/imports-test.ts
index 70da265..4841e13 100644
--- a/test/imports-test.ts
+++ b/test/imports-test.ts
@@ -1025,7 +1025,7 @@ module.exports = exports.default;
     );
   });
 
-  it("handles dynamic imports", () => {
+  it("transforms dynamic imports by default", () => {
     assertResult(
       `
       async function loadThing() {
@@ -1040,6 +1040,26 @@ module.exports = exports.default;
     );
   });
 
+  it("preserves dynamic import when configured to do so", () => {
+    assertResult(
+      `
+      import Bar from './Bar.js';
+      async function loadThing() {
+        const foo = await import('foo');
+        console.log(Bar);
+      }
+    `,
+      `"use strict";${IMPORT_DEFAULT_PREFIX}
+      var _Barjs = require('./Bar.js'); var _Barjs2 = _interopRequireDefault(_Barjs);
+      async function loadThing() {
+        const foo = await import('foo');
+        console.log(_Barjs2.default);
+      }
+    `,
+      {transforms: ["imports"], preserveDynamicImport: true},
+    );
+  });
+
   it("properly handles object destructuring with imported names", () => {
     assertResult(
       `