Fix dynamic import on plain CJS files

Fixes #778

Previously, `import()` transpiled to a promise-wrapped `require`, which worked
fine for ESM-to-CJS-transpiled modules, but when importing plain CJS modules,
the behavior was inconsistent with other implementations. In that case, we
needed to call `_interopRequireWildcard` to nest the module under a `default`
key.

I tested to confirm that this updated behavior is consistent with Node
ESM-to-CJS dynamic import, as well as Babel, TypeScript, and swc, so this change
will be considered a bug fix rather than a breaking change, even though it is
possible that existing use cases may have been relying on the old behavior.

integration-test/integration-tests.ts

@@ -30,6 +30,19 @@ describe("integration tests", () => {
     }
   }
 
+  /**
+   * Find sucrase/register integration tests.
+   *
+   * Each test has a file starting with "main" (e.g. main.ts, main.tsx,
+   * main.mts, etc) that's used as the entry point. The test should be written
+   * in such a way that the execution throws an exception if the test fails.
+   */
+  for (const testFile of discoverTests("test-cases/register-cases", "main")) {
+    it(testFile, async () => {
+      await execPromise(`node -r ${__dirname}/../register ${testFile}`);
+    });
+  }
+
   /**
    * Find Jest integration tests.
    *

integration-test/test-cases/register-cases/allows-dynamic-import/main.js

@@ -0,0 +1,14 @@
+async function main() {
+  const plainCJSFile = await import("./plain-cjs-file");
+  const transpiledESMFile = await import("./transpiled-esm-file");
+  if (plainCJSFile.default !== 15) {
+    throw new Error();
+  }
+  if (transpiledESMFile.a !== 1) {
+    throw new Error();
+  }
+  if (transpiledESMFile.default !== 3) {
+    throw new Error();
+  }
+}
+main();

integration-test/test-cases/register-cases/allows-dynamic-import/plain-cjs-file.js

@@ -0,0 +1 @@
+module.exports = 15;

integration-test/test-cases/register-cases/allows-dynamic-import/transpiled-esm-file.js

@@ -0,0 +1,2 @@
+export const a = 1;
+export default 3;

src/transformers/CJSImportTransformer.ts

@@ -1,4 +1,5 @@
 import type CJSImportProcessor from "../CJSImportProcessor";
+import type {HelperManager} from "../HelperManager";
 import type NameManager from "../NameManager";
 import {IdentifierRole, isDeclaration, isObjectShorthandDeclaration} from "../parser/tokenizer";
 import {ContextualKeyword} from "../parser/tokenizer/keywords";
@@ -30,6 +31,7 @@ export default class CJSImportTransformer extends Transformer {
     readonly tokens: TokenProcessor,
     readonly importProcessor: CJSImportProcessor,
     readonly nameManager: NameManager,
+    readonly helperManager: HelperManager,
     readonly reactHotLoaderTransformer: ReactHotLoaderTransformer | null,
     readonly enableLegacyBabel5ModuleInterop: boolean,
     readonly isTypeScriptTransformEnabled: boolean,
@@ -122,7 +124,10 @@ export default class CJSImportTransformer extends Transformer {
         this.tokens.copyToken();
         return;
       }
-      this.tokens.replaceToken("Promise.resolve().then(() => require");
+      const interopRequireWildcardName = this.helperManager.getHelperName("interopRequireWildcard");
+      this.tokens.replaceToken(
+        `Promise.resolve().then(() => ${interopRequireWildcardName}(require`,
+      );
       const contextId = this.tokens.currentToken().contextId;
       if (contextId == null) {
         throw new Error("Expected context ID on dynamic import invocation.");
@@ -131,7 +136,7 @@ export default class CJSImportTransformer extends Transformer {
       while (!this.tokens.matchesContextIdAndLabel(tt.parenR, contextId)) {
         this.rootTransformer.processToken();
       }
-      this.tokens.replaceToken("))");
+      this.tokens.replaceToken(")))");
       return;
     }
 

src/transformers/RootTransformer.ts

@@ -83,6 +83,7 @@ export default class RootTransformer {
           tokenProcessor,
           importProcessor,
           this.nameManager,
+          this.helperManager,
           reactHotLoaderTransformer,
           enableLegacyBabel5ModuleInterop,
           transforms.includes("typescript"),

test/imports-test.ts

@@ -1055,9 +1055,9 @@ module.exports = exports.default;
         const foo = await import('foo');
       }
     `,
-      `"use strict";
+      `"use strict";${IMPORT_WILDCARD_PREFIX}
       async function loadThing() {
-        const foo = await Promise.resolve().then(() => require('foo'));
+        const foo = await Promise.resolve().then(() => _interopRequireWildcard(require('foo')));
       }
     `,
     );