Add keepUnusedImports option (#811)

Supersedes #615
Fixes #791

This PR builds off of #615 with a number of additional tests and cases. High-level, it introduces a new option `keepUnusedImports` that disables all automatic import and export elision done by TypeScript, somewhat analogous to the `verbatimModuleSyntax` TS option. For the most part, this is just a matter of flagging off the name tracking and elision code
for both imports and exports in the various places that they happen.

When this mode is enabled, some of the preprocessing steps can be skipped, and scope handling can be fully skipped with
the imports transform also disabled. Performance numbers measured by `yarn benchmark jest-dev`:
* With `imports` and `typescript` transforms: 855k -> 910k lines per second (6% faster)
* With only `typescript` transform: 935k -> 1110k lines per second (19% faster)

Some additional details:
* While this feature is intended to be used with TypeScript, I decided to also implement the behavior for Flow as well. Flow's implementation in Babel will remove an import statement if all individual names are removed, but `flow-remove-types` won't do this, so Sucrase `keepUnusedImports` matches the behavior of `flow-remove-types`.
* Babel doesn't currently implement this option, so for the playground I mapped it to `onlyRemoveTypeImports`, which
  seems to have the closest behavior.

README.md

@@ -73,7 +73,9 @@ transforms are available:
   annotations and handling features like enums. Does not check types. Sucrase
   transforms each file independently, so you should enable the `isolatedModules`
   TypeScript flag so that the typechecker will disallow the few features like
-  `const enum`s that need cross-file compilation.
+  `const enum`s that need cross-file compilation. The Sucrase option `keepUnusedImports`
+  can be used to disable all automatic removal of imports and exports, analogous to TS
+  `verbatimModuleSyntax`.
 * **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

integration-test/test-cases/ts-node-cases/option-cases/respects-verbatim-module-syntax/main.ts

@@ -0,0 +1,6 @@
+// This import should be preserved.
+import A from './set-global-to-3.js'
+
+if (global.testValue !== 3) {
+  throw new Error();
+}

integration-test/test-cases/ts-node-cases/option-cases/respects-verbatim-module-syntax/package.json

@@ -0,0 +1,3 @@
+{
+  "type": "module"
+}

integration-test/test-cases/ts-node-cases/option-cases/respects-verbatim-module-syntax/set-global-to-3.ts

@@ -0,0 +1,2 @@
+global.testValue = 3;
+export default 2;

integration-test/test-cases/ts-node-cases/option-cases/respects-verbatim-module-syntax/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "compilerOptions": {
+    "module": "NodeNext",
+    "target": "ESNext",
+    "esModuleInterop": true,
+    "verbatimModuleSyntax": true
+  },
+  "ts-node": {
+    "experimentalResolver": true
+  }
+}

src/CJSImportProcessor.ts

@@ -43,6 +43,7 @@ export default class CJSImportProcessor {
     readonly enableLegacyTypeScriptModuleInterop: boolean,
     readonly options: Options,
     readonly isTypeScriptTransformEnabled: boolean,
+    readonly keepUnusedImports: boolean,
     readonly helperManager: HelperManager,
   ) {}
 
@@ -65,8 +66,8 @@ export default class CJSImportProcessor {
   }
 
   /**
-   * In TypeScript, import statements that only import types should be removed. This does not count
-   * bare imports.
+   * In TypeScript, import statements that only import types should be removed.
+   * This includes `import {} from 'foo';`, but not `import 'foo';`.
    */
   pruneTypeOnlyImports(): void {
     this.nonTypeIdentifiers = getNonTypeIdentifiers(this.tokens, this.options);
@@ -84,14 +85,18 @@ export default class CJSImportProcessor {
         ...importInfo.wildcardNames,
         ...importInfo.namedImports.map(({localName}) => localName),
       ];
-      if (names.every((name) => this.isTypeName(name))) {
+      if (names.every((name) => this.shouldAutomaticallyElideImportedName(name))) {
         this.importsToReplace.set(path, "");
       }
     }
   }
 
-  isTypeName(name: string): boolean {
-    return this.isTypeScriptTransformEnabled && !this.nonTypeIdentifiers.has(name);
+  shouldAutomaticallyElideImportedName(name: string): boolean {
+    return (
+      this.isTypeScriptTransformEnabled &&
+      !this.keepUnusedImports &&
+      !this.nonTypeIdentifiers.has(name)
+    );
   }
 
   private generateImportReplacements(): void {

src/Options-gen-types.ts

@@ -25,6 +25,7 @@ export const Options = t.iface([], {
   jsxImportSource: t.opt("string"),
   jsxPragma: t.opt("string"),
   jsxFragmentPragma: t.opt("string"),
+  keepUnusedImports: t.opt("boolean"),
   preserveDynamicImport: t.opt("boolean"),
   injectCreateRequireForImportRequire: t.opt("boolean"),
   enableLegacyTypeScriptModuleInterop: t.opt("boolean"),

src/Options.ts

@@ -55,6 +55,12 @@ export interface Options {
    * compiling JSX with the classic runtime.
    */
   jsxFragmentPragma?: string;
+  /**
+   * If specified, disable automatic removal of type-only import and export
+   * statements and names. Only statements and names that explicitly use the
+   * `type` keyword are removed.
+   */
+  keepUnusedImports?: boolean;
   /**
    * If specified, the imports transform does not attempt to change dynamic
    * import() expressions into require() calls.

src/index.ts

@@ -115,16 +115,18 @@ function getSucraseContext(code: string, options: Options): SucraseContext {
       enableLegacyTypeScriptModuleInterop,
       options,
       options.transforms.includes("typescript"),
+      Boolean(options.keepUnusedImports),
       helperManager,
     );
     importProcessor.preprocessTokens();
     // We need to mark shadowed globals after processing imports so we know that the globals are,
     // but before type-only import pruning, since that relies on shadowing information.
     identifyShadowedGlobals(tokenProcessor, scopes, importProcessor.getGlobalNames());
-    if (options.transforms.includes("typescript")) {
+    if (options.transforms.includes("typescript") && !options.keepUnusedImports) {
       importProcessor.pruneTypeOnlyImports();
     }
-  } else if (options.transforms.includes("typescript")) {
+  } else if (options.transforms.includes("typescript") && !options.keepUnusedImports) {
+    // Shadowed global detection is needed for TS implicit elision of imported names.
     identifyShadowedGlobals(tokenProcessor, scopes, getTSImportedNames(tokenProcessor));
   }
   return {tokenProcessor, scopes, nameManager, importProcessor, helperManager};

src/transformers/CJSImportTransformer.ts

@@ -39,6 +39,7 @@ export default class CJSImportTransformer extends Transformer {
     readonly isTypeScriptTransformEnabled: boolean,
     readonly isFlowTransformEnabled: boolean,
     readonly preserveDynamicImport: boolean,
+    readonly keepUnusedImports: boolean,
   ) {
     super();
     this.declarationInfo = isTypeScriptTransformEnabled
@@ -101,7 +102,7 @@ export default class CJSImportTransformer extends Transformer {
 
   private processImportEquals(): boolean {
     const importName = this.tokens.identifierNameAtIndex(this.tokens.currentIndex() + 1);
-    if (this.importProcessor.isTypeName(importName)) {
+    if (this.importProcessor.shouldAutomaticallyElideImportedName(importName)) {
       // If this name is only used as a type, elide the whole import.
       elideImportEquals(this.tokens);
     } else {
@@ -222,6 +223,9 @@ export default class CJSImportTransformer extends Transformer {
       }
       this.tokens.removeToken();
     }
+    if (this.keepUnusedImports) {
+      return false;
+    }
     if (this.isTypeScriptTransformEnabled) {
       return !foundNonTypeImport;
     } else if (this.isFlowTransformEnabled) {
@@ -546,7 +550,12 @@ export default class CJSImportTransformer extends Transformer {
       this.tokens.appendCode(` exports.default = ${name};`);
       // After this point, this is a plain "export default E" statement.
     } else if (
-      shouldElideDefaultExport(this.isTypeScriptTransformEnabled, this.tokens, this.declarationInfo)
+      shouldElideDefaultExport(
+        this.isTypeScriptTransformEnabled,
+        this.keepUnusedImports,
+        this.tokens,
+        this.declarationInfo,
+      )
     ) {
       // If the exported value is just an identifier and should be elided by TypeScript
       // rules, then remove it entirely. It will always have the form `export default e`,
@@ -898,6 +907,10 @@ export default class CJSImportTransformer extends Transformer {
   }
 
   private shouldElideExportedIdentifier(name: string): boolean {
-    return this.isTypeScriptTransformEnabled && !this.declarationInfo.valueDeclarations.has(name);
+    return (
+      this.isTypeScriptTransformEnabled &&
+      !this.keepUnusedImports &&
+      !this.declarationInfo.valueDeclarations.has(name)
+    );
   }
 }

src/transformers/ESMImportTransformer.ts

@@ -33,15 +33,18 @@ export default class ESMImportTransformer extends Transformer {
     readonly reactHotLoaderTransformer: ReactHotLoaderTransformer | null,
     readonly isTypeScriptTransformEnabled: boolean,
     readonly isFlowTransformEnabled: boolean,
+    readonly keepUnusedImports: boolean,
     options: Options,
   ) {
     super();
-    this.nonTypeIdentifiers = isTypeScriptTransformEnabled
-      ? getNonTypeIdentifiers(tokens, options)
-      : new Set();
-    this.declarationInfo = isTypeScriptTransformEnabled
-      ? getDeclarationInfo(tokens)
-      : EMPTY_DECLARATION_INFO;
+    this.nonTypeIdentifiers =
+      isTypeScriptTransformEnabled && !keepUnusedImports
+        ? getNonTypeIdentifiers(tokens, options)
+        : new Set();
+    this.declarationInfo =
+      isTypeScriptTransformEnabled && !keepUnusedImports
+        ? getDeclarationInfo(tokens)
+        : EMPTY_DECLARATION_INFO;
     this.injectCreateRequireForImportRequire = Boolean(options.injectCreateRequireForImportRequire);
   }
 
@@ -277,6 +280,9 @@ export default class ESMImportTransformer extends Transformer {
       this.tokens.copyExpectedToken(tt.braceR);
     }
 
+    if (this.keepUnusedImports) {
+      return false;
+    }
     if (this.isTypeScriptTransformEnabled) {
       return !foundNonTypeImport;
     } else if (this.isFlowTransformEnabled) {
@@ -288,12 +294,21 @@ export default class ESMImportTransformer extends Transformer {
   }
 
   private shouldAutomaticallyElideImportedName(name: string): boolean {
-    return this.isTypeScriptTransformEnabled && !this.nonTypeIdentifiers.has(name);
+    return (
+      this.isTypeScriptTransformEnabled &&
+      !this.keepUnusedImports &&
+      !this.nonTypeIdentifiers.has(name)
+    );
   }
 
   private processExportDefault(): boolean {
     if (
-      shouldElideDefaultExport(this.isTypeScriptTransformEnabled, this.tokens, this.declarationInfo)
+      shouldElideDefaultExport(
+        this.isTypeScriptTransformEnabled,
+        this.keepUnusedImports,
+        this.tokens,
+        this.declarationInfo,
+      )
     ) {
       // If the exported value is just an identifier and should be elided by TypeScript
       // rules, then remove it entirely. It will always have the form `export default e`,
@@ -373,7 +388,7 @@ export default class ESMImportTransformer extends Transformer {
     }
     this.tokens.copyExpectedToken(tt.braceR);
 
-    if (isReExport && !foundNonTypeExport) {
+    if (!this.keepUnusedImports && isReExport && !foundNonTypeExport) {
       // This is a type-only re-export, so skip evaluating the other module. Technically this
       // leaves the statement as `export {}`, but that's ok since that's a no-op.
       this.tokens.removeToken();
@@ -392,6 +407,7 @@ export default class ESMImportTransformer extends Transformer {
   private shouldElideExportedName(name: string): boolean {
     return (
       this.isTypeScriptTransformEnabled &&
+      !this.keepUnusedImports &&
       this.declarationInfo.typeDeclarations.has(name) &&
       !this.declarationInfo.valueDeclarations.has(name)
     );

src/transformers/RootTransformer.ts

@@ -97,6 +97,7 @@ export default class RootTransformer {
           transforms.includes("typescript"),
           transforms.includes("flow"),
           Boolean(options.preserveDynamicImport),
+          Boolean(options.keepUnusedImports),
         ),
       );
     } else {
@@ -108,6 +109,7 @@ export default class RootTransformer {
           reactHotLoaderTransformer,
           transforms.includes("typescript"),
           transforms.includes("flow"),
+          Boolean(options.keepUnusedImports),
           options,
         ),
       );

src/util/shouldElideDefaultExport.ts

@@ -7,10 +7,11 @@ import type {DeclarationInfo} from "./getDeclarationInfo";
  */
 export default function shouldElideDefaultExport(
   isTypeScriptTransformEnabled: boolean,
+  keepUnusedImports: boolean,
   tokens: TokenProcessor,
   declarationInfo: DeclarationInfo,
 ): boolean {
-  if (!isTypeScriptTransformEnabled) {
+  if (!isTypeScriptTransformEnabled || keepUnusedImports) {
     return false;
   }
   const exportToken = tokens.currentToken();

test/flow-test.ts

@@ -21,7 +21,7 @@ function assertFlowESMResult(
 }
 
 describe("transform flow", () => {
-  it("removes `import type` statements", () => {
+  it("removes `import type` statements when targeting CJS", () => {
     assertFlowResult(
       `
       import type {a} from 'b';
@@ -42,6 +42,47 @@ describe("transform flow", () => {
     );
   });
 
+  it("properly prunes flow imported names when when targeting ESM", () => {
+    assertFlowESMResult(
+      `
+      import a, {type n as b, m as c, type d} from './e';
+      import type f from './g';
+      import {type h} from './i';
+      import j, {} from './k';
+    `,
+      `
+      import a, { m as c,} from './e';
+
+
+      import j, {} from './k';
+    `,
+    );
+  });
+
+  it("respects keepUnusedImports when targeting CJS", () => {
+    assertFlowResult(
+      `
+      import {type T} from 'a';
+    `,
+      `"use strict";
+      require('a');
+    `,
+      {keepUnusedImports: true},
+    );
+  });
+
+  it("respects keepUnusedImports when targeting ESM", () => {
+    assertFlowESMResult(
+      `
+      import {type T} from 'a';
+    `,
+      `
+      import {} from 'a';
+    `,
+      {keepUnusedImports: true},
+    );
+  });
+
   it("preserves import {} statements when targeting CJS", () => {
     assertFlowResult(
       `
@@ -270,23 +311,6 @@ describe("transform flow", () => {
     );
   });
 
-  it("properly prunes flow imported names", () => {
-    assertFlowESMResult(
-      `
-      import a, {type n as b, m as c, type d} from './e';
-      import type f from './g';
-      import {type h} from './i';
-      import j, {} from './k';
-    `,
-      `
-      import a, { m as c,} from './e';
-
-
-      import j, {} from './k';
-    `,
-    );
-  });
-
   it("removes @flow directives", () => {
     assertFlowResult(
       `

test/identifyShadowedGlobals-test.ts

@@ -20,6 +20,7 @@ function assertHasShadowedGlobals(code: string, expected: boolean): void {
       transforms: [],
     },
     false,
+    false,
     helperManager,
   );
   importProcessor.preprocessTokens();