Bugs in missing import codefix

- We didn't locate the package.json correctly in cases where the module to be imported is in a subdirectory of the package
- We didn't look at the types element in package.json (just typings)
- We didn't remove /index.js from the path if the main module was in a subdirectory

Fixes #16963
This commit is contained in:
Mine Starks 2017-07-19 11:02:49 -07:00
parent abb229e91b
commit 15d294d350
4 changed files with 117 additions and 24 deletions

View file

@ -504,42 +504,60 @@ namespace ts.codefix {
return undefined; return undefined;
} }
const indexOfNodeModules = moduleFileName.indexOf("node_modules"); const indexOfTopNodeModules = moduleFileName.indexOf("node_modules");
if (indexOfNodeModules < 0) { if (indexOfTopNodeModules < 0) {
return undefined; return undefined;
} }
let relativeFileName: string; // Simplify the full file path to something that can be resolved by Node.
if (sourceDirectory.indexOf(moduleFileName.substring(0, indexOfNodeModules - 1)) === 0) { // First remove the extension
// if node_modules folder is in this folder or any of its parent folder, no need to keep it. let moduleSpecifier = removeFileExtension(moduleFileName);
relativeFileName = moduleFileName.substring(indexOfNodeModules + 13 /* "node_modules\".length */); // If the module could be imported by a directory name, use that directory's name
} moduleSpecifier = getDirectoryOrFileName(moduleSpecifier);
else { // Get a path that's relative to node_modules or the importing file's path
relativeFileName = getRelativePath(moduleFileName, sourceDirectory); moduleSpecifier = getNodeResolvablePath(moduleSpecifier);
} // If the module was found in @types, get the actual node package name
return getPackageNameFromAtTypesDirectory(moduleSpecifier);
relativeFileName = removeFileExtension(relativeFileName); function getDirectoryOrFileName(fullModulePathWithoutExtension: string): string {
if (endsWith(relativeFileName, "/index")) { // If the file is the main module, it can be imported by the package name
relativeFileName = getDirectoryPath(relativeFileName); const indexOfLastNodeModules = moduleFileName.lastIndexOf("node_modules");
} const indexOfSlashAtPackageRoot = moduleFileName.indexOf("/", indexOfLastNodeModules + 13 /* "node_modules\".length */);
else { const packageRootPath = moduleFileName.substring(0, indexOfSlashAtPackageRoot);
try { const packageJsonPath = combinePaths(packageRootPath, "package.json");
const moduleDirectory = getDirectoryPath(moduleFileName); if (context.host.fileExists(packageJsonPath)) {
const packageJsonContent = JSON.parse(context.host.readFile(combinePaths(moduleDirectory, "package.json"))); const packageJsonContent = JSON.parse(context.host.readFile(packageJsonPath));
if (packageJsonContent) { if (packageJsonContent) {
const mainFile = packageJsonContent.main || packageJsonContent.typings; const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main;
if (mainFile) { if (mainFileRelative) {
const mainExportFile = toPath(mainFile, moduleDirectory, getCanonicalFileName); const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName);
if (removeFileExtension(mainExportFile) === removeFileExtension(moduleFileName)) { if (removeFileExtension(mainExportFile) === removeFileExtension(moduleFileName)) {
relativeFileName = getDirectoryPath(relativeFileName); return packageRootPath;
} }
} }
} }
} }
catch (e) { }
// If the file is index.js, it can be imported by its directory name
if (endsWith(fullModulePathWithoutExtension, "/index")) {
return getDirectoryPath(fullModulePathWithoutExtension);
}
return fullModulePathWithoutExtension;
} }
return getPackageNameFromAtTypesDirectory(relativeFileName); function getNodeResolvablePath(path: string): string {
const fullPathUptoNodeModules = moduleFileName.substring(0, indexOfTopNodeModules - 1);
if (sourceDirectory.indexOf(fullPathUptoNodeModules) === 0) {
const indexOfTopPackageName = indexOfTopNodeModules + 13 /* "node_modules\".length */;
// if node_modules folder is in this folder or any of its parent folders, no need to keep it.
const relativeToTopNodeModules = path.substring(indexOfTopPackageName);
return relativeToTopNodeModules;
}
else {
return getRelativePath(path, sourceDirectory);
}
}
} }
} }

View file

@ -0,0 +1,25 @@
/// <reference path="fourslash.ts" />
//// [|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('');`
]);

View file

@ -0,0 +1,25 @@
/// <reference path="fourslash.ts" />
//// [|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('');`
]);

View file

@ -0,0 +1,25 @@
/// <reference path="fourslash.ts" />
//// [|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('');`
]);