diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index d32bf92b007..c6ec0f257b6 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2405,7 +2405,7 @@ namespace FourSlash { const sortedActualArray = actualTextArray.sort(); if (!ts.arrayIsEqualTo(sortedExpectedArray, sortedActualArray)) { this.raiseError( - `Actual text array doesn't match expected text array. \nActual: \n"${sortedActualArray.join("\n\n")}"\n---\nExpected: \n'${sortedExpectedArray.join("\n\n")}'`); + `Actual text array doesn't match expected text array. \nActual: \n'${sortedActualArray.join("\n\n")}'\n---\nExpected: \n'${sortedExpectedArray.join("\n\n")}'`); } } diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 2be20f35c16..cbf7211ecb5 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -501,45 +501,113 @@ namespace ts.codefix { return undefined; } - const indexOfNodeModules = moduleFileName.indexOf("node_modules"); - if (indexOfNodeModules < 0) { + const parts = getNodeModulePathParts(moduleFileName); + + if (!parts) { return undefined; } - let relativeFileName: string; - if (sourceDirectory.indexOf(moduleFileName.substring(0, indexOfNodeModules - 1)) === 0) { - // if node_modules folder is in this folder or any of its parent folder, no need to keep it. - relativeFileName = moduleFileName.substring(indexOfNodeModules + 13 /* "node_modules\".length */); - } - else { - relativeFileName = getRelativePath(moduleFileName, sourceDirectory); - } + // Simplify the full file path to something that can be resolved by Node. - relativeFileName = removeFileExtension(relativeFileName); - if (endsWith(relativeFileName, "/index")) { - relativeFileName = getDirectoryPath(relativeFileName); - } - else { - try { - const moduleDirectory = getDirectoryPath(moduleFileName); - const packageJsonContent = JSON.parse(context.host.readFile(combinePaths(moduleDirectory, "package.json"))); + // If the module could be imported by a directory name, use that directory's name + let moduleSpecifier = getDirectoryOrExtensionlessFileName(moduleFileName); + // Get a path that's relative to node_modules or the importing file's path + moduleSpecifier = getNodeResolvablePath(moduleSpecifier); + // If the module was found in @types, get the actual Node package name + return getPackageNameFromAtTypesDirectory(moduleSpecifier); + + function getDirectoryOrExtensionlessFileName(path: string): string { + // If the file is the main module, it can be imported by the package name + const packageRootPath = path.substring(0, parts.packageRootIndex); + const packageJsonPath = combinePaths(packageRootPath, "package.json"); + if (context.host.fileExists(packageJsonPath)) { + const packageJsonContent = JSON.parse(context.host.readFile(packageJsonPath)); if (packageJsonContent) { - const mainFile = packageJsonContent.main || packageJsonContent.typings; - if (mainFile) { - const mainExportFile = toPath(mainFile, moduleDirectory, getCanonicalFileName); - if (removeFileExtension(mainExportFile) === removeFileExtension(moduleFileName)) { - relativeFileName = getDirectoryPath(relativeFileName); + const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main; + if (mainFileRelative) { + const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName); + if (mainExportFile === getCanonicalFileName(path)) { + return packageRootPath; } } } } - catch (e) { } + + // We still have a file name - remove the extension + const fullModulePathWithoutExtension = removeFileExtension(path); + + // If the file is /index, it can be imported by its directory name + if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index") { + return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex); + } + + return fullModulePathWithoutExtension; } - return getPackageNameFromAtTypesDirectory(relativeFileName); + function getNodeResolvablePath(path: string): string { + const basePath = path.substring(0, parts.topLevelNodeModulesIndex); + if (sourceDirectory.indexOf(basePath) === 0) { + // if node_modules folder is in this folder or any of its parent folders, no need to keep it. + return path.substring(parts.topLevelPackageNameIndex + 1); + } + else { + return getRelativePath(path, sourceDirectory); + } + } } } + function getNodeModulePathParts(fullPath: string) { + // If fullPath can't be valid module file within node_modules, returns undefined. + // Example of expected pattern: /base/path/node_modules/[otherpackage/node_modules/]package/[subdirectory/]file.js + // Returns indices: ^ ^ ^ ^ + + let topLevelNodeModulesIndex = 0; + let topLevelPackageNameIndex = 0; + let packageRootIndex = 0; + let fileNameIndex = 0; + + const enum States { + BeforeNodeModules, + NodeModules, + PackageContent + } + + let partStart = 0; + let partEnd = 0; + let state = States.BeforeNodeModules; + + while (partEnd >= 0) { + partStart = partEnd; + partEnd = fullPath.indexOf("/", partStart + 1); + switch (state) { + case States.BeforeNodeModules: + if (fullPath.indexOf("/node_modules/", partStart) === partStart) { + topLevelNodeModulesIndex = partStart; + topLevelPackageNameIndex = partEnd; + state = States.NodeModules; + } + break; + case States.NodeModules: + packageRootIndex = partEnd; + state = States.PackageContent; + break; + case States.PackageContent: + if (fullPath.indexOf("/node_modules/", partStart) === partStart) { + state = States.NodeModules; + } + else { + state = States.PackageContent; + } + break; + } + } + + fileNameIndex = partStart; + + return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined; + } + function getPathRelativeToRootDirs(path: string, rootDirs: string[]) { for (const rootDir of rootDirs) { const relativeName = getRelativePathIfInDirectory(path, rootDir); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules4.ts b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules4.ts new file mode 100644 index 00000000000..9969bdcf46a --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules4.ts @@ -0,0 +1,25 @@ +/// + +//// [|f1/*0*/('');|] + +// @Filename: package.json +//// { "dependencies": { "package-name": "latest" } } + +// @Filename: node_modules/package-name/bin/lib/libfile.d.ts +//// export function f1(text: string): string; + +// @Filename: node_modules/package-name/bin/lib/libfile.js +//// function f1(text) { } +//// exports.f1 = f1; + +// @Filename: node_modules/package-name/package.json +//// { +//// "main": "bin/lib/libfile.js", +//// "types": "bin/lib/libfile.d.ts" +//// } + +verify.importFixAtPosition([ +`import { f1 } from "package-name"; + +f1('');` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules5.ts b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules5.ts new file mode 100644 index 00000000000..24100362994 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules5.ts @@ -0,0 +1,25 @@ +/// + +//// [|f1/*0*/('');|] + +// @Filename: package.json +//// { "dependencies": { "package-name": "latest" } } + +// @Filename: node_modules/package-name/node_modules/package-name2/bin/lib/libfile.d.ts +//// export function f1(text: string): string; + +// @Filename: node_modules/package-name/node_modules/package-name2/bin/lib/libfile.js +//// function f1(text) { } +//// exports.f1 = f1; + +// @Filename: node_modules/package-name/node_modules/package-name2/package.json +//// { +//// "main": "bin/lib/libfile.js", +//// "types": "bin/lib/libfile.d.ts" +//// } + +verify.importFixAtPosition([ +`import { f1 } from "package-name/node_modules/package-name2"; + +f1('');` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules6.ts b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules6.ts new file mode 100644 index 00000000000..7a52e3f67fe --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules6.ts @@ -0,0 +1,25 @@ +/// + +//// [|f1/*0*/('');|] + +// @Filename: package.json +//// { "dependencies": { "package-name": "latest" } } + +// @Filename: node_modules/package-name/bin/lib/index.d.ts +//// export function f1(text: string): string; + +// @Filename: node_modules/package-name/bin/lib/index.js +//// function f1(text) { } +//// exports.f1 = f1; + +// @Filename: node_modules/package-name/package.json +//// { +//// "main": "bin/lib/index.js", +//// "types": "bin/lib/index.d.ts" +//// } + +verify.importFixAtPosition([ +`import { f1 } from "package-name"; + +f1('');` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules7.ts b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules7.ts new file mode 100644 index 00000000000..9032018a7f5 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules7.ts @@ -0,0 +1,29 @@ +/// + +//// [|f1/*0*/('');|] + +// @Filename: package.json +//// { "dependencies": { "package-name": "0.0.1" } } + +// @Filename: node_modules/package-name/bin/lib/libfile.d.ts +//// export declare function f1(text: string): string; + +// @Filename: node_modules/package-name/bin/lib/libfile.js +//// function f1(text) {} +//// exports.f1 = f1; + +// @Filename: node_modules/package-name/package.json +//// { "main": "bin/lib/libfile.js" } + + +// In this case, importing the module by its package name: +// import { f1 } from 'package-name' +// could in theory work, however the resulting code compiles with a module resolution error +// since bin/lib/libfile.d.ts isn't declared under "typings" in package.json +// Therefore just import the module by its qualified path + +verify.importFixAtPosition([ +`import { f1 } from "package-name/bin/lib/libfile"; + +f1('');` +]); \ No newline at end of file