fix: Numerous issues with elision due to new TS features in v5+ (fixes #184)

.github/workflows/main.yml

@@ -8,8 +8,7 @@ jobs:
     strategy:
       matrix:
         os: [ ubuntu-latest, windows-latest, macos-latest ]
-        # Node 16.10 due to https://github.com/facebook/jest/issues/11956
-        node-version: [ 16.10.x, 18.x ]
+        node-version: [ 18, 20, 21 ]
 
     steps:
       - name: Checkout
@@ -39,6 +38,7 @@ jobs:
         run: yarn build
 
       - name: Test
-        run: yarn test --runInBand=false --maxWorkers=2 --workerIdleMemoryLimit=1700MB # https://github.com/facebook/jest/issues/11956
+        run: yarn test --runInBand=false --maxWorkers=2 --workerIdleMemoryLimit=2GB # https://github.com/facebook/jest/issues/11956
         env:
           CI: true
+          NODE_OPTIONS: --max_old_space_size=4096

package.json

@@ -11,11 +11,11 @@
     "release": "standard-version",
     "--------------": "",
     "format": "prettier --write \"{src,test}/**/{*.js,!(*.d).ts}\"",
-    "clean": "npx -y rimraf dist **/*.tsbuildinfo ./test/projects/nx/dist",
-    "clean:all": "yarn run clean && npx -y rimraf node_modules **/node_modules **/yarn.lock yarn.lock",
-    "reset": "yarn run clean:all && yarn install",
+    "clean": "npx -y rimraf -g dist **/*.tsbuildinfo ./test/projects/nx/dist",
+    "clean:all": "yarn run clean && npx -y rimraf -g node_modules **/node_modules **/yarn.lock yarn.lock",
+    "reset": "yarn run clean:all && yarn install && yarn build",
     "-------------- ": "",
-    "prebuild": "npx -y rimraf dist",
+    "prebuild": "npx -y rimraf -g dist",
     "install:tests": "cd test && yarn install",
     "prepare": "yarn run install:tests"
   },
@@ -40,9 +40,12 @@
   },
   "license": "MIT",
   "contributors": [
-    "Daniel Perez Alvarez <[email protected]>",
-    "Ron S. <[email protected]>"
+    "Daniel Perez Alvarez <[email protected]>"
   ],
+  "author": {
+    "name": "Ron S.",
+    "url": "https://twitter.com/Ron"
+  },
   "files": [
     "dist",
     "types",
@@ -55,15 +58,15 @@
     "@types/jest": "^29.2.0",
     "@types/minimatch": "^5.1.2",
     "@types/node": "^18.11.2",
-    "jest": "^29.3.1",
+    "jest": "^29.7.0",
     "prettier": "^2.7.1",
-    "rimraf": "^3.0.2",
+    "rimraf": "^5.0.5",
     "standard-version": "^9.5.0",
     "@types/ts-expose-internals": "npm:[email protected]",
-    "ts-jest": "^29.0.3",
+    "ts-jest": "^29.1.2",
     "ts-node": "^10.9.1",
-    "ts-patch": "^2.1.0",
-    "typescript": "^4.9.4"
+    "ts-patch": "^3.1.2",
+    "typescript": "^5.3.3"
   },
   "peerDependencies": {
     "typescript": ">=3.6.5"

src/utils/elide-import-export.ts

@@ -1,4 +1,11 @@
 /**
+ * ----------------------------------------------------------------------------
+ * UPDATE:
+ *
+ * TODO - In next major version, we can remove this file entirely due to TS PR 57223
+ * https://github.com/microsoft/TypeScript/pull/57223
+ * ----------------------------------------------------------------------------
+ *
  * This file and its contents are due to an issue in TypeScript (affecting *at least* up to 4.1) which causes type
  * elision to break during emit for nodes which have been transformed. Specifically, if the 'original' property is set,
  * elision functionality no longer works.
@@ -9,6 +16,7 @@
  * the clause with the properly elided information
  *
  * Issues:
+ * @see https://github.com/LeDDGroup/typescript-transform-paths/issues/184
  * @see https://github.com/microsoft/TypeScript/issues/40603
  * @see https://github.com/microsoft/TypeScript/issues/31446
  *
@@ -28,15 +36,21 @@
  * import { A, B } from './b'
  * export { A } from './b'
  */
-import { ImportOrExportClause, ImportOrExportDeclaration, VisitorContext } from "../types";
+import { ImportOrExportDeclaration, VisitorContext } from "../types";
 import {
-  ExportDeclaration,
+  Debug,
+  EmitResolver,
   ExportSpecifier,
   ImportClause,
-  ImportDeclaration,
+  ImportsNotUsedAsValues,
   ImportSpecifier,
+  isInJSFile,
+  NamedExportBindings,
   NamedExports,
   NamedImportBindings,
+  NamespaceExport,
+  Node,
+  StringLiteral,
   Visitor,
   VisitResult,
 } from "typescript";
@@ -51,19 +65,21 @@ import {
  *
  * @returns import or export clause or undefined if it entire declaration should be elided
  */
-export function elideImportOrExportClause<T extends ImportOrExportDeclaration>(
+export function elideImportOrExportDeclaration<T extends ImportOrExportDeclaration>(
   context: VisitorContext,
-  node: T
-): (T extends ImportDeclaration ? ImportDeclaration["importClause"] : ExportDeclaration["exportClause"]) | undefined;
+  node: T,
+  newModuleSpecifier: StringLiteral,
+  resolver: EmitResolver
+): T | undefined;
 
-export function elideImportOrExportClause(
+export function elideImportOrExportDeclaration(
   context: VisitorContext,
-  node: ImportOrExportDeclaration
-): ImportOrExportClause | undefined {
-  const { tsInstance, transformationContext, factory } = context;
-  const resolver = transformationContext.getEmitResolver();
-  // Resolver may not be present if run manually (without Program)
-  if (!resolver) return tsInstance.isImportDeclaration(node) ? node.importClause : node.exportClause;
+  node: ImportOrExportDeclaration,
+  newModuleSpecifier: StringLiteral,
+  resolver: EmitResolver
+): ImportOrExportDeclaration | undefined {
+  const { tsInstance, factory } = context;
+  const { compilerOptions } = context;
 
   const {
     visitNode,
@@ -72,20 +88,77 @@ export function elideImportOrExportClause(
     SyntaxKind,
     visitNodes,
     isNamedExportBindings,
+    // 3.8 does not have this, so we have to define it ourselves
+    // isNamespaceExport,
+    isIdentifier,
     isExportSpecifier,
   } = tsInstance;
 
+  const isNamespaceExport = tsInstance.isNamespaceExport ?? ((node: Node): node is NamespaceExport => node.kind === SyntaxKind.NamespaceExport);
+
   if (tsInstance.isImportDeclaration(node)) {
-    if (node.importClause!.isTypeOnly) return undefined;
-    return visitNode(node.importClause, <Visitor>visitImportClause);
+    // Do not elide a side-effect only import declaration.
+    //  import "foo";
+    if (!node.importClause) return node.importClause;
+
+    // Always elide type-only imports
+    if (node.importClause.isTypeOnly) return undefined;
+
+    const importClause = visitNode(node.importClause, <Visitor>visitImportClause);
+
+    if (
+      importClause ||
+      compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
+      compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error
+    )
+      return factory.updateImportDeclaration(
+        node,
+        /*modifiers*/ undefined,
+        importClause,
+        newModuleSpecifier,
+        // This will be changed in the next release of TypeScript, but by that point we can drop elision entirely
+        (node as any).attributes || node.assertClause
+      );
+    else return undefined;
   } else {
     if (node.isTypeOnly) return undefined;
-    return visitNode(node.exportClause, <Visitor>visitNamedExports, isNamedExportBindings);
+
+    if (!node.exportClause || node.exportClause.kind === SyntaxKind.NamespaceExport) {
+      // never elide `export <whatever> from <whereever>` declarations -
+      // they should be kept for sideffects/untyped exports, even when the
+      // type checker doesn't know about any exports
+      return node;
+    }
+
+    const allowEmpty =
+      !!compilerOptions.verbatimModuleSyntax ||
+      (!!node.moduleSpecifier &&
+        (compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
+          compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error));
+
+    const exportClause = visitNode(
+      node.exportClause,
+      <Visitor>((bindings: NamedExportBindings) => visitNamedExportBindings(bindings, allowEmpty)),
+      isNamedExportBindings
+    );
+
+    return exportClause
+      ? factory.updateExportDeclaration(
+          node,
+          /*modifiers*/ undefined,
+          node.isTypeOnly,
+          exportClause,
+          newModuleSpecifier,
+          // This will be changed in the next release of TypeScript, but by that point we can drop elision entirely
+          (node as any).attributes || node.assertClause
+        )
+      : undefined;
   }
 
   /* ********************************************************* *
    * Helpers
    * ********************************************************* */
+
   // The following visitors are adapted from the TS source-base src/compiler/transformers/ts
 
   /**
@@ -95,7 +168,7 @@ export function elideImportOrExportClause(
    */
   function visitImportClause(node: ImportClause): VisitResult<ImportClause> {
     // Elide the import clause if we elide both its name and its named bindings.
-    const name = resolver.isReferencedAliasDeclaration(node) ? node.name : undefined;
+    const name = shouldEmitAliasDeclaration(node) ? node.name : undefined;
     const namedBindings = visitNode(node.namedBindings, <Visitor>visitNamedImportBindings, isNamedImportBindings);
     return name || namedBindings
       ? factory.updateImportClause(node, /*isTypeOnly*/ false, name, namedBindings)
@@ -110,11 +183,17 @@ export function elideImportOrExportClause(
   function visitNamedImportBindings(node: NamedImportBindings): VisitResult<NamedImportBindings> {
     if (node.kind === SyntaxKind.NamespaceImport) {
       // Elide a namespace import if it is not referenced.
-      return resolver.isReferencedAliasDeclaration(node) ? node : undefined;
+      return shouldEmitAliasDeclaration(node) ? node : undefined;
     } else {
       // Elide named imports if all of its import specifiers are elided.
+      const allowEmpty =
+        compilerOptions.verbatimModuleSyntax ||
+        (compilerOptions.preserveValueImports &&
+          (compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
+            compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error));
+
       const elements = visitNodes(node.elements, <Visitor>visitImportSpecifier, isImportSpecifier);
-      return tsInstance.some(elements) ? factory.updateNamedImports(node, elements) : undefined;
+      return allowEmpty || tsInstance.some(elements) ? factory.updateNamedImports(node, elements) : undefined;
     }
   }
 
@@ -125,19 +204,30 @@ export function elideImportOrExportClause(
    */
   function visitImportSpecifier(node: ImportSpecifier): VisitResult<ImportSpecifier> {
     // Elide an import specifier if it is not referenced.
-    return resolver.isReferencedAliasDeclaration(node) ? node : undefined;
+    return !node.isTypeOnly && shouldEmitAliasDeclaration(node) ? node : undefined;
   }
 
   /**
    * Visits named exports, eliding it if it does not contain an export specifier that
    * resolves to a value.
-   *
-   * @param node The named exports node.
    */
-  function visitNamedExports(node: NamedExports): VisitResult<NamedExports> {
+  function visitNamedExports(node: NamedExports, allowEmpty: boolean): VisitResult<NamedExports> | undefined {
     // Elide the named exports if all of its export specifiers were elided.
     const elements = visitNodes(node.elements, <Visitor>visitExportSpecifier, isExportSpecifier);
-    return tsInstance.some(elements) ? factory.updateNamedExports(node, elements) : undefined;
+    return allowEmpty || tsInstance.some(elements) ? factory.updateNamedExports(node, elements) : undefined;
+  }
+
+  function visitNamedExportBindings(
+    node: NamedExportBindings,
+    allowEmpty: boolean
+  ): VisitResult<NamedExportBindings> | undefined {
+    return isNamespaceExport(node) ? visitNamespaceExports(node) : visitNamedExports(node, allowEmpty);
+  }
+
+  function visitNamespaceExports(node: NamespaceExport): VisitResult<NamespaceExport> {
+    // Note: This may not work entirely properly, more likely it's just extraneous, but this won't matter soon,
+    // as we'll be removing elision entirely
+    return factory.updateNamespaceExport(node, Debug.checkDefined(visitNode(node.name, (n) => n, isIdentifier)));
   }
 
   /**
@@ -147,7 +237,19 @@ export function elideImportOrExportClause(
    */
   function visitExportSpecifier(node: ExportSpecifier): VisitResult<ExportSpecifier> {
     // Elide an export specifier if it does not reference a value.
-    return resolver.isValueAliasDeclaration(node) ? node : undefined;
+    return !node.isTypeOnly && (compilerOptions.verbatimModuleSyntax || resolver.isValueAliasDeclaration(node))
+      ? node
+      : undefined;
+  }
+
+  function shouldEmitAliasDeclaration(node: Node): boolean {
+    return (
+      !!compilerOptions.verbatimModuleSyntax ||
+      isInJSFile(node) ||
+      (compilerOptions.preserveValueImports
+        ? resolver.isValueAliasDeclaration(node)
+        : resolver.isReferencedAliasDeclaration(node))
+    );
   }
 }
 

src/visitor.ts

@@ -1,6 +1,6 @@
 import ts from "typescript";
 import { VisitorContext } from "./types";
-import { elideImportOrExportClause, resolvePathAndUpdateNode } from "./utils";
+import { elideImportOrExportDeclaration, resolvePathAndUpdateNode } from "./utils";
 
 /* ****************************************************************************************************************** *
  * Helpers
@@ -108,15 +108,16 @@ export function nodeVisitor(this: VisitorContext, node: ts.Node): ts.Node | unde
    */
   if (tsInstance.isImportDeclaration(node) && node.moduleSpecifier && tsInstance.isStringLiteral(node.moduleSpecifier))
     return resolvePathAndUpdateNode(this, node, node.moduleSpecifier.text, (p) => {
-      let importClause = node.importClause;
-
-      if (!this.isDeclarationFile && importClause?.namedBindings) {
-        const updatedImportClause = elideImportOrExportClause(this, node);
-        if (!updatedImportClause) return undefined; // No imports left, elide entire declaration
-        importClause = updatedImportClause;
+      // TODO - In next major version, we can remove this entirely due to TS PR 57223
+      //   see: https://github.com/microsoft/TypeScript/pull/57223
+      //   We should at least skip this if doing a minor version update if the ts version is high enough to not need it
+      if (!this.isDeclarationFile && node.importClause?.namedBindings) {
+        const resolver = transformationContext.getEmitResolver();
+        // If run in "manual" mode without a Program, we won't have a resolver, so we can't elide
+        if (resolver) return elideImportOrExportDeclaration(this, node, p, resolver);
       }
 
-      return factory.updateImportDeclaration(node, node.modifiers, importClause, p, node.assertClause);
+      return factory.updateImportDeclaration(node, node.modifiers, node.importClause, p, node.assertClause);
     });
 
   /**
@@ -126,15 +127,23 @@ export function nodeVisitor(this: VisitorContext, node: ts.Node): ts.Node | unde
    */
   if (tsInstance.isExportDeclaration(node) && node.moduleSpecifier && tsInstance.isStringLiteral(node.moduleSpecifier))
     return resolvePathAndUpdateNode(this, node, node.moduleSpecifier.text, (p) => {
-      let exportClause = node.exportClause;
-
-      if (!this.isDeclarationFile && exportClause && tsInstance.isNamedExports(exportClause)) {
-        const updatedExportClause = elideImportOrExportClause(this, node);
-        if (!updatedExportClause) return undefined; // No export left, elide entire declaration
-        exportClause = updatedExportClause;
+      // TODO - In next major version, we can remove this entirely due to TS PR 57223
+      //   see: https://github.com/microsoft/TypeScript/pull/57223
+      //   We should at least skip this if doing a minor version update if the ts version is high enough to not need it
+      if (!this.isDeclarationFile && node.exportClause && tsInstance.isNamedExports(node.exportClause)) {
+        const resolver = transformationContext.getEmitResolver();
+        // If run in "manual" mode without a Program, we won't have a resolver, so we can't elide
+        if (resolver) return elideImportOrExportDeclaration(this, node, p, resolver);
       }
 
-      return factory.updateExportDeclaration(node, node.modifiers, node.isTypeOnly, exportClause, p, node.assertClause);
+      return factory.updateExportDeclaration(
+        node,
+        node.modifiers,
+        node.isTypeOnly,
+        node.exportClause,
+        p,
+        node.assertClause
+      );
     });
 
   /**