From 7588c15ec208e882c8c436b0a6b10b5d6eb61424 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 11 Dec 2017 10:18:42 -0800 Subject: [PATCH 1/2] In import fixes, use a ".js" extension if other imports do --- src/services/codefixes/importFixes.ts | 22 ++++++++++++++----- .../importNameCodeFix_jsExtension.ts | 20 +++++++++++++++++ .../fourslash/importNameCodeFix_symlink.ts | 2 +- 3 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFix_jsExtension.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 20d6ac4d5bab5..5814f2a0980dd 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -306,6 +306,10 @@ namespace ts.codefix { return literal; } + function usesJsExtensionOnImports(sourceFile: SourceFile): boolean { + return firstDefined(sourceFile.imports, ({ text }) => pathIsRelative(text) ? fileExtensionIs(text, Extension.Js) : undefined) || false; + } + function createImportClauseOfKind(kind: ImportKind.Default | ImportKind.Named | ImportKind.Namespace, symbolName: string) { const id = createIdentifier(symbolName); switch (kind) { @@ -329,18 +333,19 @@ namespace ts.codefix { host: LanguageServiceHost, ): string[] { const { baseUrl, paths, rootDirs } = options; + const addJsExtension = usesJsExtensionOnImports(sourceFile); const choicesForEachExportingModule = flatMap(moduleSymbols, moduleSymbol => getAllModulePaths(program, moduleSymbol.valueDeclaration.getSourceFile()).map(moduleFileName => { const sourceDirectory = getDirectoryPath(sourceFile.fileName); const global = tryGetModuleNameFromAmbientModule(moduleSymbol) - || tryGetModuleNameFromTypeRoots(options, host, getCanonicalFileName, moduleFileName) + || tryGetModuleNameFromTypeRoots(options, host, getCanonicalFileName, moduleFileName, addJsExtension) || tryGetModuleNameAsNodeModule(options, moduleFileName, host, getCanonicalFileName, sourceDirectory) || rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName); if (global) { return [global]; } - const relativePath = removeExtensionAndIndexPostFix(getRelativePath(moduleFileName, sourceDirectory, getCanonicalFileName), options); + const relativePath = removeExtensionAndIndexPostFix(getRelativePath(moduleFileName, sourceDirectory, getCanonicalFileName), options, addJsExtension); if (!baseUrl) { return [relativePath]; } @@ -350,7 +355,7 @@ namespace ts.codefix { return [relativePath]; } - const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, options); + const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, options, addJsExtension); if (paths) { const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths); if (fromPaths) { @@ -459,12 +464,13 @@ namespace ts.codefix { host: GetEffectiveTypeRootsHost, getCanonicalFileName: (file: string) => string, moduleFileName: string, + addJsExtension: boolean, ): string | undefined { const roots = getEffectiveTypeRoots(options, host); return roots && firstDefined(roots, unNormalizedTypeRoot => { const typeRoot = toPath(unNormalizedTypeRoot, /*basePath*/ undefined, getCanonicalFileName); if (startsWith(moduleFileName, typeRoot)) { - return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), options); + return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), options, addJsExtension); } }); } @@ -598,9 +604,13 @@ namespace ts.codefix { return firstDefined(rootDirs, rootDir => getRelativePathIfInDirectory(path, rootDir, getCanonicalFileName)); } - function removeExtensionAndIndexPostFix(fileName: string, options: CompilerOptions): string { + function removeExtensionAndIndexPostFix(fileName: string, options: CompilerOptions, addJsExtension: boolean): string { const noExtension = removeFileExtension(fileName); - return getEmitModuleResolutionKind(options) === ModuleResolutionKind.NodeJs ? removeSuffix(noExtension, "/index") : noExtension; + return addJsExtension + ? noExtension + ".js" + : getEmitModuleResolutionKind(options) === ModuleResolutionKind.NodeJs + ? removeSuffix(noExtension, "/index") + : noExtension; } function getRelativePathIfInDirectory(path: string, directoryPath: string, getCanonicalFileName: GetCanonicalFileName): string | undefined { diff --git a/tests/cases/fourslash/importNameCodeFix_jsExtension.ts b/tests/cases/fourslash/importNameCodeFix_jsExtension.ts new file mode 100644 index 0000000000000..b817dd3395721 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_jsExtension.ts @@ -0,0 +1,20 @@ +/// + +// @moduleResolution: node +// @noLib: true + +// @Filename: /a.ts +////export function a() {} + +// @Filename: /b.ts +////export function b() {} + +// @Filename: /c.ts +////import { a } from "./a.js"; +////[|b;|] + +goTo.file("/c.ts"); +verify.importFixAtPosition([ +`import { b } from "./b.js"; +b;`, +]); diff --git a/tests/cases/fourslash/importNameCodeFix_symlink.ts b/tests/cases/fourslash/importNameCodeFix_symlink.ts index 8da44de7797a3..1fcd231b23d9f 100644 --- a/tests/cases/fourslash/importNameCodeFix_symlink.ts +++ b/tests/cases/fourslash/importNameCodeFix_symlink.ts @@ -11,7 +11,7 @@ ////import { foo } from "link"; // @Filename: /b.ts -////[|foo/**/;|] +////[|foo;|] // Uses "link" instead of "real" because `a` did. goTo.file("/b.ts"); From a2bd053539ba8d240f742063f501775c42a0ff95 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 3 Jan 2018 07:27:39 -0800 Subject: [PATCH 2/2] Code review --- src/services/codefixes/importFixes.ts | 4 ++-- tests/cases/fourslash/importNameCodeFix_jsExtension.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 5814f2a0980dd..a69152ad2b5ff 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -609,8 +609,8 @@ namespace ts.codefix { return addJsExtension ? noExtension + ".js" : getEmitModuleResolutionKind(options) === ModuleResolutionKind.NodeJs - ? removeSuffix(noExtension, "/index") - : noExtension; + ? removeSuffix(noExtension, "/index") + : noExtension; } function getRelativePathIfInDirectory(path: string, directoryPath: string, getCanonicalFileName: GetCanonicalFileName): string | undefined { diff --git a/tests/cases/fourslash/importNameCodeFix_jsExtension.ts b/tests/cases/fourslash/importNameCodeFix_jsExtension.ts index b817dd3395721..98c7c9e6cad28 100644 --- a/tests/cases/fourslash/importNameCodeFix_jsExtension.ts +++ b/tests/cases/fourslash/importNameCodeFix_jsExtension.ts @@ -10,7 +10,9 @@ ////export function b() {} // @Filename: /c.ts +////import * as g from "global"; // Global imports skipped ////import { a } from "./a.js"; +////import { a as a2 } from "./a"; // Ignored, only the first relative import is considered ////[|b;|] goTo.file("/c.ts");