Repository: alangpierce/sucrase
Pull request #728 (2022-07-26 07:04:15)

Add flag to emit createRequire matching TS nodenext behavior
Progress toward #726

This is currently an opt-in flag handling a nuance in how TS transpiles imports
like these:
```ts
import foo = require('foo');
```
In the new nodenext mode when targeting ESM, TS now transforms this code to use
`createRequire`. The change is described here:
https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#commonjs-interoperability

This PR adds a flag `injectCreateRequireForImportRequire` to enable this
different behavior. I'm gating this behind a flag out of caution, though it's
worth noting that TS gives an error when using this syntax and targeting
module esnext, so it will likely be safe to switch to this new emit strategy as
the default behavior in the future.

As I understand it, the main benefit of this change over explicit `createRequire`
is that it allows a single codebase to be transpiled to Node ESM and Node CJS
while using this syntax. A downside is that `createRequire` is Node-specific and
needs special support from bundlers, but it looks like webpack can recognize the
pattern. In most situations, real ESM `import` syntax is probably preferable,
but this syntax makes it possible to force the use of CJS.

The ts-node transpiler plugin system expects transpilers to have this mode as an
option, so this is a step closer to having a compliant ts-node plugin.

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

```diff
@@           Coverage Diff           @@
##             main     #728   +/-   ##
=======================================
  Coverage   87.16%   87.17%           
=======================================
  Files          54       54           
  Lines        5711     5722   +11     
  Branches     1346     1349    +3     
=======================================
+ Hits         4978     4988   +10     
  Misses        466      466           
- Partials      267      268    +1     
```


| [Impacted Files](https://codecov.io/gh/alangpierce/sucrase/pull/728?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/728/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/728/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/728/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/HelperManager.ts](https://codecov.io/gh/alangpierce/sucrase/pull/728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce#diff-c3JjL0hlbHBlck1hbmFnZXIudHM=) | `96.87% <80.00%> (-3.13%)` | :arrow_down: |
| [src/transformers/ESMImportTransformer.ts](https://codecov.io/gh/alangpierce/sucrase/pull/728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce#diff-c3JjL3RyYW5zZm9ybWVycy9FU01JbXBvcnRUcmFuc2Zvcm1lci50cw==) | `94.11% <100.00%> (+0.24%)` | :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:** 393 thousand lines per second
**After this PR:**  392.4 thousand lines per second

**Measured change:** 0.15% slower (0.96% slower to 0.31% faster)
**Summary:** Likely no significant difference

Patch:
diff --git a/README.md b/README.md
index 8dfef42..ea3c840 100644
--- a/README.md
+++ b/README.md
@@ -75,6 +75,12 @@ transforms are available:
   the same way as [babel-plugin-jest-hoist](https://github.com/facebook/jest/tree/master/packages/babel-plugin-jest-hoist).
   Does not validate the arguments passed to `jest.mock`, but the same rules still apply.
 
+When the `imports` transform is *not* specified (i.e. when targeting ESM), the
+`injectCreateRequireForImportRequire` option can be specified to transform TS
+`import foo = require("foo");` in a way that matches the
+[TypeScript 4.7 behavior](https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#commonjs-interoperability)
+with `module: nodenext`.
+
 These newer JS features are transformed by default:
 
 * [Optional chaining](https://github.com/tc39/proposal-optional-chaining): `a?.b`
diff --git a/src/HelperManager.ts b/src/HelperManager.ts
index 0bc5162..8571e49 100644
--- a/src/HelperManager.ts
+++ b/src/HelperManager.ts
@@ -1,6 +1,10 @@
 import type NameManager from "./NameManager";
 
 const HELPERS: {[name: string]: string} = {
+  require: `
+    import {createRequire as CREATE_REQUIRE_NAME} from "module";
+    const require = CREATE_REQUIRE_NAME(import.meta.url);
+  `,
   interopRequireWildcard: `
     function interopRequireWildcard(obj) {
       if (obj && obj.__esModule) {
@@ -125,6 +129,7 @@ const HELPERS: {[name: string]: string} = {
 
 export class HelperManager {
   helperNames: {[baseName in keyof typeof HELPERS]?: string} = {};
+  createRequireName: string | null = null;
   constructor(readonly nameManager: NameManager) {}
 
   getHelperName(baseName: keyof typeof HELPERS): string {
@@ -155,6 +160,11 @@ export class HelperManager {
           "ASYNC_OPTIONAL_CHAIN_NAME",
           this.helperNames.asyncOptionalChain!,
         );
+      } else if (baseName === "require") {
+        if (this.createRequireName === null) {
+          this.createRequireName = this.nameManager.claimFreeName("_createRequire");
+        }
+        helperCode = helperCode.replace(/CREATE_REQUIRE_NAME/g, this.createRequireName);
       }
       if (helperName) {
         resultCode += " ";
diff --git a/src/Options-gen-types.ts b/src/Options-gen-types.ts
index 26b886e..584394c 100644
--- a/src/Options-gen-types.ts
+++ b/src/Options-gen-types.ts
@@ -28,6 +28,7 @@ export const Options = t.iface([], {
   production: t.opt("boolean"),
   disableESTransforms: t.opt("boolean"),
   preserveDynamicImport: t.opt("boolean"),
+  injectCreateRequireForImportRequire: t.opt("boolean"),
 });
 
 const exportedTypeSuite: t.ITypeSuite = {
diff --git a/src/Options.ts b/src/Options.ts
index a007e3b..f56a2c2 100644
--- a/src/Options.ts
+++ b/src/Options.ts
@@ -56,6 +56,17 @@ export interface Options {
    * expressions into require() calls.
    */
   preserveDynamicImport?: boolean;
+  /**
+   * Only relevant when targeting ESM (i.e. when the imports transform is *not*
+   * specified). This flag changes the behavior of TS require imports:
+   *
+   * import Foo = require("foo");
+   *
+   * to import createRequire, create a require function, and use that function.
+   * This is the TS behavior with module: nodenext and makes it easier for the
+   * same code to target ESM and CJS.
+   */
+  injectCreateRequireForImportRequire?: boolean;
 }
 
 export function validateOptions(options: Options): void {
diff --git a/src/transformers/ESMImportTransformer.ts b/src/transformers/ESMImportTransformer.ts
index 2a2c394..84e64c4 100644
--- a/src/transformers/ESMImportTransformer.ts
+++ b/src/transformers/ESMImportTransformer.ts
@@ -1,3 +1,4 @@
+import type {HelperManager} from "../HelperManager";
 import type {Options} from "../index";
 import type NameManager from "../NameManager";
 import {ContextualKeyword} from "../parser/tokenizer/keywords";
@@ -21,10 +22,12 @@ import Transformer from "./Transformer";
 export default class ESMImportTransformer extends Transformer {
   private nonTypeIdentifiers: Set<string>;
   private declarationInfo: DeclarationInfo;
+  private injectCreateRequireForImportRequire: boolean;
 
   constructor(
     readonly tokens: TokenProcessor,
     readonly nameManager: NameManager,
+    readonly helperManager: HelperManager,
     readonly reactHotLoaderTransformer: ReactHotLoaderTransformer | null,
     readonly isTypeScriptTransformEnabled: boolean,
     options: Options,
@@ -36,6 +39,7 @@ export default class ESMImportTransformer extends Transformer {
     this.declarationInfo = isTypeScriptTransformEnabled
       ? getDeclarationInfo(tokens)
       : EMPTY_DECLARATION_INFO;
+    this.injectCreateRequireForImportRequire = Boolean(options.injectCreateRequireForImportRequire);
   }
 
   process(): boolean {
@@ -109,8 +113,19 @@ export default class ESMImportTransformer extends Transformer {
     if (this.isTypeName(importName)) {
       // If this name is only used as a type, elide the whole import.
       elideImportEquals(this.tokens);
+    } else if (this.injectCreateRequireForImportRequire) {
+      // We're using require in an environment (Node ESM) that doesn't provide
+      // it as a global, so generate a helper to import it.
+      // import -> const
+      this.tokens.replaceToken("const");
+      // Foo
+      this.tokens.copyToken();
+      // =
+      this.tokens.copyToken();
+      // require
+      this.tokens.replaceToken(this.helperManager.getHelperName("require"));
     } else {
-      // Otherwise, switch `import` to `const`.
+      // Otherwise, just switch `import` to `const`.
       this.tokens.replaceToken("const");
     }
     return true;
diff --git a/src/transformers/RootTransformer.ts b/src/transformers/RootTransformer.ts
index 9b15a80..5a1aae1 100644
--- a/src/transformers/RootTransformer.ts
+++ b/src/transformers/RootTransformer.ts
@@ -92,6 +92,7 @@ export default class RootTransformer {
         new ESMImportTransformer(
           tokenProcessor,
           this.nameManager,
+          this.helperManager,
           reactHotLoaderTransformer,
           transforms.includes("typescript"),
           options,


Test patch:
diff --git a/test/prefixes.ts b/test/prefixes.ts
index 99633b0..aba561b 100644
--- a/test/prefixes.ts
+++ b/test/prefixes.ts
@@ -1,4 +1,6 @@
 export const JSX_PREFIX = 'const _jsxFileName = "";';
+export const CREATE_REQUIRE_PREFIX = ` import {createRequire as _createRequire} from "module"; \
+const _require = _createRequire(import.meta.url);`;
 export const IMPORT_DEFAULT_PREFIX = ` function _interopRequireDefault(obj) { \
 return obj && obj.__esModule ? obj : { default: obj }; }`;
 export const IMPORT_WILDCARD_PREFIX = ` function _interopRequireWildcard(obj) { \
diff --git a/test/typescript-test.ts b/test/typescript-test.ts
index b05fd3b..16bde5d 100644
--- a/test/typescript-test.ts
+++ b/test/typescript-test.ts
@@ -1,5 +1,6 @@
 import type {Options} from "../src/Options";
 import {
+  CREATE_REQUIRE_PREFIX,
   CREATE_STAR_EXPORT_PREFIX,
   ESMODULE_PREFIX,
   IMPORT_DEFAULT_PREFIX,
@@ -893,6 +894,47 @@ describe("typescript transform", () => {
     );
   });
 
+  it("ignores injectCreateRequireForImportRequire when targeting CJS", () => {
+    assertTypeScriptResult(
+      `
+      import a = require('a');
+      console.log(a);
+    `,
+      `"use strict";
+      const a = require('a');
+      console.log(a);
+    `,
+      {injectCreateRequireForImportRequire: true},
+    );
+  });
+
+  it("preserves import = require by default when targeting ESM", () => {
+    assertTypeScriptESMResult(
+      `
+      import a = require('a');
+      console.log(a);
+    `,
+      `
+      const a = require('a');
+      console.log(a);
+    `,
+    );
+  });
+
+  it("transforms import = require when targeting ESM and injectCreateRequireForImportRequire is enabled", () => {
+    assertTypeScriptESMResult(
+      `
+      import a = require('a');
+      console.log(a);
+    `,
+      `${CREATE_REQUIRE_PREFIX}
+      const a = _require('a');
+      console.log(a);
+    `,
+      {injectCreateRequireForImportRequire: true},
+    );
+  });
+
   it("allows this types in functions", () => {
     assertTypeScriptResult(
       `