Reduce file lookups in node_modules module resolution

Author: Andy Hanson

src/compiler/moduleNameResolver.ts

@@ -32,10 +32,27 @@ namespace ts {
      * Kinds of file that we are currently looking for.
      * Typically there is one pass with Extensions.TypeScript, then a second pass with Extensions.JavaScript.
      */
-    enum Extensions {
-        TypeScript, /** '.ts', '.tsx', or '.d.ts' */
-        JavaScript, /** '.js' or '.jsx' */
-        DtsOnly /** Only '.d.ts' */
+    const enum Extensions {
+        /**
+         * '.ts', '.tsx', or '.d.ts'
+         * This is for regular imports of TypeScript code.
+         */
+        TypeScript = "TypeScript",
+        /**
+         * Same as TypeScript, but look for a '.d.ts' file first.
+         * This is used inside node_modules (in the first phase, before we look for '.js')
+         */
+        TypeScriptDtsFirst = "TypeScript ('.d.ts' preferred)",
+        /**
+         * '.js' or '.jsx'
+         * This is done only after either TypeScript or TypeScriptDtsFirst has been tried.
+         */
+        JavaScript = "JavaScript",
+        /**
+         * Only '.d.ts'
+         * This is used for lookups in '@types'
+         */
+        DtsOnly = "'.d.ts' only"
     }
 
     /** Used with `Extensions.DtsOnly` to extract the path from TypeScript results. */
@@ -717,7 +734,7 @@ namespace ts {
 
             if (moduleHasNonRelativeName(moduleName)) {
                 if (traceEnabled) {
-                    trace(host, Diagnostics.Loading_module_0_from_node_modules_folder_target_file_type_1, moduleName, Extensions[extensions]);
+                    trace(host, Diagnostics.Loading_module_0_from_node_modules_folder_target_file_type_1, moduleName, extensions);
                 }
                 const resolved = loadModuleFromNodeModules(extensions, moduleName, containingDirectory, failedLookupLocations, state, cache);
                 // For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files.
@@ -745,7 +762,7 @@ namespace ts {
 
     function nodeLoadModuleByRelativeName(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson: boolean): Resolved | undefined {
         if (state.traceEnabled) {
-            trace(state.host, Diagnostics.Loading_module_as_file_Slash_folder_candidate_module_location_0_target_file_type_1, candidate, Extensions[extensions]);
+            trace(state.host, Diagnostics.Loading_module_as_file_Slash_folder_candidate_module_location_0_target_file_type_1, candidate, extensions);
         }
         if (!pathEndsWithDirectorySeparator(candidate)) {
             if (!onlyRecordFailures) {
@@ -818,6 +835,8 @@ namespace ts {
                 return tryExtension(Extension.Dts);
             case Extensions.TypeScript:
                 return tryExtension(Extension.Ts) || tryExtension(Extension.Tsx) || tryExtension(Extension.Dts);
+            case Extensions.TypeScriptDtsFirst:
+                return tryExtension(Extension.Dts) || tryExtension(Extension.Ts) || tryExtension(Extension.Tsx);
             case Extensions.JavaScript:
                 return tryExtension(Extension.Js) || tryExtension(Extension.Jsx);
         }
@@ -920,22 +939,26 @@ namespace ts {
         return combinePaths(directory, "package.json");
     }
 
-    function loadModuleFromNodeModulesFolder(extensions: Extensions, moduleName: string, nodeModulesFolder: string, nodeModulesFolderExists: boolean, failedLookupLocations: Push<string>, state: ModuleResolutionState): Resolved | undefined {
-        const candidate = normalizePath(combinePaths(nodeModulesFolder, moduleName));
+    // Inside of node_modules we should be looking for '.d.ts' files before '.ts' files.
+    type NodeModulesExtensions = Extensions.TypeScriptDtsFirst | Extensions.JavaScript | Extensions.DtsOnly;
 
-        return loadModuleFromFile(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state) ||
-            loadNodeModuleFromDirectory(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state);
+    function loadModuleFromNodeModulesFolder(extensions: NodeModulesExtensions, moduleName: string, nodeModulesFolder: string, nodeModulesFolderExists: boolean, failedLookupLocations: Push<string>, state: ModuleResolutionState): Resolved | undefined {
+        const packageDir = normalizePath(combinePaths(nodeModulesFolder, moduleName));
+        return loadNodeModuleFromDirectory(extensions, packageDir, failedLookupLocations, !nodeModulesFolderExists, state);
     }
 
     function loadModuleFromNodeModules(extensions: Extensions, moduleName: string, directory: string, failedLookupLocations: Push<string>, state: ModuleResolutionState, cache: NonRelativeModuleNameResolutionCache): SearchResult<Resolved> {
-        return loadModuleFromNodeModulesWorker(extensions, moduleName, directory, failedLookupLocations, state, /*typesOnly*/ false, cache);
+        // In node_modules, we will look for a '.d.ts' file first.
+        const ext = extensions === Extensions.TypeScript ? Extensions.TypeScriptDtsFirst : extensions;
+        return loadModuleFromNodeModulesWorker(ext, moduleName, directory, failedLookupLocations, state, /*typesOnly*/ false, cache);
     }
+
     function loadModuleFromNodeModulesAtTypes(moduleName: string, directory: string, failedLookupLocations: Push<string>, state: ModuleResolutionState): SearchResult<Resolved> {
         // Extensions parameter here doesn't actually matter, because typesOnly ensures we're just doing @types lookup, which is always DtsOnly.
         return loadModuleFromNodeModulesWorker(Extensions.DtsOnly, moduleName, directory, failedLookupLocations, state, /*typesOnly*/ true, /*cache*/ undefined);
     }
 
-    function loadModuleFromNodeModulesWorker(extensions: Extensions, moduleName: string, directory: string, failedLookupLocations: Push<string>, state: ModuleResolutionState, typesOnly: boolean, cache: NonRelativeModuleNameResolutionCache): SearchResult<Resolved> {
+    function loadModuleFromNodeModulesWorker(extensions: NodeModulesExtensions, moduleName: string, directory: string, failedLookupLocations: Push<string>, state: ModuleResolutionState, typesOnly: boolean, cache: NonRelativeModuleNameResolutionCache): SearchResult<Resolved> {
         const perModuleNameCache = cache && cache.getOrCreateCacheForModuleName(moduleName);
         return forEachAncestorDirectory(normalizeSlashes(directory), ancestorDirectory => {
             if (getBaseFileName(ancestorDirectory) !== "node_modules") {
@@ -949,7 +972,7 @@ namespace ts {
     }
 
     /** Load a module from a single node_modules directory, but not from any ancestors' node_modules directories. */
-    function loadModuleFromNodeModulesOneLevel(extensions: Extensions, moduleName: string, directory: string, failedLookupLocations: Push<string>, state: ModuleResolutionState, typesOnly = false): Resolved | undefined {
+    function loadModuleFromNodeModulesOneLevel(extensions: NodeModulesExtensions, moduleName: string, directory: string, failedLookupLocations: Push<string>, state: ModuleResolutionState, typesOnly = false): Resolved | undefined {
         const nodeModulesFolder = combinePaths(directory, "node_modules");
         const nodeModulesFolderExists = directoryProbablyExists(nodeModulesFolder, state.host);
         if (!nodeModulesFolderExists && state.traceEnabled) {

src/harness/unittests/moduleResolution.ts

@@ -182,36 +182,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.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/package.json",
-
+                    "/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.d.ts",
                     "/a/b/c/d/node_modules/@types/foo/package.json",
-
                     "/a/b/c/d/node_modules/@types/foo/index.d.ts",
 
-                    "/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/package.json",
-
+                    "/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.d.ts",
                     "/a/b/c/node_modules/@types/foo/package.json",
-
                     "/a/b/c/node_modules/@types/foo/index.d.ts",
+
+                    "/a/b/node_modules/foo/package.json",
                 ]);
             }
         });
@@ -237,55 +227,31 @@ namespace ts {
                 const moduleFile = { name: "/a/node_modules/foo/index.d.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.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/package.json",
-
+                    "/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.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/index.d.ts",
 
-                    "/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/package.json",
-
+                    "/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.d.ts",
                     "/a/node_modules/b/c/node_modules/@types/foo/package.json",
-
                     "/a/node_modules/b/c/node_modules/@types/foo/index.d.ts",
 
-                    "/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/package.json",
-
+                    "/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.d.ts",
                     "/a/node_modules/b/node_modules/@types/foo/package.json",
-
                     "/a/node_modules/b/node_modules/@types/foo/index.d.ts",
 
-                    "/a/node_modules/foo.ts",
-                    "/a/node_modules/foo.tsx",
-                    "/a/node_modules/foo.d.ts",
                     "/a/node_modules/foo/package.json",
-
-                    "/a/node_modules/foo/index.ts",
-                    "/a/node_modules/foo/index.tsx"
                 ]);
             }
         });
@@ -574,7 +540,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 = {
@@ -681,22 +647,19 @@ import b = require("./moduleB");
                     "/root/generated/file6/index.d.ts",
 
                     // fallback to standard node behavior
-                    // 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.d.ts",
 
                     "/root/folder1/node_modules/@types/file6/package.json",
                     "/root/folder1/node_modules/@types/file6/index.d.ts",
-                    // success on /root/node_modules/file6.ts
+
+                    "/root/node_modules/file6/package.json",
+                    "/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) {
@@ -977,11 +940,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" };