Skip to content

Commit c334999

Browse files
weswighammhegazy
weswigham
authored and
mhegazy
committed
Use the full local file path as the id for a submodule (#21471) (#21486)
* Use the full file path as the id for a submodule * Informal code review feedback
1 parent 461450b commit c334999

21 files changed

+193
-21
lines changed

‎src/compiler/moduleNameResolver.ts

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,7 @@ namespace ts {
801801
}
802802
const resolvedFromFile = loadModuleFromFile(extensions, candidate, failedLookupLocations, onlyRecordFailures, state);
803803
if (resolvedFromFile) {
804-
const nm = considerPackageJson ? parseNodeModuleFromPath(resolvedFromFile.path) : undefined;
804+
const nm = considerPackageJson ? parseNodeModuleFromPath(resolvedFromFile) : undefined;
805805
const packageId = nm && getPackageJsonInfo(nm.packageDirectory, nm.subModuleName, failedLookupLocations, /*onlyRecordFailures*/ false, state).packageId;
806806
return withPackageId(packageId, resolvedFromFile);
807807
}
@@ -826,12 +826,13 @@ namespace ts {
826826
*
827827
* packageDirectory is the directory of the package itself.
828828
* subModuleName is the path within the package.
829-
* For `blah/node_modules/foo/index.d.ts` this is { packageDirectory: "foo", subModuleName: "" }. (Part before "/node_modules/" is ignored.)
830-
* For `/node_modules/foo/bar.d.ts` this is { packageDirectory: "foo", subModuleName": "bar" }.
831-
* For `/node_modules/@types/foo/bar/index.d.ts` this is { packageDirectory: "@types/foo", subModuleName: "bar" }.
829+
* For `blah/node_modules/foo/index.d.ts` this is { packageDirectory: "foo", subModuleName: "index.d.ts" }. (Part before "/node_modules/" is ignored.)
830+
* For `/node_modules/foo/bar.d.ts` this is { packageDirectory: "foo", subModuleName": "bar/index.d.ts" }.
831+
* For `/node_modules/@types/foo/bar/index.d.ts` this is { packageDirectory: "@types/foo", subModuleName: "bar/index.d.ts" }.
832+
* For `/node_modules/foo/bar/index.d.ts` this is { packageDirectory: "foo", subModuleName": "bar/index.d.ts" }.
832833
*/
833-
function parseNodeModuleFromPath(path: string): { packageDirectory: string, subModuleName: string } | undefined {
834-
path = normalizePath(path);
834+
function parseNodeModuleFromPath(resolved: PathAndExtension): { packageDirectory: string, subModuleName: string } | undefined {
835+
const path = normalizePath(resolved.path);
835836
const idx = path.lastIndexOf(nodeModulesPathPart);
836837
if (idx === -1) {
837838
return undefined;
@@ -843,7 +844,7 @@ namespace ts {
843844
indexAfterPackageName = moveToNextDirectorySeparatorIfAvailable(path, indexAfterPackageName);
844845
}
845846
const packageDirectory = path.slice(0, indexAfterPackageName);
846-
const subModuleName = removeExtensionAndIndex(path.slice(indexAfterPackageName + 1));
847+
const subModuleName = removeExtension(path.slice(indexAfterPackageName + 1), resolved.ext) + Extension.Dts;
847848
return { packageDirectory, subModuleName };
848849
}
849850

@@ -852,9 +853,17 @@ namespace ts {
852853
return nextSeparatorIndex === -1 ? prevSeparatorIndex : nextSeparatorIndex;
853854
}
854855

855-
function removeExtensionAndIndex(path: string): string {
856-
const noExtension = removeFileExtension(path);
857-
return noExtension === "index" ? "" : removeSuffix(noExtension, "/index");
856+
function addExtensionAndIndex(path: string): string {
857+
if (path === "") {
858+
return "index.d.ts";
859+
}
860+
if (endsWith(path, ".d.ts")) {
861+
return path;
862+
}
863+
if (endsWith(path, "/index")) {
864+
return path + ".d.ts";
865+
}
866+
return path + "/index.d.ts";
858867
}
859868

860869
/* @internal */
@@ -955,12 +964,31 @@ namespace ts {
955964
subModuleName: string,
956965
failedLookupLocations: Push<string>,
957966
onlyRecordFailures: boolean,
958-
{ host, traceEnabled }: ModuleResolutionState,
967+
state: ModuleResolutionState,
959968
): { found: boolean, packageJsonContent: PackageJsonPathFields | undefined, packageId: PackageId | undefined } {
969+
const { host, traceEnabled } = state;
960970
const directoryExists = !onlyRecordFailures && directoryProbablyExists(nodeModuleDirectory, host);
961971
const packageJsonPath = pathToPackageJson(nodeModuleDirectory);
962972
if (directoryExists && host.fileExists(packageJsonPath)) {
963973
const packageJsonContent = readJson(packageJsonPath, host);
974+
if (subModuleName === "") { // looking up the root - need to handle types/typings/main redirects for subModuleName
975+
const path = tryReadPackageJsonFields(/*readTypes*/ true, packageJsonContent, nodeModuleDirectory, state);
976+
if (typeof path === "string") {
977+
subModuleName = addExtensionAndIndex(path.substring(nodeModuleDirectory.length + 1));
978+
}
979+
else {
980+
const jsPath = tryReadPackageJsonFields(/*readTypes*/ false, packageJsonContent, nodeModuleDirectory, state);
981+
if (typeof jsPath === "string") {
982+
subModuleName = removeExtension(removeExtension(jsPath.substring(nodeModuleDirectory.length + 1), Extension.Js), Extension.Jsx) + Extension.Dts;
983+
}
984+
else {
985+
subModuleName = "index.d.ts";
986+
}
987+
}
988+
}
989+
if (!endsWith(subModuleName, Extension.Dts)) {
990+
subModuleName = addExtensionAndIndex(subModuleName);
991+
}
964992
const packageId: PackageId = typeof packageJsonContent.name === "string" && typeof packageJsonContent.version === "string"
965993
? { name: packageJsonContent.name, subModuleName, version: packageJsonContent.version }
966994
: undefined;

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
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-
"Found 'package.json' at '/node_modules/foo/package.json'. Package ID is 'foo/[email protected]'.",
5+
"Found 'package.json' at '/node_modules/foo/package.json'. Package ID is 'foo/use/index.d.ts@1.2.3'.",
66
"File '/node_modules/foo/use.ts' does not exist.",
77
"File '/node_modules/foo/use.tsx' does not exist.",
88
"File '/node_modules/foo/use.d.ts' exist - use it as a name resolution result.",
@@ -26,12 +26,15 @@
2626
"File '/node_modules/foo/index.ts' does not exist.",
2727
"File '/node_modules/foo/index.tsx' does not exist.",
2828
"File '/node_modules/foo/index.d.ts' exist - use it as a name resolution result.",
29-
"Found 'package.json' at '/node_modules/foo/package.json'. Package ID is '[email protected]'.",
29+
"Found 'package.json' at '/node_modules/foo/package.json'. Package ID is 'foo/index.d.ts@1.2.3'.",
3030
"======== Module name './index' was successfully resolved to '/node_modules/foo/index.d.ts'. ========",
3131
"======== Resolving module 'foo' from '/node_modules/a/index.d.ts'. ========",
3232
"Module resolution kind is not specified, using 'NodeJs'.",
3333
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
34-
"Found 'package.json' at '/node_modules/a/node_modules/foo/package.json'. Package ID is '[email protected]'.",
34+
"'package.json' does not have a 'typings' field.",
35+
"'package.json' does not have a 'types' field.",
36+
"'package.json' does not have a 'main' field.",
37+
"Found 'package.json' at '/node_modules/a/node_modules/foo/package.json'. Package ID is 'foo/[email protected]'.",
3538
"File '/node_modules/a/node_modules/foo.ts' does not exist.",
3639
"File '/node_modules/a/node_modules/foo.tsx' does not exist.",
3740
"File '/node_modules/a/node_modules/foo.d.ts' does not exist.",

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
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-
"Found 'package.json' at '/node_modules/@foo/bar/package.json'. Package ID is '@foo/bar/[email protected]'.",
5+
"Found 'package.json' at '/node_modules/@foo/bar/package.json'. Package ID is '@foo/bar/use/index.d.ts@1.2.3'.",
66
"File '/node_modules/@foo/bar/use.ts' does not exist.",
77
"File '/node_modules/@foo/bar/use.tsx' does not exist.",
88
"File '/node_modules/@foo/bar/use.d.ts' exist - use it as a name resolution result.",
@@ -26,12 +26,15 @@
2626
"File '/node_modules/@foo/bar/index.ts' does not exist.",
2727
"File '/node_modules/@foo/bar/index.tsx' does not exist.",
2828
"File '/node_modules/@foo/bar/index.d.ts' exist - use it as a name resolution result.",
29-
"Found 'package.json' at '/node_modules/@foo/bar/package.json'. Package ID is '@foo/[email protected]'.",
29+
"Found 'package.json' at '/node_modules/@foo/bar/package.json'. Package ID is '@foo/bar/index.d.ts@1.2.3'.",
3030
"======== Module name './index' was successfully resolved to '/node_modules/@foo/bar/index.d.ts'. ========",
3131
"======== Resolving module '@foo/bar' from '/node_modules/a/index.d.ts'. ========",
3232
"Module resolution kind is not specified, using 'NodeJs'.",
3333
"Loading module '@foo/bar' from 'node_modules' folder, target file type 'TypeScript'.",
34-
"Found 'package.json' at '/node_modules/a/node_modules/@foo/bar/package.json'. Package ID is '@foo/[email protected]'.",
34+
"'package.json' does not have a 'typings' field.",
35+
"'package.json' does not have a 'types' field.",
36+
"'package.json' does not have a 'main' field.",
37+
"Found 'package.json' at '/node_modules/a/node_modules/@foo/bar/package.json'. Package ID is '@foo/bar/[email protected]'.",
3538
"File '/node_modules/a/node_modules/@foo/bar.ts' does not exist.",
3639
"File '/node_modules/a/node_modules/@foo/bar.tsx' does not exist.",
3740
"File '/node_modules/a/node_modules/@foo/bar.d.ts' does not exist.",

‎tests/baselines/reference/library-reference-10.trace.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
[
22
"======== Resolving type reference directive 'jquery', containing file '/foo/consumer.ts', root directory './types'. ========",
33
"Resolving with primary search path './types'.",
4+
"'package.json' has 'typings' field 'jquery.d.ts' that references 'types/jquery/jquery.d.ts'.",
45
"Found 'package.json' at './types/jquery/package.json'.",
56
"'package.json' has 'typings' field 'jquery.d.ts' that references 'types/jquery/jquery.d.ts'.",
67
"File 'types/jquery/jquery.d.ts' exist - use it as a name resolution result.",
78
"Resolving real path for 'types/jquery/jquery.d.ts', result '/foo/types/jquery/jquery.d.ts'.",
89
"======== Type reference directive 'jquery' was successfully resolved to '/foo/types/jquery/jquery.d.ts', primary: true. ========",
910
"======== Resolving type reference directive 'jquery', containing file '/foo/__inferred type names__.ts', root directory './types'. ========",
1011
"Resolving with primary search path './types'.",
12+
"'package.json' has 'typings' field 'jquery.d.ts' that references 'types/jquery/jquery.d.ts'.",
1113
"Found 'package.json' at './types/jquery/package.json'.",
1214
"'package.json' has 'typings' field 'jquery.d.ts' that references 'types/jquery/jquery.d.ts'.",
1315
"File 'types/jquery/jquery.d.ts' exist - use it as a name resolution result.",

‎tests/baselines/reference/library-reference-11.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"Root directory cannot be determined, skipping primary search paths.",
44
"Looking up in 'node_modules' folder, initial location '/a/b'.",
55
"Directory '/a/b/node_modules' does not exist, skipping all lookups in it.",
6+
"'package.json' has 'typings' field 'jquery.d.ts' that references '/a/node_modules/jquery/jquery.d.ts'.",
67
"Found 'package.json' at '/a/node_modules/jquery/package.json'.",
78
"File '/a/node_modules/jquery.d.ts' does not exist.",
89
"'package.json' has 'typings' field 'jquery.d.ts' that references '/a/node_modules/jquery/jquery.d.ts'.",

‎tests/baselines/reference/library-reference-12.trace.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
"Root directory cannot be determined, skipping primary search paths.",
44
"Looking up in 'node_modules' folder, initial location '/a/b'.",
55
"Directory '/a/b/node_modules' does not exist, skipping all lookups in it.",
6+
"'package.json' does not have a 'typings' field.",
7+
"'package.json' has 'types' field 'dist/jquery.d.ts' that references '/a/node_modules/jquery/dist/jquery.d.ts'.",
68
"Found 'package.json' at '/a/node_modules/jquery/package.json'.",
79
"File '/a/node_modules/jquery.d.ts' does not exist.",
810
"'package.json' does not have a 'typings' field.",

‎tests/baselines/reference/library-reference-2.trace.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
[
22
"======== Resolving type reference directive 'jquery', containing file '/consumer.ts', root directory '/types'. ========",
33
"Resolving with primary search path '/types'.",
4+
"'package.json' does not have a 'typings' field.",
5+
"'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.",
46
"Found 'package.json' at '/types/jquery/package.json'.",
57
"'package.json' does not have a 'typings' field.",
68
"'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.",
@@ -9,6 +11,8 @@
911
"======== Type reference directive 'jquery' was successfully resolved to '/types/jquery/jquery.d.ts', primary: true. ========",
1012
"======== Resolving type reference directive 'jquery', containing file 'test/__inferred type names__.ts', root directory '/types'. ========",
1113
"Resolving with primary search path '/types'.",
14+
"'package.json' does not have a 'typings' field.",
15+
"'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.",
1216
"Found 'package.json' at '/types/jquery/package.json'.",
1317
"'package.json' does not have a 'typings' field.",
1418
"'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.",

‎tests/baselines/reference/moduleLocalImportNotIncorrectlyRedirected.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//// [tests/cases/compiler/moduleLocalImportNotIncorrectlyRedirected.ts] ////
2+
3+
//// [package.json]
4+
{
5+
"name": "troublesome-lib",
6+
"typings": "lib/index.d.ts",
7+
"version": "0.0.1"
8+
}
9+
//// [index.d.ts]
10+
import { Position } from './utilities/positioning';
11+
export interface ISpinButton {}
12+
//// [positioning.d.ts]
13+
export * from './positioning/index';
14+
//// [index.d.ts]
15+
export declare enum Position {
16+
top,
17+
}
18+
//// [index.ts]
19+
import { ISpinButton } from "troublesome-lib";
20+
21+
//// [index.js]
22+
"use strict";
23+
exports.__esModule = true;

‎tests/baselines/reference/moduleLocalImportNotIncorrectlyRedirected.symbols

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
=== tests/cases/compiler/index.ts ===
2+
import { ISpinButton } from "troublesome-lib";
3+
>ISpinButton : Symbol(ISpinButton, Decl(index.ts, 0, 8))
4+
5+
=== tests/cases/compiler/node_modules/troublesome-lib/lib/index.d.ts ===
6+
import { Position } from './utilities/positioning';
7+
>Position : Symbol(Position, Decl(index.d.ts, 0, 8))
8+
9+
export interface ISpinButton {}
10+
>ISpinButton : Symbol(ISpinButton, Decl(index.d.ts, 0, 51))
11+
12+
=== tests/cases/compiler/node_modules/troublesome-lib/lib/utilities/positioning.d.ts ===
13+
export * from './positioning/index';
14+
No type information for this code.=== tests/cases/compiler/node_modules/troublesome-lib/lib/utilities/positioning/index.d.ts ===
15+
export declare enum Position {
16+
>Position : Symbol(Position, Decl(index.d.ts, 0, 0))
17+
18+
top,
19+
>top : Symbol(Position.top, Decl(index.d.ts, 0, 30))
20+
}

‎tests/baselines/reference/moduleLocalImportNotIncorrectlyRedirected.types

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
=== tests/cases/compiler/index.ts ===
2+
import { ISpinButton } from "troublesome-lib";
3+
>ISpinButton : any
4+
5+
=== tests/cases/compiler/node_modules/troublesome-lib/lib/index.d.ts ===
6+
import { Position } from './utilities/positioning';
7+
>Position : typeof Position
8+
9+
export interface ISpinButton {}
10+
>ISpinButton : ISpinButton
11+
12+
=== tests/cases/compiler/node_modules/troublesome-lib/lib/utilities/positioning.d.ts ===
13+
export * from './positioning/index';
14+
No type information for this code.=== tests/cases/compiler/node_modules/troublesome-lib/lib/utilities/positioning/index.d.ts ===
15+
export declare enum Position {
16+
>Position : Position
17+
18+
top,
19+
>top : Position
20+
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
"======== Resolving module 'normalize.css' from '/a.ts'. ========",
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module 'normalize.css' from 'node_modules' folder, target file type 'TypeScript'.",
5+
"'package.json' does not have a 'typings' field.",
6+
"'package.json' does not have a 'types' field.",
7+
"'package.json' has 'main' field 'normalize.css' that references '/node_modules/normalize.css/normalize.css'.",
58
"Found 'package.json' at '/node_modules/normalize.css/package.json'.",
69
"File '/node_modules/normalize.css.ts' does not exist.",
710
"File '/node_modules/normalize.css.tsx' does not exist.",
@@ -13,6 +16,9 @@
1316
"File '/node_modules/normalize.css/index.d.ts' does not exist.",
1417
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
1518
"Loading module 'normalize.css' from 'node_modules' folder, target file type 'JavaScript'.",
19+
"'package.json' does not have a 'typings' field.",
20+
"'package.json' does not have a 'types' field.",
21+
"'package.json' has 'main' field 'normalize.css' that references '/node_modules/normalize.css/normalize.css'.",
1622
"Found 'package.json' at '/node_modules/normalize.css/package.json'.",
1723
"File '/node_modules/normalize.css.js' does not exist.",
1824
"File '/node_modules/normalize.css.jsx' does not exist.",

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
"======== Resolving module 'foo' from '/a.ts'. ========",
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
5+
"'package.json' does not have a 'typings' field.",
6+
"'package.json' has 'types' field 'foo.js' that references '/node_modules/foo/foo.js'.",
57
"Found 'package.json' at '/node_modules/foo/package.json'.",
68
"File '/node_modules/foo.ts' does not exist.",
79
"File '/node_modules/foo.tsx' does not exist.",
@@ -24,6 +26,8 @@
2426
"File '/node_modules/foo/index.d.ts' does not exist.",
2527
"Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
2628
"Loading module 'foo' from 'node_modules' folder, target file type 'JavaScript'.",
29+
"'package.json' does not have a 'typings' field.",
30+
"'package.json' has 'types' field 'foo.js' that references '/node_modules/foo/foo.js'.",
2731
"Found 'package.json' at '/node_modules/foo/package.json'.",
2832
"File '/node_modules/foo.js' does not exist.",
2933
"File '/node_modules/foo.jsx' does not exist.",

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
"======== Resolving module 'foo/bar' from '/a.ts'. ========",
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module 'foo/bar' from 'node_modules' folder, target file type 'TypeScript'.",
5+
"'package.json' does not have a 'typings' field.",
6+
"'package.json' has 'types' field 'types.d.ts' that references '/node_modules/foo/bar/types.d.ts'.",
57
"Found 'package.json' at '/node_modules/foo/bar/package.json'.",
68
"File '/node_modules/foo/bar.ts' does not exist.",
79
"File '/node_modules/foo/bar.tsx' does not exist.",

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
"======== Resolving module 'foo/@bar' from '/a.ts'. ========",
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module 'foo/@bar' from 'node_modules' folder, target file type 'TypeScript'.",
5+
"'package.json' does not have a 'typings' field.",
6+
"'package.json' has 'types' field 'types.d.ts' that references '/node_modules/foo/@bar/types.d.ts'.",
57
"Found 'package.json' at '/node_modules/foo/@bar/package.json'.",
68
"File '/node_modules/foo/@bar.ts' does not exist.",
79
"File '/node_modules/foo/@bar.tsx' does not exist.",

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
"======== Resolving module '@foo/bar' from '/a.ts'. ========",
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module '@foo/bar' from 'node_modules' folder, target file type 'TypeScript'.",
5+
"'package.json' does not have a 'typings' field.",
6+
"'package.json' has 'types' field 'types.d.ts' that references '/node_modules/@foo/bar/types.d.ts'.",
57
"Found 'package.json' at '/node_modules/@foo/bar/package.json'.",
68
"File '/node_modules/@foo/bar.ts' does not exist.",
79
"File '/node_modules/@foo/bar.tsx' does not exist.",

0 commit comments

Comments
 (0)