From ece640da517c27d189ebdc601750715cd99b674f Mon Sep 17 00:00:00 2001 From: Philip Nuzhnyi Date: Mon, 3 Feb 2025 16:42:22 +0000 Subject: [PATCH] correcly handle imports with aliases --- deno.json | 2 +- jurassic/utils.test.ts | 103 +++++++++++++++++-------------- jurassic/utils.ts | 99 ++++++++++++++++-------------- nbs/utils.ipynb | 136 +++++++++++++++++++---------------------- 4 files changed, 173 insertions(+), 167 deletions(-) diff --git a/deno.json b/deno.json index 7a93be5..4e5d8ce 100644 --- a/deno.json +++ b/deno.json @@ -1,6 +1,6 @@ { "name": "@jurassic/jurassic", - "version": "0.1.49", + "version": "0.1.50", "license": "MIT", "tasks": { "build": "deno run -A --reload jsr:@jurassic/jurassic/export . && deno task runnbs && deno check . && deno lint && deno fmt && deno task clean && deno test --allow-all --env-file", diff --git a/jurassic/utils.test.ts b/jurassic/utils.test.ts index c756330..f05e084 100644 --- a/jurassic/utils.test.ts +++ b/jurassic/utils.test.ts @@ -266,17 +266,14 @@ export const removeDuplicateImports = (sourceCode: string): string => { ts.ScriptTarget.Latest, true, ); - const imports = new Map< - string, - { - specifiers: Set; - defaultName?: string; - namespaceImport?: string; - typeOnlySpecifiers: Set; - isBareImport?: boolean; - } - >(); const importNodes: ts.ImportDeclaration[] = []; + const imports = new Map; // name -> alias + defaultName?: string; + namespaceImport?: string; + typeOnlySpecifiers: Map; // name -> alias + isBareImport?: boolean; + }>(); function visit(node: ts.Node) { if (ts.isImportDeclaration(node)) { @@ -285,47 +282,54 @@ export const removeDuplicateImports = (sourceCode: string): string => { if (!imports.has(source)) { imports.set(source, { - specifiers: new Set(), - typeOnlySpecifiers: new Set(), + specifiers: new Map(), + typeOnlySpecifiers: new Map(), isBareImport: !node.importClause, }); } - // Handle bare imports if (!node.importClause) { imports.get(source)!.isBareImport = true; return; } - if (node.importClause) { - const isTypeOnly = node.importClause.isTypeOnly; - - if ( - node.importClause.namedBindings && - ts.isNamespaceImport(node.importClause.namedBindings) - ) { - imports.get(source)!.namespaceImport = - node.importClause.namedBindings.name.text; - } else if ( - node.importClause.namedBindings && - ts.isNamedImports(node.importClause.namedBindings) - ) { - node.importClause.namedBindings.elements.forEach((element) => { - if (isTypeOnly || element.isTypeOnly) { - imports.get(source)?.typeOnlySpecifiers.add(element.name.text); + const isTypeOnly = node.importClause.isTypeOnly; + + if ( + node.importClause.namedBindings && + ts.isNamespaceImport(node.importClause.namedBindings) + ) { + imports.get(source)!.namespaceImport = + node.importClause.namedBindings.name.text; + } else if ( + node.importClause.namedBindings && + ts.isNamedImports(node.importClause.namedBindings) + ) { + node.importClause.namedBindings.elements.forEach((element) => { + const name = element.propertyName?.text || element.name.text; + const alias = element.propertyName ? element.name.text : undefined; + + if (isTypeOnly || element.isTypeOnly) { + if (alias) { + imports.get(source)?.typeOnlySpecifiers.set(name, alias); } else { - imports.get(source)?.specifiers.add(element.name.text); + imports.get(source)?.typeOnlySpecifiers.set(name, name); } - }); - } - if (node.importClause.name) { - imports.get(source)!.defaultName = node.importClause.name.text; - if (isTypeOnly) { - imports.get(source)?.typeOnlySpecifiers.add("default"); } else { - imports.get(source)?.specifiers.add("default"); + if (alias) { + imports.get(source)?.specifiers.set(name, alias); + } else { + imports.get(source)?.specifiers.set(name, name); + } } - } + }); + } + if (node.importClause.name) { + imports.get(source)!.defaultName = node.importClause.name.text; + const target = isTypeOnly + ? imports.get(source)?.typeOnlySpecifiers + : imports.get(source)?.specifiers; + target?.set("default", "default"); } } ts.forEachChild(node, visit); @@ -346,29 +350,32 @@ export const removeDuplicateImports = (sourceCode: string): string => { }, ] of imports ) { - // Handle bare imports first if (isBareImport) { newImports.push(`import '${source}';`); continue; } - // Handle type-only imports if (typeOnlySpecifiers.size > 0) { - let importStr = "import type "; - const namedImports = Array.from(typeOnlySpecifiers).sort().join(", "); - importStr += `{ ${namedImports} }`; - importStr += ` from '${source}';`; + let importStr = "import type { "; + const namedImports = Array.from(typeOnlySpecifiers.entries()) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([name, alias]) => name === alias ? name : `${name} as ${alias}`) + .join(", "); + importStr += namedImports; + importStr += ` } from '${source}';`; newImports.push(importStr); } - // Handle regular imports if (specifiers.size > 0 || namespaceImport) { let importStr = "import "; if (namespaceImport) { importStr += `* as ${namespaceImport}`; } else { const hasDefault = specifiers.delete("default"); - const namedImports = Array.from(specifiers).sort().join(", "); + const namedImports = Array.from(specifiers.entries()) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([name, alias]) => name === alias ? name : `${name} as ${alias}`) + .join(", "); if (hasDefault && defaultName) { importStr += `${defaultName} `; @@ -414,6 +421,7 @@ Deno.test("findDenoTests", () => { Deno.test("removeDuplicateImports", () => { assertEquals( removeDuplicateImports(` +import { Record as Message } from "tinychat/api/types/chat/tinychat/core/message.ts"; import type { Config } from "jurassic/config.ts"; import * as ts from "typescript"; import { assertEquals } from "jsr:@std/assert"; @@ -427,7 +435,8 @@ import path from "node:path"; Deno.test("t2", () => {}); `), - `import type { Config } from 'jurassic/config.ts'; + `import { Record as Message } from 'tinychat/api/types/chat/tinychat/core/message.ts'; +import type { Config } from 'jurassic/config.ts'; import * as ts from 'typescript'; import { assertEquals } from 'jsr:@std/assert'; import path from 'node:path'; diff --git a/jurassic/utils.ts b/jurassic/utils.ts index add595a..57a7b23 100644 --- a/jurassic/utils.ts +++ b/jurassic/utils.ts @@ -266,17 +266,14 @@ export const removeDuplicateImports = (sourceCode: string): string => { ts.ScriptTarget.Latest, true, ); - const imports = new Map< - string, - { - specifiers: Set; - defaultName?: string; - namespaceImport?: string; - typeOnlySpecifiers: Set; - isBareImport?: boolean; - } - >(); const importNodes: ts.ImportDeclaration[] = []; + const imports = new Map; // name -> alias + defaultName?: string; + namespaceImport?: string; + typeOnlySpecifiers: Map; // name -> alias + isBareImport?: boolean; + }>(); function visit(node: ts.Node) { if (ts.isImportDeclaration(node)) { @@ -285,47 +282,54 @@ export const removeDuplicateImports = (sourceCode: string): string => { if (!imports.has(source)) { imports.set(source, { - specifiers: new Set(), - typeOnlySpecifiers: new Set(), + specifiers: new Map(), + typeOnlySpecifiers: new Map(), isBareImport: !node.importClause, }); } - // Handle bare imports if (!node.importClause) { imports.get(source)!.isBareImport = true; return; } - if (node.importClause) { - const isTypeOnly = node.importClause.isTypeOnly; - - if ( - node.importClause.namedBindings && - ts.isNamespaceImport(node.importClause.namedBindings) - ) { - imports.get(source)!.namespaceImport = - node.importClause.namedBindings.name.text; - } else if ( - node.importClause.namedBindings && - ts.isNamedImports(node.importClause.namedBindings) - ) { - node.importClause.namedBindings.elements.forEach((element) => { - if (isTypeOnly || element.isTypeOnly) { - imports.get(source)?.typeOnlySpecifiers.add(element.name.text); + const isTypeOnly = node.importClause.isTypeOnly; + + if ( + node.importClause.namedBindings && + ts.isNamespaceImport(node.importClause.namedBindings) + ) { + imports.get(source)!.namespaceImport = + node.importClause.namedBindings.name.text; + } else if ( + node.importClause.namedBindings && + ts.isNamedImports(node.importClause.namedBindings) + ) { + node.importClause.namedBindings.elements.forEach((element) => { + const name = element.propertyName?.text || element.name.text; + const alias = element.propertyName ? element.name.text : undefined; + + if (isTypeOnly || element.isTypeOnly) { + if (alias) { + imports.get(source)?.typeOnlySpecifiers.set(name, alias); } else { - imports.get(source)?.specifiers.add(element.name.text); + imports.get(source)?.typeOnlySpecifiers.set(name, name); } - }); - } - if (node.importClause.name) { - imports.get(source)!.defaultName = node.importClause.name.text; - if (isTypeOnly) { - imports.get(source)?.typeOnlySpecifiers.add("default"); } else { - imports.get(source)?.specifiers.add("default"); + if (alias) { + imports.get(source)?.specifiers.set(name, alias); + } else { + imports.get(source)?.specifiers.set(name, name); + } } - } + }); + } + if (node.importClause.name) { + imports.get(source)!.defaultName = node.importClause.name.text; + const target = isTypeOnly + ? imports.get(source)?.typeOnlySpecifiers + : imports.get(source)?.specifiers; + target?.set("default", "default"); } } ts.forEachChild(node, visit); @@ -346,29 +350,32 @@ export const removeDuplicateImports = (sourceCode: string): string => { }, ] of imports ) { - // Handle bare imports first if (isBareImport) { newImports.push(`import '${source}';`); continue; } - // Handle type-only imports if (typeOnlySpecifiers.size > 0) { - let importStr = "import type "; - const namedImports = Array.from(typeOnlySpecifiers).sort().join(", "); - importStr += `{ ${namedImports} }`; - importStr += ` from '${source}';`; + let importStr = "import type { "; + const namedImports = Array.from(typeOnlySpecifiers.entries()) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([name, alias]) => name === alias ? name : `${name} as ${alias}`) + .join(", "); + importStr += namedImports; + importStr += ` } from '${source}';`; newImports.push(importStr); } - // Handle regular imports if (specifiers.size > 0 || namespaceImport) { let importStr = "import "; if (namespaceImport) { importStr += `* as ${namespaceImport}`; } else { const hasDefault = specifiers.delete("default"); - const namedImports = Array.from(specifiers).sort().join(", "); + const namedImports = Array.from(specifiers.entries()) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([name, alias]) => name === alias ? name : `${name} as ${alias}`) + .join(", "); if (hasDefault && defaultName) { importStr += `${defaultName} `; diff --git a/nbs/utils.ipynb b/nbs/utils.ipynb index 2f09e62..cf5492d 100644 --- a/nbs/utils.ipynb +++ b/nbs/utils.ipynb @@ -578,72 +578,70 @@ "//| export\n", "\n", "export const removeDuplicateImports = (sourceCode: string): string => {\n", - " const sourceFile = ts.createSourceFile(\n", - " \"temp.ts\",\n", - " sourceCode,\n", - " ts.ScriptTarget.Latest,\n", - " true,\n", - " );\n", - " const imports = new Map<\n", - " string,\n", - " {\n", - " specifiers: Set;\n", - " defaultName?: string;\n", - " namespaceImport?: string;\n", - " typeOnlySpecifiers: Set;\n", - " isBareImport?: boolean;\n", - " }\n", - " >();\n", - " const importNodes: ts.ImportDeclaration[] = [];\n", + " const sourceFile = ts.createSourceFile(\n", + " \"temp.ts\",\n", + " sourceCode,\n", + " ts.ScriptTarget.Latest,\n", + " true\n", + " );\n", + " const importNodes: ts.ImportDeclaration[] = [];\n", + " const imports = new Map, // name -> alias\n", + " defaultName?: string,\n", + " namespaceImport?: string,\n", + " typeOnlySpecifiers: Map, // name -> alias\n", + " isBareImport?: boolean\n", + " }>();\n", "\n", " function visit(node: ts.Node) {\n", " if (ts.isImportDeclaration(node)) {\n", " importNodes.push(node);\n", " const source = node.moduleSpecifier.getText().replace(/['\"]/g, \"\");\n", - "\n", + " \n", " if (!imports.has(source)) {\n", - " imports.set(source, {\n", - " specifiers: new Set(),\n", - " typeOnlySpecifiers: new Set(),\n", - " isBareImport: !node.importClause,\n", + " imports.set(source, { \n", + " specifiers: new Map(),\n", + " typeOnlySpecifiers: new Map(),\n", + " isBareImport: !node.importClause\n", " });\n", " }\n", "\n", - " // Handle bare imports\n", " if (!node.importClause) {\n", " imports.get(source)!.isBareImport = true;\n", " return;\n", " }\n", "\n", - " if (node.importClause) {\n", - " const isTypeOnly = node.importClause.isTypeOnly;\n", - "\n", - " if (\n", - " node.importClause.namedBindings &&\n", - " ts.isNamespaceImport(node.importClause.namedBindings)\n", - " ) {\n", - " imports.get(source)!.namespaceImport =\n", - " node.importClause.namedBindings.name.text;\n", - " } else if (\n", - " node.importClause.namedBindings &&\n", - " ts.isNamedImports(node.importClause.namedBindings)\n", - " ) {\n", - " node.importClause.namedBindings.elements.forEach((element) => {\n", - " if (isTypeOnly || element.isTypeOnly) {\n", - " imports.get(source)?.typeOnlySpecifiers.add(element.name.text);\n", + " const isTypeOnly = node.importClause.isTypeOnly;\n", + " \n", + " if (node.importClause.namedBindings && \n", + " ts.isNamespaceImport(node.importClause.namedBindings)) {\n", + " imports.get(source)!.namespaceImport = node.importClause.namedBindings.name.text;\n", + " }\n", + " else if (node.importClause.namedBindings && \n", + " ts.isNamedImports(node.importClause.namedBindings)) {\n", + " node.importClause.namedBindings.elements.forEach((element) => {\n", + " const name = element.propertyName?.text || element.name.text;\n", + " const alias = element.propertyName ? element.name.text : undefined;\n", + " \n", + " if (isTypeOnly || element.isTypeOnly) {\n", + " if (alias) {\n", + " imports.get(source)?.typeOnlySpecifiers.set(name, alias);\n", " } else {\n", - " imports.get(source)?.specifiers.add(element.name.text);\n", + " imports.get(source)?.typeOnlySpecifiers.set(name, name);\n", " }\n", - " });\n", - " }\n", - " if (node.importClause.name) {\n", - " imports.get(source)!.defaultName = node.importClause.name.text;\n", - " if (isTypeOnly) {\n", - " imports.get(source)?.typeOnlySpecifiers.add(\"default\");\n", " } else {\n", - " imports.get(source)?.specifiers.add(\"default\");\n", + " if (alias) {\n", + " imports.get(source)?.specifiers.set(name, alias);\n", + " } else {\n", + " imports.get(source)?.specifiers.set(name, name);\n", + " }\n", " }\n", - " }\n", + " });\n", + " }\n", + " if (node.importClause.name) {\n", + " imports.get(source)!.defaultName = node.importClause.name.text;\n", + " const target = isTypeOnly ? imports.get(source)?.typeOnlySpecifiers : imports.get(source)?.specifiers;\n", + " target?.set(\"default\", \"default\");\n", " }\n", " }\n", " ts.forEachChild(node, visit);\n", @@ -652,49 +650,39 @@ " visit(sourceFile);\n", "\n", " const newImports: string[] = [];\n", - " for (\n", - " const [\n", - " source,\n", - " {\n", - " specifiers,\n", - " typeOnlySpecifiers,\n", - " defaultName,\n", - " namespaceImport,\n", - " isBareImport,\n", - " },\n", - " ] of imports\n", - " ) {\n", - " // Handle bare imports first\n", + " for (const [source, { specifiers, typeOnlySpecifiers, defaultName, namespaceImport, isBareImport }] of imports) {\n", " if (isBareImport) {\n", " newImports.push(`import '${source}';`);\n", " continue;\n", " }\n", "\n", - " // Handle type-only imports\n", " if (typeOnlySpecifiers.size > 0) {\n", - " let importStr = \"import type \";\n", - " const namedImports = Array.from(typeOnlySpecifiers).sort().join(\", \");\n", - " importStr += `{ ${namedImports} }`;\n", - " importStr += ` from '${source}';`;\n", + " let importStr = \"import type { \";\n", + " const namedImports = Array.from(typeOnlySpecifiers.entries())\n", + " .sort(([a], [b]) => a.localeCompare(b))\n", + " .map(([name, alias]) => name === alias ? name : `${name} as ${alias}`)\n", + " .join(\", \");\n", + " importStr += namedImports;\n", + " importStr += ` } from '${source}';`;\n", " newImports.push(importStr);\n", " }\n", "\n", - " // Handle regular imports\n", " if (specifiers.size > 0 || namespaceImport) {\n", " let importStr = \"import \";\n", " if (namespaceImport) {\n", " importStr += `* as ${namespaceImport}`;\n", " } else {\n", " const hasDefault = specifiers.delete(\"default\");\n", - " const namedImports = Array.from(specifiers).sort().join(\", \");\n", + " const namedImports = Array.from(specifiers.entries())\n", + " .sort(([a], [b]) => a.localeCompare(b))\n", + " .map(([name, alias]) => name === alias ? name : `${name} as ${alias}`)\n", + " .join(\", \");\n", "\n", " if (hasDefault && defaultName) {\n", " importStr += `${defaultName} `;\n", " }\n", " if (namedImports) {\n", - " importStr += hasDefault\n", - " ? `, { ${namedImports} }`\n", - " : `{ ${namedImports} }`;\n", + " importStr += hasDefault ? `, { ${namedImports} }` : `{ ${namedImports} }`;\n", " }\n", " }\n", " importStr += ` from '${source}';`;\n", @@ -723,6 +711,7 @@ "Deno.test(\"removeDuplicateImports\", () => {\n", " assertEquals(\n", " removeDuplicateImports(`\n", + "import { Record as Message } from \"tinychat/api/types/chat/tinychat/core/message.ts\"; \n", "import type { Config } from \"jurassic/config.ts\"; \n", "import * as ts from \"typescript\"; \n", "import { assertEquals } from \"jsr:@std/assert\";\n", @@ -736,7 +725,8 @@ "\n", "Deno.test(\"t2\", () => {});\n", "`),\n", - " `import type { Config } from 'jurassic/config.ts';\n", + " `import { Record as Message } from 'tinychat/api/types/chat/tinychat/core/message.ts';\n", + "import type { Config } from 'jurassic/config.ts';\n", "import * as ts from 'typescript';\n", "import { assertEquals } from 'jsr:@std/assert';\n", "import path from 'node:path';\n", @@ -746,7 +736,7 @@ "\n", "\n", "\n", - "Deno.test(\"t2\", () => {});`,\n", + "Deno.test(\"t2\", () => {});`\n", " );\n", "});" ],