Skip to content

Reduce file lookups in node_modules module resolution #16568

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

Closed
wants to merge 6 commits into from
Closed

Reduce file lookups in node_modules module resolution #16568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a316827
Reduce file lookups in node_modules module resolution
Jun 15, 2017
80bbeea
Rename Extensions -> ExtensionsToTry
Jun 23, 2017
ed30611
Update baselines
Jun 26, 2017
60eee20
Merge branch 'master' into dts-first
Jul 14, 2017
2564ca6
Merge branch 'master' into dts-first
Jul 17, 2017
5140312
Merge branch 'master' into dts-first
Jan 9, 2018
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
175 changes: 101 additions & 74 deletions src/compiler/moduleNameResolver.ts
View file
Open in desktop

Large diffs are not rendered by default.

68 changes: 12 additions & 56 deletions src/harness/unittests/moduleResolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,36 +195,26 @@ namespace ts {

function test(hasDirectoryExists: boolean) {
const containingFile = { name: "/a/b/c/d/e.ts" };
const moduleFile = { name: "/a/b/node_modules/foo.ts" };
const moduleFile = { name: "/a/b/node_modules/foo/index.d.ts" };
const resolution = nodeModuleNameResolver("foo", containingFile.name, {}, createModuleResolutionHost(hasDirectoryExists, containingFile, moduleFile));
checkResolvedModuleWithFailedLookupLocations(resolution, createResolvedModule(moduleFile.name, /*isExternalLibraryImport*/ true), [
"/a/b/c/d/node_modules/foo/package.json",
"/a/b/c/d/node_modules/foo.ts",
"/a/b/c/d/node_modules/foo.tsx",
"/a/b/c/d/node_modules/foo.d.ts",
"/a/b/c/d/node_modules/foo/index.d.ts",

"/a/b/c/d/node_modules/foo/index.ts",
"/a/b/c/d/node_modules/foo/index.tsx",
"/a/b/c/d/node_modules/foo/index.d.ts",

"/a/b/c/d/node_modules/@types/foo/package.json",
"/a/b/c/d/node_modules/@types/foo.d.ts",

"/a/b/c/d/node_modules/@types/foo/index.d.ts",

"/a/b/c/node_modules/foo/package.json",
"/a/b/c/node_modules/foo.ts",
"/a/b/c/node_modules/foo.tsx",
"/a/b/c/node_modules/foo.d.ts",

"/a/b/c/node_modules/foo/index.d.ts",
"/a/b/c/node_modules/foo/index.ts",
"/a/b/c/node_modules/foo/index.tsx",
"/a/b/c/node_modules/foo/index.d.ts",

"/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",
]);
}
Expand Down Expand Up @@ -252,54 +242,30 @@ namespace ts {
const resolution = nodeModuleNameResolver("foo", containingFile.name, {}, createModuleResolutionHost(hasDirectoryExists, containingFile, moduleFile));
checkResolvedModuleWithFailedLookupLocations(resolution, createResolvedModule(moduleFile.name, /*isExternalLibraryImport*/ true), [
"/a/node_modules/b/c/node_modules/d/node_modules/foo/package.json",
"/a/node_modules/b/c/node_modules/d/node_modules/foo.ts",
"/a/node_modules/b/c/node_modules/d/node_modules/foo.tsx",
"/a/node_modules/b/c/node_modules/d/node_modules/foo.d.ts",

"/a/node_modules/b/c/node_modules/d/node_modules/foo/index.d.ts",
"/a/node_modules/b/c/node_modules/d/node_modules/foo/index.ts",
"/a/node_modules/b/c/node_modules/d/node_modules/foo/index.tsx",
"/a/node_modules/b/c/node_modules/d/node_modules/foo/index.d.ts",

"/a/node_modules/b/c/node_modules/d/node_modules/@types/foo/package.json",
"/a/node_modules/b/c/node_modules/d/node_modules/@types/foo.d.ts",

"/a/node_modules/b/c/node_modules/d/node_modules/@types/foo/index.d.ts",

"/a/node_modules/b/c/node_modules/foo/package.json",
"/a/node_modules/b/c/node_modules/foo.ts",
"/a/node_modules/b/c/node_modules/foo.tsx",
"/a/node_modules/b/c/node_modules/foo.d.ts",

"/a/node_modules/b/c/node_modules/foo/index.d.ts",
"/a/node_modules/b/c/node_modules/foo/index.ts",
"/a/node_modules/b/c/node_modules/foo/index.tsx",
"/a/node_modules/b/c/node_modules/foo/index.d.ts",

"/a/node_modules/b/c/node_modules/@types/foo/package.json",
"/a/node_modules/b/c/node_modules/@types/foo.d.ts",

"/a/node_modules/b/c/node_modules/@types/foo/index.d.ts",

"/a/node_modules/b/node_modules/foo/package.json",
"/a/node_modules/b/node_modules/foo.ts",
"/a/node_modules/b/node_modules/foo.tsx",
"/a/node_modules/b/node_modules/foo.d.ts",

"/a/node_modules/b/node_modules/foo/index.d.ts",
"/a/node_modules/b/node_modules/foo/index.ts",
"/a/node_modules/b/node_modules/foo/index.tsx",
"/a/node_modules/b/node_modules/foo/index.d.ts",

"/a/node_modules/b/node_modules/@types/foo/package.json",
"/a/node_modules/b/node_modules/@types/foo.d.ts",

"/a/node_modules/b/node_modules/@types/foo/index.d.ts",

"/a/node_modules/foo/package.json",
"/a/node_modules/foo.ts",
"/a/node_modules/foo.tsx",
"/a/node_modules/foo.d.ts",

"/a/node_modules/foo/index.ts",
"/a/node_modules/foo/index.tsx"
]);
}
});
Expand Down Expand Up @@ -604,7 +570,7 @@ import b = require("./moduleB");
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.ts" }; // fallback to node
const file6: File = { name: "/root/node_modules/file6/index.ts" }; // fallback to node
const host = createModuleResolutionHost(hasDirectoryExists, file1, file2, file3, file4, file4Typings, file5, file6);

const options: CompilerOptions = {
Expand Down Expand Up @@ -711,24 +677,19 @@ import b = require("./moduleB");
"/root/generated/file6/index.d.ts",

// fallback to standard node behavior
"/root/folder1/node_modules/file6/package.json",

// load from file
"/root/folder1/node_modules/file6.ts",
"/root/folder1/node_modules/file6.tsx",
"/root/folder1/node_modules/file6.d.ts",

// load from folder
"/root/folder1/node_modules/file6/package.json",
"/root/folder1/node_modules/file6/index.d.ts",
"/root/folder1/node_modules/file6/index.ts",
"/root/folder1/node_modules/file6/index.tsx",
"/root/folder1/node_modules/file6/index.d.ts",

"/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",
// success on /root/node_modules/file6.ts
"/root/node_modules/file6/index.d.ts",
// success on /root/node_modules/file6/index.ts
], /*isExternalLibraryImport*/ true);

function check(name: string, expected: File, expectedFailedLookups: string[], isExternalLibraryImport = false) {
Expand Down Expand Up @@ -1009,11 +970,6 @@ import b = require("./moduleB");
}
});
it("Can be resolved from secondary location", () => {
{
const f1 = { name: "/root/src/app.ts" };
const f2 = { name: "/root/node_modules/lib.d.ts" };
test(/*typesRoot*/"/root/src/types", /* typeDirective */"lib", /*primary*/ false, f1, f2);
}
{
const f1 = { name: "/root/src/app.ts" };
const f2 = { name: "/root/node_modules/lib/index.d.ts" };
Expand Down
23 changes: 3 additions & 20 deletions src/harness/unittests/reuseProgramStructure.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,21 +454,15 @@ namespace ts {
[
"======== Resolving module 'a' from 'file1.ts'. ========",
"Explicitly specified module resolution kind: 'NodeJs'.",
"Loading module 'a' from 'node_modules' folder, target file type 'TypeScript'.",
"Loading module 'a' from 'node_modules' folder, target file type 'TypeScript ('.ts' preferred)'.",
"File 'node_modules/a/package.json' does not exist.",
"File 'node_modules/a.ts' does not exist.",
"File 'node_modules/a.tsx' does not exist.",
"File 'node_modules/a.d.ts' does not exist.",
"File 'node_modules/a/index.d.ts' does not exist.",
"File 'node_modules/a/index.ts' does not exist.",
"File 'node_modules/a/index.tsx' does not exist.",
"File 'node_modules/a/index.d.ts' does not exist.",
"File 'node_modules/@types/a/package.json' does not exist.",
"File 'node_modules/@types/a.d.ts' does not exist.",
"File 'node_modules/@types/a/index.d.ts' does not exist.",
"Loading module 'a' from 'node_modules' folder, target file type 'JavaScript'.",
"File 'node_modules/a/package.json' does not exist.",
"File 'node_modules/a.js' does not exist.",
"File 'node_modules/a.jsx' does not exist.",
"File 'node_modules/a/index.js' does not exist.",
"File 'node_modules/a/index.jsx' does not exist.",
"======== Module name 'a' was not resolved. ========"
Expand All @@ -487,13 +481,8 @@ namespace ts {
[
"======== Resolving module 'a' from 'file1.ts'. ========",
"Explicitly specified module resolution kind: 'NodeJs'.",
"Loading module 'a' from 'node_modules' folder, target file type 'TypeScript'.",
"Loading module 'a' from 'node_modules' folder, target file type 'TypeScript ('.ts' preferred)'.",
"File 'node_modules/a/package.json' does not exist.",
"File 'node_modules/a.ts' does not exist.",
"File 'node_modules/a.tsx' does not exist.",
"File 'node_modules/a.d.ts' does not exist.",
"File 'node_modules/a/index.ts' does not exist.",
"File 'node_modules/a/index.tsx' does not exist.",
"File 'node_modules/a/index.d.ts' exist - use it as a name resolution result.",
"======== Module name 'a' was successfully resolved to 'node_modules/a/index.d.ts'. ========"
],
Expand Down Expand Up @@ -525,13 +514,10 @@ namespace ts {
"File '/fs.tsx' does not exist.",
"File '/fs.d.ts' does not exist.",
"File '/a/b/node_modules/@types/fs/package.json' does not exist.",
"File '/a/b/node_modules/@types/fs.d.ts' does not exist.",
"File '/a/b/node_modules/@types/fs/index.d.ts' does not exist.",
"File '/a/node_modules/@types/fs/package.json' does not exist.",
"File '/a/node_modules/@types/fs.d.ts' does not exist.",
"File '/a/node_modules/@types/fs/index.d.ts' does not exist.",
"File '/node_modules/@types/fs/package.json' does not exist.",
"File '/node_modules/@types/fs.d.ts' does not exist.",
"File '/node_modules/@types/fs/index.d.ts' does not exist.",
"File '/a/b/fs.js' does not exist.",
"File '/a/b/fs.jsx' does not exist.",
Expand Down Expand Up @@ -567,13 +553,10 @@ namespace ts {
"File '/fs.tsx' does not exist.",
"File '/fs.d.ts' does not exist.",
"File '/a/b/node_modules/@types/fs/package.json' does not exist.",
"File '/a/b/node_modules/@types/fs.d.ts' does not exist.",
"File '/a/b/node_modules/@types/fs/index.d.ts' does not exist.",
"File '/a/node_modules/@types/fs/package.json' does not exist.",
"File '/a/node_modules/@types/fs.d.ts' does not exist.",
"File '/a/node_modules/@types/fs/index.d.ts' does not exist.",
"File '/node_modules/@types/fs/package.json' does not exist.",
"File '/node_modules/@types/fs.d.ts' does not exist.",
"File '/node_modules/@types/fs/index.d.ts' does not exist.",
"File '/a/b/fs.js' does not exist.",
"File '/a/b/fs.jsx' does not exist.",
Expand Down
2 changes: 1 addition & 1 deletion src/harness/unittests/tscWatchMode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ namespace ts.tscWatch {
content: `import { T } from "module1";`
};
const nodeModuleFile: FileOrFolder = {
path: "/a/b/node_modules/module1.ts",
path: "/a/b/node_modules/module1/index.d.ts",
content: `export interface T {}`
};
const classicModuleFile: FileOrFolder = {
Expand Down
8 changes: 3 additions & 5 deletions src/harness/unittests/tsserverProjectSystem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,7 @@ namespace ts.projectSystem {
content: `import { T } from "module1";`
};
const nodeModuleFile: FileOrFolder = {
path: "/a/b/node_modules/module1.ts",
path: "/a/b/node_modules/module1/index.ts",
content: `export interface T {}`
};
const classicModuleFile: FileOrFolder = {
Expand Down Expand Up @@ -3024,7 +3024,7 @@ namespace ts.projectSystem {
assert.deepEqual(resolutionTrace, [
"======== Resolving module 'lib' from '/a/b/app.js'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module 'lib' from 'node_modules' folder, target file type 'TypeScript'.",
"Loading module 'lib' from 'node_modules' folder, target file type 'TypeScript ('.ts' preferred)'.",
"Directory '/a/b/node_modules' does not exist, skipping all lookups in it.",
"Directory '/a/node_modules' does not exist, skipping all lookups in it.",
"Directory '/node_modules' does not exist, skipping all lookups in it.",
Expand All @@ -3034,9 +3034,7 @@ namespace ts.projectSystem {
"Directory '/node_modules' does not exist, skipping all lookups in it.",
"======== Module name 'lib' was not resolved. ========",
`Auto discovery for typings is enabled in project '${proj.getProjectName()}'. Running extra resolution pass for module 'lib' using cache location '/a/cache'.`,
"File '/a/cache/node_modules/lib.d.ts' does not exist.",
"File '/a/cache/node_modules/@types/lib/package.json' does not exist.",
"File '/a/cache/node_modules/@types/lib.d.ts' does not exist.",
"File '/a/cache/node_modules/@types/lib/index.d.ts' exist - use it as a name resolution result.",
]);
checkProjectActualFiles(proj, [file1.path, lib.path]);
Expand Down Expand Up @@ -6279,7 +6277,7 @@ namespace ts.projectSystem {
content: 'import a from "file2"'
};
const file2: FileOrFolder = {
path: rootFolder + "a/b/node_modules/file2.d.ts",
path: rootFolder + "a/b/node_modules/file2/index.d.ts",
content: "export class a { }"
};
const file3: FileOrFolder = {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/cachedModuleResolution1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//// [tests/cases/compiler/cachedModuleResolution1.ts] ////

//// [foo.d.ts]
//// [index.d.ts]
export declare let x: number

//// [app.ts]
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/cachedModuleResolution1.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== /a/b/node_modules/foo.d.ts ===
=== /a/b/node_modules/foo/index.d.ts ===
export declare let x: number
>x : Symbol(x, Decl(foo.d.ts, 0, 18))
>x : Symbol(x, Decl(index.d.ts, 0, 18))

=== /a/b/c/d/e/app.ts ===
import {x} from "foo";
Expand Down
17 changes: 8 additions & 9 deletions tests/baselines/reference/cachedModuleResolution1.trace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
[
"======== Resolving module 'foo' from '/a/b/c/d/e/app.ts'. ========",
"Explicitly specified module resolution kind: 'NodeJs'.",
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript ('.ts' preferred)'.",
"Directory '/a/b/c/d/e/node_modules' does not exist, skipping all lookups in it.",
"Directory '/a/b/c/d/node_modules' does not exist, skipping all lookups in it.",
"Directory '/a/b/c/node_modules' does not exist, skipping all lookups in it.",
"File '/a/b/node_modules/foo.ts' does not exist.",
"File '/a/b/node_modules/foo.tsx' does not exist.",
"File '/a/b/node_modules/foo.d.ts' exist - use it as a name resolution result.",
"Resolving real path for '/a/b/node_modules/foo.d.ts', result '/a/b/node_modules/foo.d.ts'.",
"======== Module name 'foo' was successfully resolved to '/a/b/node_modules/foo.d.ts'. ========",
"File '/a/b/node_modules/foo/package.json' does not exist.",
"File '/a/b/node_modules/foo/index.d.ts' exist - use it as a name resolution result.",
"Resolving real path for '/a/b/node_modules/foo/index.d.ts', result '/a/b/node_modules/foo/index.d.ts'.",
"======== Module name 'foo' was successfully resolved to '/a/b/node_modules/foo/index.d.ts'. ========",
"======== Resolving module 'foo' from '/a/b/c/lib.ts'. ========",
"Explicitly specified module resolution kind: 'NodeJs'.",
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript ('.ts' preferred)'.",
"Resolution for module 'foo' was found in cache.",
"Resolving real path for '/a/b/node_modules/foo.d.ts', result '/a/b/node_modules/foo.d.ts'.",
"======== Module name 'foo' was successfully resolved to '/a/b/node_modules/foo.d.ts'. ========"
"Resolving real path for '/a/b/node_modules/foo/index.d.ts', result '/a/b/node_modules/foo/index.d.ts'.",
"======== Module name 'foo' was successfully resolved to '/a/b/node_modules/foo/index.d.ts'. ========"
]
2 changes: 1 addition & 1 deletion tests/baselines/reference/cachedModuleResolution1.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
=== /a/b/node_modules/foo.d.ts ===
=== /a/b/node_modules/foo/index.d.ts ===
export declare let x: number
>x : number

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/cachedModuleResolution2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//// [tests/cases/compiler/cachedModuleResolution2.ts] ////

//// [foo.d.ts]
//// [index.d.ts]
export declare let x: number

//// [lib.ts]
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/cachedModuleResolution2.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== /a/b/node_modules/foo.d.ts ===
=== /a/b/node_modules/foo/index.d.ts ===
export declare let x: number
>x : Symbol(x, Decl(foo.d.ts, 0, 18))
>x : Symbol(x, Decl(index.d.ts, 0, 18))

=== /a/b/c/lib.ts ===
import {x} from "foo";
Expand Down
Loading