Skip to content

Commit 5d188a8

Browse files
sheetalkamatsheetalkamat
committed
Always use resolved file to figure out subModule name in package id
Fixes microsoft#30429
1 parent 85d3c5d commit 5d188a8

26 files changed

+154
-238
lines changed

‎src/compiler/moduleNameResolver.ts

+52-77
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,23 @@ namespace ts {
1616
push(value: T): void;
1717
}
1818

19-
function withPackageId(packageId: PackageId | undefined, r: PathAndExtension | undefined): Resolved | undefined {
19+
function withPackageId(packageInfo: PackageJsonInfo | undefined, r: PathAndExtension | undefined): Resolved | undefined {
20+
let packageId: PackageId | undefined;
21+
if (r && packageInfo) {
22+
const packageJsonContent = packageInfo.packageJsonContent as PackageJson;
23+
if (typeof packageJsonContent.name === "string" && typeof packageJsonContent.version === "string") {
24+
packageId = {
25+
name: packageJsonContent.name,
26+
subModuleName: r.path.slice(packageInfo.packageDirectory.length + directorySeparator.length),
27+
version: packageJsonContent.version
28+
};
29+
}
30+
}
2031
return r && { path: r.path, extension: r.ext, packageId };
2132
}
2233

2334
function noPackageId(r: PathAndExtension | undefined): Resolved | undefined {
24-
return withPackageId(/*packageId*/ undefined, r);
35+
return withPackageId(/*packageInfo*/ undefined, r);
2536
}
2637

2738
function removeIgnoredPackageId(r: Resolved | undefined): PathAndExtension | undefined {
@@ -978,10 +989,9 @@ namespace ts {
978989
}
979990
const resolvedFromFile = loadModuleFromFile(extensions, candidate, onlyRecordFailures, state);
980991
if (resolvedFromFile) {
981-
const nm = considerPackageJson ? parseNodeModuleFromPath(resolvedFromFile) : undefined;
982-
const packageInfo = nm && getPackageJsonInfo(nm.packageDirectory, nm.subModuleName, /*onlyRecordFailures*/ false, state);
983-
const packageId = packageInfo && packageInfo.packageId;
984-
return withPackageId(packageId, resolvedFromFile);
992+
const packageDirectory = considerPackageJson ? parseNodeModuleFromPath(resolvedFromFile) : undefined;
993+
const packageInfo = packageDirectory ? getPackageJsonInfo(packageDirectory, /*onlyRecordFailures*/ false, state) : undefined;
994+
return withPackageId(packageInfo, resolvedFromFile);
985995
}
986996
}
987997
if (!onlyRecordFailures) {
@@ -1008,13 +1018,12 @@ namespace ts {
10081018
* (Not neeeded for `loadModuleFromNodeModules` as that looks up the `package.json` as part of resolution.)
10091019
*
10101020
* packageDirectory is the directory of the package itself.
1011-
* subModuleName is the path within the package.
1012-
* For `blah/node_modules/foo/index.d.ts` this is { packageDirectory: "foo", subModuleName: "index.d.ts" }. (Part before "/node_modules/" is ignored.)
1013-
* For `/node_modules/foo/bar.d.ts` this is { packageDirectory: "foo", subModuleName": "bar/index.d.ts" }.
1014-
* For `/node_modules/@types/foo/bar/index.d.ts` this is { packageDirectory: "@types/foo", subModuleName: "bar/index.d.ts" }.
1015-
* For `/node_modules/foo/bar/index.d.ts` this is { packageDirectory: "foo", subModuleName": "bar/index.d.ts" }.
1021+
* For `blah/node_modules/foo/index.d.ts` this is packageDirectory: "foo"
1022+
* For `/node_modules/foo/bar.d.ts` this is packageDirectory: "foo"
1023+
* For `/node_modules/@types/foo/bar/index.d.ts` this is packageDirectory: "@types/foo"
1024+
* For `/node_modules/foo/bar/index.d.ts` this is packageDirectory: "foo"
10161025
*/
1017-
function parseNodeModuleFromPath(resolved: PathAndExtension): { packageDirectory: string, subModuleName: string } | undefined {
1026+
function parseNodeModuleFromPath(resolved: PathAndExtension): string | undefined {
10181027
const path = normalizePath(resolved.path);
10191028
const idx = path.lastIndexOf(nodeModulesPathPart);
10201029
if (idx === -1) {
@@ -1026,29 +1035,14 @@ namespace ts {
10261035
if (path.charCodeAt(indexAfterNodeModules) === CharacterCodes.at) {
10271036
indexAfterPackageName = moveToNextDirectorySeparatorIfAvailable(path, indexAfterPackageName);
10281037
}
1029-
const packageDirectory = path.slice(0, indexAfterPackageName);
1030-
const subModuleName = removeExtension(path.slice(indexAfterPackageName + 1), resolved.ext) + Extension.Dts;
1031-
return { packageDirectory, subModuleName };
1038+
return path.slice(0, indexAfterPackageName);
10321039
}
10331040

10341041
function moveToNextDirectorySeparatorIfAvailable(path: string, prevSeparatorIndex: number): number {
10351042
const nextSeparatorIndex = path.indexOf(directorySeparator, prevSeparatorIndex + 1);
10361043
return nextSeparatorIndex === -1 ? prevSeparatorIndex : nextSeparatorIndex;
10371044
}
10381045

1039-
function addExtensionAndIndex(path: string): string {
1040-
if (path === "") {
1041-
return "index.d.ts";
1042-
}
1043-
if (endsWith(path, ".d.ts")) {
1044-
return path;
1045-
}
1046-
if (path === "index" || endsWith(path, "/index")) {
1047-
return path + ".d.ts";
1048-
}
1049-
return path + "/index.d.ts";
1050-
}
1051-
10521046
function loadModuleFromFileNoPackageId(extensions: Extensions, candidate: string, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved | undefined {
10531047
return noPackageId(loadModuleFromFile(extensions, candidate, onlyRecordFailures, state));
10541048
}
@@ -1129,56 +1123,29 @@ namespace ts {
11291123
}
11301124

11311125
function loadNodeModuleFromDirectory(extensions: Extensions, candidate: string, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson = true) {
1132-
const packageInfo = considerPackageJson ? getPackageJsonInfo(candidate, "", onlyRecordFailures, state) : undefined;
1133-
const packageId = packageInfo && packageInfo.packageId;
1126+
const packageInfo = considerPackageJson ? getPackageJsonInfo(candidate, onlyRecordFailures, state) : undefined;
11341127
const packageJsonContent = packageInfo && packageInfo.packageJsonContent;
11351128
const versionPaths = packageInfo && packageInfo.versionPaths;
1136-
return withPackageId(packageId, loadNodeModuleFromDirectoryWorker(extensions, candidate, onlyRecordFailures, state, packageJsonContent, versionPaths));
1129+
return withPackageId(packageInfo, loadNodeModuleFromDirectoryWorker(extensions, candidate, onlyRecordFailures, state, packageJsonContent, versionPaths));
11371130
}
11381131

11391132
interface PackageJsonInfo {
1140-
packageJsonContent: PackageJsonPathFields | undefined;
1141-
packageId: PackageId | undefined;
1133+
packageDirectory: string;
1134+
packageJsonContent: PackageJsonPathFields;
11421135
versionPaths: VersionPaths | undefined;
11431136
}
11441137

1145-
function getPackageJsonInfo(packageDirectory: string, subModuleName: string, onlyRecordFailures: boolean, state: ModuleResolutionState): PackageJsonInfo | undefined {
1138+
function getPackageJsonInfo(packageDirectory: string, onlyRecordFailures: boolean, state: ModuleResolutionState): PackageJsonInfo | undefined {
11461139
const { host, traceEnabled } = state;
11471140
const directoryExists = !onlyRecordFailures && directoryProbablyExists(packageDirectory, host);
11481141
const packageJsonPath = combinePaths(packageDirectory, "package.json");
11491142
if (directoryExists && host.fileExists(packageJsonPath)) {
11501143
const packageJsonContent = readJson(packageJsonPath, host) as PackageJson;
1151-
if (subModuleName === "") { // looking up the root - need to handle types/typings/main redirects for subModuleName
1152-
const path = readPackageJsonTypesFields(packageJsonContent, packageDirectory, state);
1153-
if (typeof path === "string") {
1154-
subModuleName = addExtensionAndIndex(path.substring(packageDirectory.length + 1));
1155-
}
1156-
else {
1157-
const jsPath = readPackageJsonMainField(packageJsonContent, packageDirectory, state);
1158-
if (typeof jsPath === "string" && jsPath.length > packageDirectory.length) {
1159-
const potentialSubModule = jsPath.substring(packageDirectory.length + 1);
1160-
subModuleName = (forEach(supportedJSExtensions, extension =>
1161-
tryRemoveExtension(potentialSubModule, extension)) || potentialSubModule) + Extension.Dts;
1162-
}
1163-
else {
1164-
subModuleName = "index.d.ts";
1165-
}
1166-
}
1167-
}
1168-
1169-
if (!endsWith(subModuleName, Extension.Dts)) {
1170-
subModuleName = addExtensionAndIndex(subModuleName);
1171-
}
1172-
1173-
const versionPaths = readPackageJsonTypesVersionPaths(packageJsonContent, state);
1174-
const packageId: PackageId | undefined = typeof packageJsonContent.name === "string" && typeof packageJsonContent.version === "string"
1175-
? { name: packageJsonContent.name, subModuleName, version: packageJsonContent.version }
1176-
: undefined;
11771144
if (traceEnabled) {
11781145
trace(host, Diagnostics.Found_package_json_at_0, packageJsonPath);
11791146
}
1180-
1181-
return { packageJsonContent, packageId, versionPaths };
1147+
const versionPaths = readPackageJsonTypesVersionPaths(packageJsonContent, state);
1148+
return { packageDirectory, packageJsonContent, versionPaths };
11821149
}
11831150
else {
11841151
if (directoryExists && traceEnabled) {
@@ -1333,42 +1300,50 @@ namespace ts {
13331300
const candidate = normalizePath(combinePaths(nodeModulesDirectory, moduleName));
13341301

13351302
// First look for a nested package.json, as in `node_modules/foo/bar/package.json`.
1336-
let packageJsonContent: PackageJsonPathFields | undefined;
1337-
let packageId: PackageId | undefined;
1338-
let versionPaths: VersionPaths | undefined;
1339-
1340-
const packageInfo = getPackageJsonInfo(candidate, "", !nodeModulesDirectoryExists, state);
1303+
let packageInfo = getPackageJsonInfo(candidate, !nodeModulesDirectoryExists, state);
13411304
if (packageInfo) {
1342-
({ packageJsonContent, packageId, versionPaths } = packageInfo);
13431305
const fromFile = loadModuleFromFile(extensions, candidate, !nodeModulesDirectoryExists, state);
13441306
if (fromFile) {
13451307
return noPackageId(fromFile);
13461308
}
13471309

1348-
const fromDirectory = loadNodeModuleFromDirectoryWorker(extensions, candidate, !nodeModulesDirectoryExists, state, packageJsonContent, versionPaths);
1349-
return withPackageId(packageId, fromDirectory);
1310+
const fromDirectory = loadNodeModuleFromDirectoryWorker(
1311+
extensions,
1312+
candidate,
1313+
!nodeModulesDirectoryExists,
1314+
state,
1315+
packageInfo.packageJsonContent,
1316+
packageInfo.versionPaths
1317+
);
1318+
return withPackageId(packageInfo, fromDirectory);
13501319
}
13511320

13521321
const loader: ResolutionKindSpecificLoader = (extensions, candidate, onlyRecordFailures, state) => {
13531322
const pathAndExtension =
13541323
loadModuleFromFile(extensions, candidate, onlyRecordFailures, state) ||
1355-
loadNodeModuleFromDirectoryWorker(extensions, candidate, onlyRecordFailures, state, packageJsonContent, versionPaths);
1356-
return withPackageId(packageId, pathAndExtension);
1324+
loadNodeModuleFromDirectoryWorker(
1325+
extensions,
1326+
candidate,
1327+
onlyRecordFailures,
1328+
state,
1329+
packageInfo && packageInfo.packageJsonContent,
1330+
packageInfo && packageInfo.versionPaths
1331+
);
1332+
return withPackageId(packageInfo, pathAndExtension);
13571333
};
13581334

13591335
const { packageName, rest } = parsePackageName(moduleName);
13601336
if (rest !== "") { // If "rest" is empty, we just did this search above.
13611337
const packageDirectory = combinePaths(nodeModulesDirectory, packageName);
13621338

13631339
// Don't use a "types" or "main" from here because we're not loading the root, but a subdirectory -- just here for the packageId and path mappings.
1364-
const packageInfo = getPackageJsonInfo(packageDirectory, rest, !nodeModulesDirectoryExists, state);
1365-
if (packageInfo) ({ packageId, versionPaths } = packageInfo);
1366-
if (versionPaths) {
1340+
packageInfo = getPackageJsonInfo(packageDirectory, !nodeModulesDirectoryExists, state);
1341+
if (packageInfo && packageInfo.versionPaths) {
13671342
if (state.traceEnabled) {
1368-
trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, versionPaths.version, version, rest);
1343+
trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, packageInfo.versionPaths.version, version, rest);
13691344
}
13701345
const packageDirectoryExists = nodeModulesDirectoryExists && directoryProbablyExists(packageDirectory, state.host);
1371-
const fromPaths = tryLoadModuleUsingPaths(extensions, rest, packageDirectory, versionPaths.paths, loader, !packageDirectoryExists, state);
1346+
const fromPaths = tryLoadModuleUsingPaths(extensions, rest, packageDirectory, packageInfo.versionPaths.paths, loader, !packageDirectoryExists, state);
13721347
if (fromPaths) {
13731348
return fromPaths.value;
13741349
}

‎tests/baselines/reference/duplicatePackage_relativeImportWithinPackage.trace.json

+4-7
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
"======== Resolving module 'foo/use' from '/index.ts'. ========",
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module 'foo/use' from 'node_modules' folder, target file type 'TypeScript'.",
5-
"'package.json' does not have a 'typesVersions' field.",
65
"Found 'package.json' at '/node_modules/foo/package.json'.",
6+
"'package.json' does not have a 'typesVersions' field.",
77
"File '/node_modules/foo/use.ts' does not exist.",
88
"File '/node_modules/foo/use.tsx' does not exist.",
99
"File '/node_modules/foo/use.d.ts' exist - use it as a name resolution result.",
1010
"Resolving real path for '/node_modules/foo/use.d.ts', result '/node_modules/foo/use.d.ts'.",
11-
"======== Module name 'foo/use' was successfully resolved to '/node_modules/foo/use.d.ts' with Package ID 'foo/use/index.d.ts@1.2.3'. ========",
11+
"======== Module name 'foo/use' was successfully resolved to '/node_modules/foo/use.d.ts' with Package ID 'foo/use.d.ts@1.2.3'. ========",
1212
"======== Resolving module 'a' from '/index.ts'. ========",
1313
"Module resolution kind is not specified, using 'NodeJs'.",
1414
"Loading module 'a' from 'node_modules' folder, target file type 'TypeScript'.",
@@ -27,17 +27,14 @@
2727
"File '/node_modules/foo/index.ts' does not exist.",
2828
"File '/node_modules/foo/index.tsx' does not exist.",
2929
"File '/node_modules/foo/index.d.ts' exist - use it as a name resolution result.",
30-
"'package.json' does not have a 'typesVersions' field.",
3130
"Found 'package.json' at '/node_modules/foo/package.json'.",
31+
"'package.json' does not have a 'typesVersions' field.",
3232
"======== Module name './index' was successfully resolved to '/node_modules/foo/index.d.ts' with Package ID 'foo/index.d.ts@1.2.3'. ========",
3333
"======== Resolving module 'foo' from '/node_modules/a/index.d.ts'. ========",
3434
"Module resolution kind is not specified, using 'NodeJs'.",
3535
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
36-
"'package.json' does not have a 'typings' field.",
37-
"'package.json' does not have a 'types' field.",
38-
"'package.json' does not have a 'main' field.",
39-
"'package.json' does not have a 'typesVersions' field.",
4036
"Found 'package.json' at '/node_modules/a/node_modules/foo/package.json'.",
37+
"'package.json' does not have a 'typesVersions' field.",
4138
"File '/node_modules/a/node_modules/foo.ts' does not exist.",
4239
"File '/node_modules/a/node_modules/foo.tsx' does not exist.",
4340
"File '/node_modules/a/node_modules/foo.d.ts' does not exist.",

‎tests/baselines/reference/duplicatePackage_relativeImportWithinPackage_scoped.trace.json

+4-7
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
"======== Resolving module '@foo/bar/use' from '/index.ts'. ========",
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module '@foo/bar/use' from 'node_modules' folder, target file type 'TypeScript'.",
5-
"'package.json' does not have a 'typesVersions' field.",
65
"Found 'package.json' at '/node_modules/@foo/bar/package.json'.",
6+
"'package.json' does not have a 'typesVersions' field.",
77
"File '/node_modules/@foo/bar/use.ts' does not exist.",
88
"File '/node_modules/@foo/bar/use.tsx' does not exist.",
99
"File '/node_modules/@foo/bar/use.d.ts' exist - use it as a name resolution result.",
1010
"Resolving real path for '/node_modules/@foo/bar/use.d.ts', result '/node_modules/@foo/bar/use.d.ts'.",
11-
"======== Module name '@foo/bar/use' was successfully resolved to '/node_modules/@foo/bar/use.d.ts' with Package ID '@foo/bar/use/index.d.ts@1.2.3'. ========",
11+
"======== Module name '@foo/bar/use' was successfully resolved to '/node_modules/@foo/bar/use.d.ts' with Package ID '@foo/bar/use.d.ts@1.2.3'. ========",
1212
"======== Resolving module 'a' from '/index.ts'. ========",
1313
"Module resolution kind is not specified, using 'NodeJs'.",
1414
"Loading module 'a' from 'node_modules' folder, target file type 'TypeScript'.",
@@ -27,17 +27,14 @@
2727
"File '/node_modules/@foo/bar/index.ts' does not exist.",
2828
"File '/node_modules/@foo/bar/index.tsx' does not exist.",
2929
"File '/node_modules/@foo/bar/index.d.ts' exist - use it as a name resolution result.",
30-
"'package.json' does not have a 'typesVersions' field.",
3130
"Found 'package.json' at '/node_modules/@foo/bar/package.json'.",
31+
"'package.json' does not have a 'typesVersions' field.",
3232
"======== Module name './index' was successfully resolved to '/node_modules/@foo/bar/index.d.ts' with Package ID '@foo/bar/index.d.ts@1.2.3'. ========",
3333
"======== Resolving module '@foo/bar' from '/node_modules/a/index.d.ts'. ========",
3434
"Module resolution kind is not specified, using 'NodeJs'.",
3535
"Loading module '@foo/bar' from 'node_modules' folder, target file type 'TypeScript'.",
36-
"'package.json' does not have a 'typings' field.",
37-
"'package.json' does not have a 'types' field.",
38-
"'package.json' does not have a 'main' field.",
39-
"'package.json' does not have a 'typesVersions' field.",
4036
"Found 'package.json' at '/node_modules/a/node_modules/@foo/bar/package.json'.",
37+
"'package.json' does not have a 'typesVersions' field.",
4138
"File '/node_modules/a/node_modules/@foo/bar.ts' does not exist.",
4239
"File '/node_modules/a/node_modules/@foo/bar.tsx' does not exist.",
4340
"File '/node_modules/a/node_modules/@foo/bar.d.ts' does not exist.",

0 commit comments

Comments
 (0)