ts-types transformer: update importers with naming collisions (#5968)

Author: mischnic

packages/core/integration-tests/test/integration/ts-types/importing-collision/expected.d.ts

@@ -0,0 +1,5 @@
+declare function _log1(message: string): void;
+declare function xyz(message: number): void;
+export function log(f: typeof _log1 | typeof xyz): void;
+
+//# sourceMappingURL=types.d.ts.map

packages/core/integration-tests/test/integration/ts-types/importing-collision/index.ts

@@ -0,0 +1,7 @@
+import { log as logFn1 } from "./other1";
+import { log as logFn2 } from "./other2";
+
+export function log(f: typeof logFn1 | typeof logFn2) {
+  logFn1("1");
+  logFn2(1);
+}

packages/core/integration-tests/test/integration/ts-types/importing-collision/other1.ts

@@ -0,0 +1 @@
+export function log(message: string) {}

packages/core/integration-tests/test/integration/ts-types/importing-collision/other2.ts

@@ -0,0 +1,2 @@
+function xyz(message: number) {}
+export { xyz as log };

packages/core/integration-tests/test/integration/ts-types/importing-collision/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "ts-types-importing",
+  "private": true,
+  "main": "dist/main.js",
+  "types": "dist/types.d.ts"
+}

packages/core/integration-tests/test/integration/ts-types/importing-collision/yarn.lock

@@ -68,6 +68,44 @@ describe('typescript types', function() {
     assert.equal(dist, expected);
   });
 
+  it('should generate ts declarations with imports and naming collisions', async function() {
+    let b = await bundle(
+      path.join(
+        __dirname,
+        '/integration/ts-types/importing-collision/index.ts',
+      ),
+    );
+
+    assertBundles(b, [
+      {
+        type: 'js',
+        assets: ['index.ts', 'other1.ts', 'other2.ts', 'esmodule-helpers.js'],
+      },
+      {
+        type: 'ts',
+        assets: ['index.ts'],
+      },
+    ]);
+
+    let dist = (
+      await outputFS.readFile(
+        path.join(
+          __dirname,
+          '/integration/ts-types/importing-collision/dist/types.d.ts',
+        ),
+        'utf8',
+      )
+    ).replace(/\r\n/g, '\n');
+    let expected = await inputFS.readFile(
+      path.join(
+        __dirname,
+        '/integration/ts-types/importing-collision/expected.d.ts',
+      ),
+      'utf8',
+    );
+    assert.equal(dist, expected);
+  });
+
   it('should generate ts declarations with exports', async function() {
     let b = await bundle(
       path.join(__dirname, '/integration/ts-types/exporting/index.ts'),

packages/core/integration-tests/test/ts-types.js

@@ -23,10 +23,11 @@ export class TSModule {
   addImport(local: string, specifier: string, imported: string) {
     this.imports.set(local, {specifier, imported});
     if (imported !== '*' && imported !== 'default') {
-      this.names.set(local, imported);
+      this.names.set(local, local);
     }
   }
 
+  // if not a reexport: imported = local, name = exported
   addExport(name: string, imported: string, specifier: ?string) {
     this.exports.push({name, specifier, imported});
   }

packages/transformers/typescript-types/src/TSModule.js

@@ -1,17 +1,16 @@
 // @flow
 import type {TSModule, Export} from './TSModule';
-import typeof TypeScriptModule from 'typescript'; // eslint-disable-line import/no-extraneous-dependencies
+
 import nullthrows from 'nullthrows';
 import invariant from 'assert';
+import ts from 'typescript';
 
 export class TSModuleGraph {
-  ts: TypeScriptModule;
   modules: Map<string, TSModule>;
   mainModuleName: string;
   mainModule: ?TSModule;
 
-  constructor(ts: TypeScriptModule, mainModuleName: string) {
-    this.ts = ts;
+  constructor(mainModuleName: string) {
     this.modules = new Map();
     this.mainModuleName = mainModuleName;
     this.mainModule = null;
@@ -29,18 +28,16 @@ export class TSModuleGraph {
   }
 
   markUsed(module: TSModule, name: string, context: any): void {
-    let {ts} = this;
-
     // If name is imported, mark used in the original module
     if (module.imports.has(name)) {
       module.used.add(name);
-      let {specifier, imported} = nullthrows(module.imports.get(name));
-      let m = this.getModule(specifier);
-      if (!m) {
+      let resolved = this.resolveImport(module, name);
+      // Missing or external
+      if (!resolved || resolved.module === module) {
         return;
       }
 
-      return this.markUsed(m, imported, context);
+      return this.markUsed(resolved.module, resolved.imported, context);
     }
 
     if (module.used.has(name)) {
@@ -112,8 +109,8 @@ export class TSModuleGraph {
     // Named export
     return {
       module: m,
-      name: m.getName(exportName),
-      imported: e.imported || exportName,
+      name: exportName,
+      imported: e.imported != null ? m.getName(e.imported) : exportName,
     };
   }
 
@@ -205,14 +202,22 @@ export class TSModuleGraph {
       exportedNames.set(e.name, e.module);
     }
 
+    let importedSymbolsToUpdate = [];
+
     // Assign unique names across all modules
     for (let m of this.modules.values()) {
       for (let [orig, name] of m.names) {
         if (exportedNames.has(name) && exportedNames.get(name) === m) {
           continue;
         }
 
-        if (!m.used.has(orig) || m.imports.get(orig)) {
+        if (!m.used.has(orig)) {
+          continue;
+        }
+
+        if (m.imports.has(orig)) {
+          // Update imports after all modules's local variables have been renamed
+          importedSymbolsToUpdate.push([m, orig]);
           continue;
         }
 
@@ -224,6 +229,11 @@ export class TSModuleGraph {
       }
     }
 
+    for (let [m, orig] of importedSymbolsToUpdate) {
+      let imported = nullthrows(this.resolveImport(m, orig));
+      m.names.set(orig, imported.imported);
+    }
+
     return exportedNames;
   }
 }

packages/transformers/typescript-types/src/TSModuleGraph.js

@@ -4,8 +4,8 @@ import {Transformer} from '@parcel/plugin';
 import path from 'path';
 import SourceMap from '@parcel/source-map';
 import type {DiagnosticCodeFrame} from '@parcel/diagnostic';
-
 import type {CompilerOptions} from 'typescript';
+
 import ts from 'typescript';
 import {CompilerHost, loadTSConfig} from '@parcel/ts-utils';
 import {escapeMarkdown} from '@parcel/diagnostic';
@@ -52,17 +52,17 @@ export default (new Transformer({
     let mainModuleName = path
       .relative(program.getCommonSourceDirectory(), asset.filePath)
       .slice(0, -path.extname(asset.filePath).length);
-    let moduleGraph = new TSModuleGraph(ts, mainModuleName);
+    let moduleGraph = new TSModuleGraph(mainModuleName);
 
     let emitResult = program.emit(undefined, undefined, undefined, true, {
       afterDeclarations: [
         // 1. Build module graph
         context => sourceFile => {
-          return collect(ts, moduleGraph, context, sourceFile);
+          return collect(moduleGraph, context, sourceFile);
         },
         // 2. Tree shake and rename types
         context => sourceFile => {
-          return shake(ts, moduleGraph, context, sourceFile);
+          return shake(moduleGraph, context, sourceFile);
         },
       ],
     });

packages/transformers/typescript-types/src/TSTypesTransformer.js

@@ -1,12 +1,12 @@
 // @flow
+import type {TSModuleGraph} from './TSModuleGraph';
+
 import nullthrows from 'nullthrows';
+import ts from 'typescript';
 import {TSModule} from './TSModule';
-import type {TSModuleGraph} from './TSModuleGraph';
-import typeof TypeScriptModule from 'typescript'; // eslint-disable-line import/no-extraneous-dependencies
 import {getExportedName, isDeclaration} from './utils';
 
 export function collect(
-  ts: TypeScriptModule,
   moduleGraph: TSModuleGraph,
   context: any,
   sourceFile: any,

packages/transformers/typescript-types/src/collect.js

@@ -1,12 +1,12 @@
 // @flow
 import {TSModule} from './TSModule';
 import type {TSModuleGraph} from './TSModuleGraph';
-import typeof TypeScriptModule from 'typescript'; // eslint-disable-line import/no-extraneous-dependencies
-import {getExportedName, isDeclaration} from './utils';
+
+import ts from 'typescript';
 import nullthrows from 'nullthrows';
+import {getExportedName, isDeclaration} from './utils';
 
 export function shake(
-  ts: TypeScriptModule,
   moduleGraph: TSModuleGraph,
   context: any,
   sourceFile: any,
@@ -32,7 +32,7 @@ export function shake(
       let statements = ts.visitEachChild(node, visit, context).body.statements;
 
       if (isFirstModule) {
-        statements.unshift(...generateImports(ts, moduleGraph));
+        statements.unshift(...generateImports(moduleGraph));
       }
 
       return statements;
@@ -207,7 +207,7 @@ export function shake(
   return ts.visitNode(sourceFile, visit);
 }
 
-function generateImports(ts: TypeScriptModule, moduleGraph: TSModuleGraph) {
+function generateImports(moduleGraph: TSModuleGraph) {
   let importStatements = [];
   for (let [specifier, names] of moduleGraph.getAllImports()) {
     let defaultSpecifier;