Auto type reference directive shouldnt lookup in node_modules

Fixes #37708
Bug 1 part of the issue

Author: sheetalkamat

src/compiler/diagnosticMessages.json

@@ -5154,6 +5154,10 @@
         "category": "Message",
         "code": 6261
     },
+    "Resolving type reference directive for program that specifies custom typeRoots, skipping lookup in 'node_modules' folder.": {
+        "category": "Message",
+        "code": 6262
+    },
 
     "Directory '{0}' has no containing package.json scope. Imports will not resolve.": {
         "category": "Message",

src/compiler/moduleNameResolver.ts

@@ -54,6 +54,7 @@ import {
     hasProperty,
     hasTrailingDirectorySeparator,
     hostGetCanonicalFileName,
+    inferredTypesContainingFile,
     isArray,
     isExternalModuleNameRelative,
     isRootedDiskPath,
@@ -396,27 +397,19 @@ export function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffecti
     }
 
     if (currentDirectory !== undefined) {
-        return getDefaultTypeRoots(currentDirectory, host);
+        return getDefaultTypeRoots(currentDirectory);
     }
 }
 
 /**
  * Returns the path to every node_modules/@types directory from some ancestor directory.
  * Returns undefined if there are none.
  */
-function getDefaultTypeRoots(currentDirectory: string, host: { directoryExists?: (directoryName: string) => boolean }): string[] | undefined {
-    if (!host.directoryExists) {
-        return [combinePaths(currentDirectory, nodeModulesAtTypes)];
-        // And if it doesn't exist, tough.
-    }
-
+function getDefaultTypeRoots(currentDirectory: string): string[] | undefined {
     let typeRoots: string[] | undefined;
     forEachAncestorDirectory(normalizePath(currentDirectory), directory => {
         const atTypes = combinePaths(directory, nodeModulesAtTypes);
-        if (host.directoryExists!(atTypes)) {
-            (typeRoots || (typeRoots = [])).push(atTypes);
-        }
-        return undefined;
+        (typeRoots ??= []).push(atTypes);
     });
     return typeRoots;
 }
@@ -576,20 +569,24 @@ export function resolveTypeReferenceDirective(typeReferenceDirectiveName: string
 
     function secondaryLookup(): PathAndPackageId | undefined {
         const initialLocationForSecondaryLookup = containingFile && getDirectoryPath(containingFile);
-
         if (initialLocationForSecondaryLookup !== undefined) {
-            // check secondary locations
-            if (traceEnabled) {
-                trace(host, Diagnostics.Looking_up_in_node_modules_folder_initial_location_0, initialLocationForSecondaryLookup);
-            }
             let result: Resolved | undefined;
-            if (!isExternalModuleNameRelative(typeReferenceDirectiveName)) {
-                const searchResult = loadModuleFromNearestNodeModulesDirectory(Extensions.Declaration, typeReferenceDirectiveName, initialLocationForSecondaryLookup, moduleResolutionState, /*cache*/ undefined, /*redirectedReference*/ undefined);
-                result = searchResult && searchResult.value;
+            if (!options.typeRoots || !endsWith(containingFile!, inferredTypesContainingFile)) {
+                // check secondary locations
+                if (traceEnabled) {
+                    trace(host, Diagnostics.Looking_up_in_node_modules_folder_initial_location_0, initialLocationForSecondaryLookup);
+                }
+                if (!isExternalModuleNameRelative(typeReferenceDirectiveName)) {
+                    const searchResult = loadModuleFromNearestNodeModulesDirectory(Extensions.Declaration, typeReferenceDirectiveName, initialLocationForSecondaryLookup, moduleResolutionState, /*cache*/ undefined, /*redirectedReference*/ undefined);
+                    result = searchResult && searchResult.value;
+                }
+                else {
+                    const { path: candidate } = normalizePathForCJSResolution(initialLocationForSecondaryLookup, typeReferenceDirectiveName);
+                    result = nodeLoadModuleByRelativeName(Extensions.Declaration, candidate, /*onlyRecordFailures*/ false, moduleResolutionState, /*considerPackageJson*/ true);
+                }
             }
-            else {
-                const { path: candidate } = normalizePathForCJSResolution(initialLocationForSecondaryLookup, typeReferenceDirectiveName);
-                result = nodeLoadModuleByRelativeName(Extensions.Declaration, candidate, /*onlyRecordFailures*/ false, moduleResolutionState, /*considerPackageJson*/ true);
+            else if (traceEnabled) {
+                trace(host, Diagnostics.Resolving_type_reference_directive_for_program_that_specifies_custom_typeRoots_skipping_lookup_in_node_modules_folder);
             }
             return resolvedTypeScriptOnly(result);
         }

src/compiler/resolutionCache.ts

@@ -1157,26 +1157,28 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
 
     function createTypeRootsWatch(typeRootPath: Path, typeRoot: string): FileWatcher {
         // Create new watch and recursive info
-        return resolutionHost.watchTypeRootsDirectory(typeRoot, fileOrDirectory => {
-            const fileOrDirectoryPath = resolutionHost.toPath(fileOrDirectory);
-            if (cachedDirectoryStructureHost) {
-                // Since the file existence changed, update the sourceFiles cache
-                cachedDirectoryStructureHost.addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath);
-            }
-
-            // For now just recompile
-            // We could potentially store more data here about whether it was/would be really be used or not
-            // and with that determine to trigger compilation but for now this is enough
-            hasChangedAutomaticTypeDirectiveNames = true;
-            resolutionHost.onChangedAutomaticTypeDirectiveNames();
+        return canWatchTypeRootPath(typeRootPath) ?
+            resolutionHost.watchTypeRootsDirectory(typeRoot, fileOrDirectory => {
+                const fileOrDirectoryPath = resolutionHost.toPath(fileOrDirectory);
+                if (cachedDirectoryStructureHost) {
+                    // Since the file existence changed, update the sourceFiles cache
+                    cachedDirectoryStructureHost.addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath);
+                }
 
-            // Since directory watchers invoked are flaky, the failed lookup location events might not be triggered
-            // So handle to failed lookup locations here as well to ensure we are invalidating resolutions
-            const dirPath = getDirectoryToWatchFailedLookupLocationFromTypeRoot(typeRoot, typeRootPath);
-            if (dirPath) {
-                scheduleInvalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath);
-            }
-        }, WatchDirectoryFlags.Recursive);
+                // For now just recompile
+                // We could potentially store more data here about whether it was/would be really be used or not
+                // and with that determine to trigger compilation but for now this is enough
+                hasChangedAutomaticTypeDirectiveNames = true;
+                resolutionHost.onChangedAutomaticTypeDirectiveNames();
+
+                // Since directory watchers invoked are flaky, the failed lookup location events might not be triggered
+                // So handle to failed lookup locations here as well to ensure we are invalidating resolutions
+                const dirPath = getDirectoryToWatchFailedLookupLocationFromTypeRoot(typeRoot, typeRootPath);
+                if (dirPath) {
+                    scheduleInvalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath);
+                }
+            }, WatchDirectoryFlags.Recursive) :
+            noopFileWatcher;
     }
 
     /**
@@ -1194,7 +1196,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
 
         // we need to assume the directories exist to ensure that we can get all the type root directories that get included
         // But filter directories that are at root level to say directory doesnt exist, so that we arent watching them
-        const typeRoots = getEffectiveTypeRoots(options, { directoryExists: directoryExistsForTypeRootWatch, getCurrentDirectory });
+        const typeRoots = getEffectiveTypeRoots(options, { getCurrentDirectory });
         if (typeRoots) {
             mutateMap(
                 typeRootsWatches,
@@ -1210,12 +1212,11 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
         }
     }
 
-    /**
-     * Use this function to return if directory exists to get type roots to watch
-     * If we return directory exists then only the paths will be added to type roots
-     * Hence return true for all directories except root directories which are filtered from watching
-     */
-    function directoryExistsForTypeRootWatch(nodeTypesDirectory: string) {
+    function canWatchTypeRootPath(nodeTypesDirectory: string) {
+        // If type roots is specified, watch that path
+        if (resolutionHost.getCompilationSettings().typeRoots) return true;
+
+        // Otherwise can watch directory only if we can watch the parent directory of node_modules/@types
         const dir = getDirectoryPath(getDirectoryPath(nodeTypesDirectory));
         const dirPath = resolutionHost.toPath(dir);
         return dirPath === rootPath || canWatchDirectoryOrFile(dirPath);

src/compiler/types.ts

@@ -9049,7 +9049,6 @@ export interface EmitTextWriter extends SymbolWriter {
 }
 
 export interface GetEffectiveTypeRootsHost {
-    directoryExists?(directoryName: string): boolean;
     getCurrentDirectory?(): string;
 }
 

src/server/project.ts

@@ -2825,7 +2825,7 @@ export class ConfiguredProject extends Project {
     }
 
     getEffectiveTypeRoots() {
-        return getEffectiveTypeRoots(this.getCompilationSettings(), this.directoryStructureHost) || [];
+        return getEffectiveTypeRoots(this.getCompilationSettings(), this) || [];
     }
 
     /** @internal */

src/testRunner/unittests/tscWatch/programUpdates.ts

@@ -937,29 +937,37 @@ declare const eval: any`
         ]
     });
 
-    verifyTscWatch({
-        scenario,
-        subScenario: "types should load from config file path if config exists",
-        commandLineArgs: ["-w", "-p", configFilePath],
-        sys: () => {
-            const f1 = {
-                path: "/a/b/app.ts",
-                content: "let x = 1"
-            };
-            const config = {
-                path: configFilePath,
-                content: JSON.stringify({ compilerOptions: { types: ["node"], typeRoots: [] } })
-            };
-            const node = {
-                path: "/a/b/node_modules/@types/node/index.d.ts",
-                content: "declare var process: any"
-            };
-            const cwd = {
-                path: "/a/c"
-            };
-            return createWatchedSystem([f1, config, node, cwd, libFile], { currentDirectory: cwd.path });
-        },
-        changes: ts.emptyArray
+    describe("types from config file", () => {
+        function verifyTypesLoad(includeTypeRoots: boolean) {
+            verifyTscWatch({
+                scenario,
+                subScenario: includeTypeRoots ?
+                    "types should not load from config file path if config exists but does not specifies typeRoots" :
+                    "types should load from config file path if config exists",
+                commandLineArgs: ["-w", "-p", configFilePath],
+                sys: () => {
+                    const f1 = {
+                        path: "/a/b/app.ts",
+                        content: "let x = 1"
+                    };
+                    const config = {
+                        path: configFilePath,
+                        content: JSON.stringify({ compilerOptions: { types: ["node"], typeRoots: includeTypeRoots ? [] : undefined } })
+                    };
+                    const node = {
+                        path: "/a/b/node_modules/@types/node/index.d.ts",
+                        content: "declare var process: any"
+                    };
+                    const cwd = {
+                        path: "/a/c"
+                    };
+                    return createWatchedSystem([f1, config, node, cwd, libFile], { currentDirectory: cwd.path });
+                },
+                changes: ts.emptyArray
+            });
+        }
+        verifyTypesLoad(/*includeTypeRoots*/ false);
+        verifyTypesLoad(/*includeTypeRoots*/ true);
     });
 
     verifyTscWatch({

src/testRunner/unittests/tsserver/resolutionCache.ts

@@ -345,27 +345,32 @@ describe("unittests:: tsserver:: resolutionCache:: tsserverProjectSystem rename
         projectService.checkNumberOfProjects({ configuredProjects: 1 });
     });
 
-    it("types should load from config file path if config exists", () => {
-        const f1 = {
-            path: "/a/b/app.ts",
-            content: "let x = 1"
-        };
-        const config = {
-            path: "/a/b/tsconfig.json",
-            content: JSON.stringify({ compilerOptions: { types: ["node"], typeRoots: [] } })
-        };
-        const node = {
-            path: "/a/b/node_modules/@types/node/index.d.ts",
-            content: "declare var process: any"
-        };
-        const cwd = {
-            path: "/a/c"
-        };
-        const host = createServerHost([f1, config, node, cwd], { currentDirectory: cwd.path });
-        const projectService = createProjectService(host);
-        projectService.openClientFile(f1.path);
-        projectService.checkNumberOfProjects({ configuredProjects: 1 });
-        checkProjectActualFiles(configuredProjectAt(projectService, 0), [f1.path, node.path, config.path]);
+    describe("types from config file", () => {
+        function verifyTypesLoad(subScenario: string, includeTypeRoots: boolean) {
+            it(subScenario, () => {
+                const f1 = {
+                    path: "/a/b/app.ts",
+                    content: "let x = 1"
+                };
+                const config = {
+                    path: "/a/b/tsconfig.json",
+                    content: JSON.stringify({ compilerOptions: { types: ["node"], typeRoots: includeTypeRoots ? [] : undefined } })
+                };
+                const node = {
+                    path: "/a/b/node_modules/@types/node/index.d.ts",
+                    content: "declare var process: any"
+                };
+                const cwd = {
+                    path: "/a/c"
+                };
+                const host = createServerHost([f1, config, node, cwd], { currentDirectory: cwd.path });
+                const projectService = createProjectService(host, { logger: createLoggerWithInMemoryLogs(host) });
+                projectService.openClientFile(f1.path);
+                baselineTsserverLogs("resolutionCache", subScenario, projectService);
+            });
+        }
+        verifyTypesLoad("types should load from config file path if config exists", /*includeTypeRoots*/ false);
+        verifyTypesLoad("types should not load from config file path if config exists but does not specifies typeRoots", /*includeTypeRoots*/ true);
     });
 });
 

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -8258,7 +8258,6 @@ declare namespace ts {
         noEmitHelpers?: boolean;
     }
     interface GetEffectiveTypeRootsHost {
-        directoryExists?(directoryName: string): boolean;
         getCurrentDirectory?(): string;
     }
     interface TextSpan {

tests/baselines/reference/api/typescript.d.ts

@@ -4322,7 +4322,6 @@ declare namespace ts {
         noEmitHelpers?: boolean;
     }
     interface GetEffectiveTypeRootsHost {
-        directoryExists?(directoryName: string): boolean;
         getCurrentDirectory?(): string;
     }
     interface TextSpan {

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

@@ -1,6 +1,7 @@
 [
-    "======== Resolving type reference directive 'jquery', containing file '/a/b/consumer.ts', root directory not set. ========",
-    "Root directory cannot be determined, skipping primary search paths.",
+    "======== Resolving type reference directive 'jquery', containing file '/a/b/consumer.ts', root directory '/node_modules/@types'. ========",
+    "Resolving with primary search path '/node_modules/@types'.",
+    "Directory '/node_modules/@types' does not exist, skipping all lookups in it.",
     "Looking up in 'node_modules' folder, initial location '/a/b'.",
     "Directory '/a/b/node_modules' does not exist, skipping all lookups in it.",
     "Found 'package.json' at '/a/node_modules/jquery/package.json'.",