Skip to content

Skip resolving files directly inside node_modules #52809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Nathan Shively-Sanders (sandersn) merged 12 commits into from
May 11, 2023
Merged

Skip resolving files directly inside node_modules #52809

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5f96e29
Skip resolving files directly inside node_modules
sandersn Feb 16, 2023
299c42b
In node_modules, skip modules that look like absolute URIs
sandersn Feb 16, 2023
b279732
lolcolons
sandersn Feb 16, 2023
b05b643
Add test + skip more
sandersn Feb 16, 2023
f311945
Merge branch 'main' into skip-resolving-ts-files-in-node_modules
sandersn Feb 17, 2023
6bf4384
accept missed baseline
sandersn Feb 17, 2023
dd92d45
Merge branch 'main' into skip-resolving-ts-files-in-node_modules
sandersn May 9, 2023
1f469d2
only skip resolving in ESM
sandersn May 10, 2023
586a892
Revert some unneeded test changes
sandersn May 10, 2023
d74fcfc
revert unit tests too
sandersn May 10, 2023
407c245
remove unused baselines
sandersn May 10, 2023
11faca6
Add test cases for nodenext+ESM module resolution
sandersn May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
revert unit tests too
  • Loading branch information
@sandersn
sandersn committed May 10, 2023
commit d74fcfc9d43ca13cc93ebbbaf67d035337e0eab9
13 changes: 9 additions & 4 deletions src/testRunner/unittests/moduleResolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ describe("unittests:: moduleResolution:: Node module resolution - non-relative p

function test(hasDirectoryExists: boolean) {
const containingFile = { name: "/a/b/c/d/e.ts" };
const moduleFile = { name: "/a/b/node_modules/foo/index.ts" };
const moduleFile = { name: "/a/b/node_modules/foo.ts" };
baselines.push(`Resolving "foo" from ${containingFile.name}${hasDirectoryExists ? "" : " with host that doesnt have directoryExists"}`);
const resolution = ts.nodeModuleNameResolver("foo", containingFile.name, {}, createModuleResolutionHost(baselines, hasDirectoryExists, containingFile, moduleFile));
baselines.push(`Resolution:: ${JSON.stringify(resolution, /*replacer*/ undefined, 2)}`);
Expand All @@ -278,7 +278,7 @@ describe("unittests:: moduleResolution:: Node module resolution - non-relative p

function test(hasDirectoryExists: boolean) {
const containingFile = { name: "/a/b/c/d/e.ts" };
const moduleFile = { name: "/a/b/node_modules/foo/index.d.ts" };
const moduleFile = { name: "/a/b/node_modules/foo.d.ts" };
baselines.push(`Resolving "foo" from ${containingFile.name}${hasDirectoryExists ? "" : " with host that doesnt have directoryExists"}`);
const resolution = ts.nodeModuleNameResolver("foo", containingFile.name, {}, createModuleResolutionHost(baselines, hasDirectoryExists, containingFile, moduleFile));
baselines.push(`Resolution:: ${JSON.stringify(resolution, /*replacer*/ undefined, 2)}`);
Expand Down Expand Up @@ -672,7 +672,7 @@ describe("unittests:: moduleResolution:: baseUrl augmented module resolution", (
const m2: File = { name: "/root/m2/index.d.ts" }; // load folder as module
const m3: File = { name: "/root/m3/package.json", content: JSON.stringify({ typings: "dist/typings.d.ts" }) };
const m3Typings: File = { name: "/root/m3/dist/typings.d.ts" };
const m4: File = { name: "/root/node_modules/m4/index.ts" }; // fallback to node
const m4: File = { name: "/root/node_modules/m4.ts" }; // fallback to node

const options: ts.CompilerOptions = { moduleResolution: ts.ModuleResolutionKind.Node10, baseUrl: "/root" };
const host = createModuleResolutionHost(baselines, hasDirectoryExists, main, m1, m2, m3, m3Typings, m4);
Expand Down Expand Up @@ -732,7 +732,7 @@ describe("unittests:: moduleResolution:: baseUrl augmented module resolution", (
const file4Typings: File = { name: "/root/generated/folder2/file4/package.json", content: JSON.stringify({ typings: "dist/types.d.ts" }) };
const file4: File = { name: "/root/generated/folder2/file4/dist/types.d.ts" }; // load file pointed by typings
const file5: File = { name: "/root/someanotherfolder/file5/index.d.ts" }; // load remapped module from folder
const file6: File = { name: "/root/node_modules/file6/index.ts" }; // fallback to node
const file6: File = { name: "/root/node_modules/file6.ts" }; // fallback to node
const host = createModuleResolutionHost(baselines, hasDirectoryExists, file1, file2, file3, file4, file4Typings, file5, file6);

const options: ts.CompilerOptions = {
Expand Down Expand Up @@ -969,6 +969,11 @@ describe("unittests:: moduleResolution:: Type reference directive resolution: ",
});
it("Can be resolved from secondary location", () => {
const baselines: string[] = [];
{
const f1 = { name: "/root/src/app.ts" };
const f2 = { name: "/root/node_modules/lib.d.ts" };
test(baselines, /*typesRoot*/"/root/src/types", /* typeDirective */"lib", f1, f2);
}
{
const f1 = { name: "/root/src/app.ts" };
const f2 = { name: "/root/node_modules/lib/index.d.ts" };
Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/tscWatch/programUpdates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ export class A {
content: `import { T } from "module1";`
};
const nodeModuleFile: File = {
path: "/a/b/node_modules/module1/index.ts",
path: "/a/b/node_modules/module1.ts",
content: `export interface T {}`
};
const classicModuleFile: File = {
Expand Down
4 changes: 2 additions & 2 deletions src/testRunner/unittests/tsserver/configuredProjects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ describe("unittests:: tsserver:: ConfiguredProjects", () => {
content: `import { T } from "module1";`
};
const nodeModuleFile: File = {
path: "/a/b/node_modules/module1/index.ts",
path: "/a/b/node_modules/module1.ts",
content: `export interface T {}`
};
const classicModuleFile: File = {
Expand Down Expand Up @@ -1139,4 +1139,4 @@ describe("unittests:: tsserver:: ConfiguredProjects:: when reading tsconfig file

baselineTsserverLogs("configuredProjects", "should be tolerated without crashing the server when reading tsconfig file fails", session);
});
});
});
2 changes: 1 addition & 1 deletion src/testRunner/unittests/tsserver/events/projectUpdatedInBackground.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ describe("unittests:: tsserver:: events:: ProjectsUpdatedInBackground", () => {
content: 'import a from "file2"'
};
const file2: File = {
path: rootFolder + "a/b/node_modules/file2/index.d.ts",
path: rootFolder + "a/b/node_modules/file2.d.ts",
content: "export class a { }"
};
const file3: File = {
Expand Down
18 changes: 6 additions & 12 deletions tests/baselines/reference/moduleResolution/node-baseUrl-path-mappings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
//// [/root/someanotherfolder/file5/index.d.ts]


//// [/root/node_modules/file6/index.ts]
//// [/root/node_modules/file6.ts]


Resolving "folder1/file1" from /root/folder1/main.ts
Expand Down Expand Up @@ -129,7 +129,7 @@ Resolution:: {
Resolving "file6" from /root/folder1/main.ts
Resolution:: {
"resolvedModule": {
"resolvedFileName": "/root/node_modules/file6/index.ts",
"resolvedFileName": "/root/node_modules/file6.ts",
"extension": ".ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand Down Expand Up @@ -159,10 +159,7 @@ Resolution:: {
"/root/folder1/node_modules/@types/file6/package.json",
"/root/folder1/node_modules/@types/file6.d.ts",
"/root/folder1/node_modules/@types/file6/index.d.ts",
"/root/node_modules/file6/package.json",
"/root/node_modules/file6.ts",
"/root/node_modules/file6.tsx",
"/root/node_modules/file6.d.ts"
"/root/node_modules/file6/package.json"
]
}

Expand All @@ -184,7 +181,7 @@ Resolution:: {
//// [/root/someanotherfolder/file5/index.d.ts]


//// [/root/node_modules/file6/index.ts]
//// [/root/node_modules/file6.ts]


Resolving "folder1/file1" from /root/folder1/main.ts with host that doesnt have directoryExists
Expand Down Expand Up @@ -297,7 +294,7 @@ Resolution:: {
Resolving "file6" from /root/folder1/main.ts with host that doesnt have directoryExists
Resolution:: {
"resolvedModule": {
"resolvedFileName": "/root/node_modules/file6/index.ts",
"resolvedFileName": "/root/node_modules/file6.ts",
"extension": ".ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand Down Expand Up @@ -327,9 +324,6 @@ Resolution:: {
"/root/folder1/node_modules/@types/file6/package.json",
"/root/folder1/node_modules/@types/file6.d.ts",
"/root/folder1/node_modules/@types/file6/index.d.ts",
"/root/node_modules/file6/package.json",
"/root/node_modules/file6.ts",
"/root/node_modules/file6.tsx",
"/root/node_modules/file6.d.ts"
"/root/node_modules/file6/package.json"
]
}
18 changes: 6 additions & 12 deletions tests/baselines/reference/moduleResolution/node-baseUrl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//// [/root/m3/dist/typings.d.ts]


//// [/root/node_modules/m4/index.ts]
//// [/root/node_modules/m4.ts]


Resolving "m1" from /root/a/b/main.ts
Expand Down Expand Up @@ -65,7 +65,7 @@ Resolution:: {
Resolving "m4" from /root/a/b/main.ts
Resolution:: {
"resolvedModule": {
"resolvedFileName": "/root/node_modules/m4/index.ts",
"resolvedFileName": "/root/node_modules/m4.ts",
"extension": ".ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand Down Expand Up @@ -98,10 +98,7 @@ Resolution:: {
"/root/a/node_modules/@types/m4/package.json",
"/root/a/node_modules/@types/m4.d.ts",
"/root/a/node_modules/@types/m4/index.d.ts",
"/root/node_modules/m4/package.json",
"/root/node_modules/m4.ts",
"/root/node_modules/m4.tsx",
"/root/node_modules/m4.d.ts"
"/root/node_modules/m4/package.json"
]
}

Expand All @@ -120,7 +117,7 @@ Resolution:: {
//// [/root/m3/dist/typings.d.ts]


//// [/root/node_modules/m4/index.ts]
//// [/root/node_modules/m4.ts]


Resolving "m1" from /root/a/b/main.ts with host that doesnt have directoryExists
Expand Down Expand Up @@ -172,7 +169,7 @@ Resolution:: {
Resolving "m4" from /root/a/b/main.ts with host that doesnt have directoryExists
Resolution:: {
"resolvedModule": {
"resolvedFileName": "/root/node_modules/m4/index.ts",
"resolvedFileName": "/root/node_modules/m4.ts",
"extension": ".ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand Down Expand Up @@ -205,9 +202,6 @@ Resolution:: {
"/root/a/node_modules/@types/m4/package.json",
"/root/a/node_modules/@types/m4.d.ts",
"/root/a/node_modules/@types/m4/index.d.ts",
"/root/node_modules/m4/package.json",
"/root/node_modules/m4.ts",
"/root/node_modules/m4.tsx",
"/root/node_modules/m4.d.ts"
"/root/node_modules/m4/package.json"
]
}
18 changes: 6 additions & 12 deletions ...elines/reference/moduleResolution/non-relative-module-name-as-file-ts-files-not-loaded.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ Resolving "foo" from /a/b/c/d/e.ts
//// [/a/b/c/d/e.ts]


//// [/a/b/node_modules/foo/index.ts]
//// [/a/b/node_modules/foo.ts]


Resolution:: {
"resolvedModule": {
"resolvedFileName": "/a/b/node_modules/foo/index.ts",
"resolvedFileName": "/a/b/node_modules/foo.ts",
"extension": ".ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand All @@ -33,23 +33,20 @@ Resolution:: {
"/a/b/c/node_modules/@types/foo/package.json",
"/a/b/c/node_modules/@types/foo.d.ts",
"/a/b/c/node_modules/@types/foo/index.d.ts",
"/a/b/node_modules/foo/package.json",
"/a/b/node_modules/foo.ts",
"/a/b/node_modules/foo.tsx",
"/a/b/node_modules/foo.d.ts"
"/a/b/node_modules/foo/package.json"
]
}

Resolving "foo" from /a/b/c/d/e.ts with host that doesnt have directoryExists
//// [/a/b/c/d/e.ts]


//// [/a/b/node_modules/foo/index.ts]
//// [/a/b/node_modules/foo.ts]


Resolution:: {
"resolvedModule": {
"resolvedFileName": "/a/b/node_modules/foo/index.ts",
"resolvedFileName": "/a/b/node_modules/foo.ts",
"extension": ".ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand All @@ -75,9 +72,6 @@ Resolution:: {
"/a/b/c/node_modules/@types/foo/package.json",
"/a/b/c/node_modules/@types/foo.d.ts",
"/a/b/c/node_modules/@types/foo/index.d.ts",
"/a/b/node_modules/foo/package.json",
"/a/b/node_modules/foo.ts",
"/a/b/node_modules/foo.tsx",
"/a/b/node_modules/foo.d.ts"
"/a/b/node_modules/foo/package.json"
]
}
18 changes: 6 additions & 12 deletions tests/baselines/reference/moduleResolution/non-relative-module-name-as-file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ Resolving "foo" from /a/b/c/d/e.ts
//// [/a/b/c/d/e.ts]


//// [/a/b/node_modules/foo/index.d.ts]
//// [/a/b/node_modules/foo.d.ts]


Resolution:: {
"resolvedModule": {
"resolvedFileName": "/a/b/node_modules/foo/index.d.ts",
"resolvedFileName": "/a/b/node_modules/foo.d.ts",
"extension": ".d.ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand Down Expand Up @@ -35,23 +35,20 @@ Resolution:: {
"/a/b/c/node_modules/@types/foo/index.d.ts",
"/a/b/node_modules/foo/package.json",
"/a/b/node_modules/foo.ts",
"/a/b/node_modules/foo.tsx",
"/a/b/node_modules/foo.d.ts",
"/a/b/node_modules/foo/index.ts",
"/a/b/node_modules/foo/index.tsx"
"/a/b/node_modules/foo.tsx"
]
}

Resolving "foo" from /a/b/c/d/e.ts with host that doesnt have directoryExists
//// [/a/b/c/d/e.ts]


//// [/a/b/node_modules/foo/index.d.ts]
//// [/a/b/node_modules/foo.d.ts]


Resolution:: {
"resolvedModule": {
"resolvedFileName": "/a/b/node_modules/foo/index.d.ts",
"resolvedFileName": "/a/b/node_modules/foo.d.ts",
"extension": ".d.ts",
"isExternalLibraryImport": true,
"resolvedUsingTsExtension": false
Expand Down Expand Up @@ -79,9 +76,6 @@ Resolution:: {
"/a/b/c/node_modules/@types/foo/index.d.ts",
"/a/b/node_modules/foo/package.json",
"/a/b/node_modules/foo.ts",
"/a/b/node_modules/foo.tsx",
"/a/b/node_modules/foo.d.ts",
"/a/b/node_modules/foo/index.ts",
"/a/b/node_modules/foo/index.tsx"
"/a/b/node_modules/foo.tsx"
]
}
27 changes: 27 additions & 0 deletions tests/baselines/reference/moduleResolution/type-reference-from-secondary-location.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,33 @@
//// [/root/src/app.ts]


//// [/root/node_modules/lib.d.ts]


Resolving "lib" from /root/src/app.ts typesRoots: [/root/src/types] with host that doesnt have directoryExists
Resolution:: {
"resolvedTypeReferenceDirective": {
"primary": false,
"resolvedFileName": "/root/node_modules/lib.d.ts",
"isExternalLibraryImport": true
},
"failedLookupLocations": [
"/root/src/types/lib.d.ts",
"/root/src/types/lib/package.json",
"/root/src/types/lib/index.d.ts",
"/root/src/node_modules/lib/package.json",
"/root/src/node_modules/lib.d.ts",
"/root/src/node_modules/lib/index.d.ts",
"/root/src/node_modules/@types/lib/package.json",
"/root/src/node_modules/@types/lib.d.ts",
"/root/src/node_modules/@types/lib/index.d.ts",
"/root/node_modules/lib/package.json"
]
}

//// [/root/src/app.ts]


//// [/root/node_modules/lib/index.d.ts]


Expand Down
Loading