In preparation of sharing resolutions, watch the resolutions right away instead of defering external module reoslutions to watch all failed lookup locations

Author: sheetalkamat

src/compiler/resolutionCache.ts

@@ -115,12 +115,10 @@ export interface ResolutionCache extends Required<CompilerHostSupportingResoluti
     countResolutionsResolvedWithGlobalCache(): number;
     countResolutionsResolvedWithoutGlobalCache(): number;
 
-    watchFailedLookupLocationsOfExternalModuleResolutions<T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>(
-        name: string,
+    watchResolution<T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>(
         resolution: T,
         filePath: Path,
         getResolutionWithResolvedFileName: GetResolutionWithResolvedFileName<T, R>,
-        deferWatchingNonRelativeResolution: boolean,
     ): void;
 
     resolveModuleNameLiterals(
@@ -181,10 +179,13 @@ export interface ResolutionCache extends Required<CompilerHostSupportingResoluti
 export interface ResolutionWithFailedLookupLocations {
     failedLookupLocations?: string[];
     affectingLocations?: string[];
+    alternateResult?: string;
     isInvalidated?: boolean;
     // Files that have this resolution using
     files?: Set<Path>;
-    alternateResult?: string;
+    watchedFailed?: number;
+    watchedAffected?: number;
+    setAtRoot?: boolean;
     globalCacheResolution?: boolean;
 }
 
@@ -599,7 +600,6 @@ export function createResolutionCache(
     rootDirForResolution: string,
 ): ResolutionCache {
     let filesWithInvalidatedResolutions: Set<Path> | undefined;
-    const nonRelativeExternalModuleResolutions = new Set<ResolutionWithFailedLookupLocations>();
 
     const resolutionsWithFailedLookups = new Set<ResolutionWithFailedLookupLocations>();
     const resolutionsWithOnlyAffectingLocations = new Set<ResolutionWithFailedLookupLocations>();
@@ -677,7 +677,7 @@ export function createResolutionCache(
         dirPathToSymlinkPackageRefCount,
         countResolutionsResolvedWithGlobalCache: () => resolutionsResolvedWithGlobalCache,
         countResolutionsResolvedWithoutGlobalCache: () => resolutionsResolvedWithoutGlobalCache,
-        watchFailedLookupLocationsOfExternalModuleResolutions,
+        watchResolution,
         getModuleResolutionCache: () => moduleResolutionCache,
         // perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update
         // (between startCachingPerDirectoryResolution and finishCachingPerDirectoryResolution)
@@ -710,7 +710,6 @@ export function createResolutionCache(
         isSymlinkCache.clear();
         packageDirWatchers.clear();
         dirPathToSymlinkPackageRefCount.clear();
-        nonRelativeExternalModuleResolutions.clear();
         closeTypeRootsWatch();
         resolvedModuleNames.clear();
         resolvedTypeReferenceDirectives.clear();
@@ -776,9 +775,6 @@ export function createResolutionCache(
         moduleResolutionCache.clearAllExceptPackageJsonInfoCache();
         typeReferenceDirectiveResolutionCache.clearAllExceptPackageJsonInfoCache();
         libraryResolutionCache.clearAllExceptPackageJsonInfoCache();
-        // perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update
-        // (between startCachingPerDirectoryResolution and finishCachingPerDirectoryResolution)
-        watchFailedLookupLocationOfNonRelativeModuleResolutions();
         isSymlinkCache.clear();
     }
 
@@ -800,7 +796,6 @@ export function createResolutionCache(
         resolutionsWithGlobalCachePassAreInvalidated = false;
         resolutionsWithoutGlobalCachePassAreInvalidated = false;
         unresolvedResolutionsWithGlobalCachePassAreInvalidated = false;
-        watchFailedLookupLocationOfNonRelativeModuleResolutions();
         // Update file watches
         if (newProgram !== oldProgram) {
             cleanupLibResolutionWatching(newProgram);
@@ -875,7 +870,6 @@ export function createResolutionCache(
         perFileCache: Map<Path, ModeAwareCache<T>>;
         loader: ResolutionLoader<Entry, T, SourceFile>;
         getResolutionWithResolvedFileName: GetResolutionWithResolvedFileName<T, R>;
-        deferWatchingNonRelativeResolution: boolean;
         onNewResolution?: CallbackOnNewResolution<T>;
     }
     function resolveNamesWithLocalCache<Entry, SourceFile, T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>({
@@ -889,7 +883,6 @@ export function createResolutionCache(
         ambientEntries,
         loader,
         getResolutionWithResolvedFileName,
-        deferWatchingNonRelativeResolution,
         onNewResolution,
     }: ResolveNamesWithLocalCacheInput<Entry, SourceFile, T, R>): readonly T[] {
         const path = resolutionHost.toPath(containingFile);
@@ -926,7 +919,7 @@ export function createResolutionCache(
                 }
                 resolutionsInFile.set(name, mode, resolution);
                 if (resolution !== existingResolution) {
-                    watchFailedLookupLocationsOfExternalModuleResolutions(name, resolution, path, getResolutionWithResolvedFileName, deferWatchingNonRelativeResolution);
+                    watchResolution(resolution, path, getResolutionWithResolvedFileName);
                     if (existingResolution) {
                         stopWatchFailedLookupLocationOfResolution(existingResolution, path, getResolutionWithResolvedFileName);
                     }
@@ -1097,7 +1090,6 @@ export function createResolutionCache(
                 typeReferenceDirectiveResolutionCache,
             ),
             getResolutionWithResolvedFileName: getResolvedTypeReferenceDirectiveFromResolution,
-            deferWatchingNonRelativeResolution: false,
         });
     }
 
@@ -1128,7 +1120,6 @@ export function createResolutionCache(
                 moduleResolutionCache,
             ),
             getResolutionWithResolvedFileName: getResolvedModuleFromResolution,
-            deferWatchingNonRelativeResolution: true, // Defer non relative resolution watch because we could be using ambient modules
             onNewResolution,
         });
     }
@@ -1145,7 +1136,7 @@ export function createResolutionCache(
             const existingResolution = resolution;
             resolution = ts_resolveLibrary(libraryName, resolveFrom, options, host, libraryResolutionCache);
             const path = resolutionHost.toPath(resolveFrom);
-            watchFailedLookupLocationsOfExternalModuleResolutions(libraryName, resolution, path, getResolvedModuleFromResolution, /*deferWatchingNonRelativeResolution*/ false);
+            watchResolution(resolution, path, getResolvedModuleFromResolution);
             resolvedLibraries.set(libFileName, resolution);
             if (existingResolution) {
                 stopWatchFailedLookupLocationOfResolution(existingResolution, path, getResolvedModuleFromResolution);
@@ -1194,25 +1185,19 @@ export function createResolutionCache(
         return endsWith(dirPath, "/node_modules/@types");
     }
 
-    function watchFailedLookupLocationsOfExternalModuleResolutions<T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>(
-        name: string,
+    function watchResolution<T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>(
         resolution: T,
         filePath: Path,
         getResolutionWithResolvedFileName: GetResolutionWithResolvedFileName<T, R>,
-        deferWatchingNonRelativeResolution: boolean,
     ) {
         (resolution.files ??= new Set()).add(filePath);
+        watchFailedLookupLocationOfResolution(resolution);
+        watchAffectingLocationsOfResolution(resolution);
         if (resolution.files.size !== 1) return;
 
         if (resolution.globalCacheResolution) resolutionsResolvedWithGlobalCache++;
         else if (resolution.globalCacheResolution === false) resolutionsResolvedWithoutGlobalCache++;
 
-        if (!deferWatchingNonRelativeResolution || isExternalModuleNameRelative(name)) {
-            watchFailedLookupLocationOfResolution(resolution);
-        }
-        else {
-            nonRelativeExternalModuleResolutions.add(resolution);
-        }
         const resolved = getResolutionWithResolvedFileName(resolution);
         if (resolved && resolved.resolvedFileName) {
             const key = resolutionHost.toPath(resolved.resolvedFileName);
@@ -1249,33 +1234,38 @@ export function createResolutionCache(
     function watchFailedLookupLocationOfResolution(resolution: ResolutionWithFailedLookupLocations) {
         Debug.assert(!!resolution.files?.size);
 
-        const { failedLookupLocations, affectingLocations, alternateResult } = resolution;
-        if (!failedLookupLocations?.length && !affectingLocations?.length && !alternateResult) return;
-        if (failedLookupLocations?.length || alternateResult) resolutionsWithFailedLookups.add(resolution);
+        const { failedLookupLocations, alternateResult, watchedFailed } = resolution;
+        // There have to be failed lookup locations if there is alternateResult so storing failedLookupLocation length is good enough,
+        // alternateResult doesnt change later only failed lookup locations get added on
+        if (watchedFailed === failedLookupLocations?.length) return;
+        if (!watchedFailed) {
+            resolutionsWithFailedLookups.add(resolution);
+            if (resolution.watchedAffected) resolutionsWithOnlyAffectingLocations.delete(resolution);
+        }
 
-        let setAtRoot = false;
-        if (failedLookupLocations) {
-            for (const failedLookupLocation of failedLookupLocations) {
-                setAtRoot = watchFailedLookupLocation(failedLookupLocation, setAtRoot);
-            }
+        let setAtRoot = !!resolution.setAtRoot;
+        for (let i = watchedFailed || 0; i < failedLookupLocations!.length; i++) {
+            setAtRoot = watchFailedLookupLocation(failedLookupLocations![i], setAtRoot);
         }
-        if (alternateResult) setAtRoot = watchFailedLookupLocation(alternateResult, setAtRoot);
-        if (setAtRoot) {
+        if (!watchedFailed && alternateResult) setAtRoot = watchFailedLookupLocation(alternateResult, setAtRoot);
+        if (!resolution.setAtRoot && setAtRoot) {
             // This is always non recursive
             setDirectoryWatcher(rootDir, rootPath, /*packageDir*/ undefined, /*packageDirPath*/ undefined, /*nonRecursive*/ true);
         }
-        watchAffectingLocationsOfResolution(resolution, !failedLookupLocations?.length && !alternateResult);
+        resolution.watchedFailed = failedLookupLocations?.length;
+        resolution.setAtRoot = setAtRoot;
     }
 
-    function watchAffectingLocationsOfResolution(resolution: ResolutionWithFailedLookupLocations, addToResolutionsWithOnlyAffectingLocations: boolean) {
+    function watchAffectingLocationsOfResolution(resolution: ResolutionWithFailedLookupLocations) {
         Debug.assert(!!resolution.files?.size);
-        const { affectingLocations } = resolution;
-        if (!affectingLocations?.length) return;
-        if (addToResolutionsWithOnlyAffectingLocations) resolutionsWithOnlyAffectingLocations.add(resolution);
+        const { affectingLocations, watchedAffected } = resolution;
+        if (affectingLocations?.length === watchedAffected) return;
+        if (!watchedAffected && !resolution.watchedFailed) resolutionsWithOnlyAffectingLocations.add(resolution);
         // Watch package json
-        for (const affectingLocation of affectingLocations) {
-            createFileWatcherOfAffectingLocation(affectingLocation, /*forResolution*/ true);
+        for (let i = watchedAffected || 0; i < affectingLocations!.length; i++) {
+            createFileWatcherOfAffectingLocation(affectingLocations![i], /*forResolution*/ true);
         }
+        resolution.watchedAffected = affectingLocations?.length;
     }
 
     function createFileWatcherOfAffectingLocation(affectingLocation: string, forResolution: boolean) {
@@ -1343,11 +1333,6 @@ export function createResolutionCache(
         packageJsonMap?.delete(resolutionHost.toPath(path));
     }
 
-    function watchFailedLookupLocationOfNonRelativeModuleResolutions() {
-        nonRelativeExternalModuleResolutions.forEach(watchFailedLookupLocationOfResolution);
-        nonRelativeExternalModuleResolutions.clear();
-    }
-
     function createDirectoryWatcherForPackageDir(
         dir: string,
         dirPath: Path,

src/harness/incrementalUtils.ts

@@ -222,7 +222,6 @@ export function verifyResolutionCache(
             path,
             resolutions,
             getResolvedModuleFileName,
-            /*deferWatchingNonRelativeResolution*/ true,
             expected.resolvedModuleNames,
             (name, mode) => actualProgram.getResolvedModule(actualProgram.getSourceFileByPath(path)!, name, mode),
         )
@@ -233,7 +232,6 @@ export function verifyResolutionCache(
             path,
             resolutions,
             getResolvedTypeRefFileName,
-            /*deferWatchingNonRelativeResolution*/ false,
             expected.resolvedTypeReferenceDirectives,
             (name, mode) =>
                 path !== inferredTypesPath ?
@@ -251,7 +249,6 @@ export function verifyResolutionCache(
             getResolvedModuleFileName(resolved),
             ts.getLibraryNameFromLibFileName(libFileName),
             /*mode*/ undefined,
-            /*deferWatchingNonRelativeResolution*/ false,
         );
         expected.resolvedLibraries.set(libFileName, expectedResolution);
     });
@@ -304,7 +301,20 @@ export function verifyResolutionCache(
             !resolution.isInvalidated,
             `${projectName}:: Resolution should not be invalidated`,
         );
-        verifySet(resolutionToExpected.get(resolution)!.files, resolution.files, `${projectName}:: Resolution files`);
+        const expected = resolutionToExpected.get(resolution)!;
+        verifySet(expected.files, resolution.files, `${projectName}:: Resolution files`);
+        ts.Debug.assert(
+            expected.watchedFailed === resolution.watchedFailed,
+            `${projectName}:: Expected watchedFailed of Resolution ${expected.watchedFailed} but got ${resolution.watchedFailed}`,
+        );
+        ts.Debug.assert(
+            expected.watchedAffected === resolution.watchedAffected,
+            `${projectName}:: Expected watchedAffected of Resolution ${expected.watchedAffected} but got ${resolution.watchedAffected}`,
+        );
+        ts.Debug.assert(
+            expected.setAtRoot === resolution.setAtRoot,
+            `${projectName}:: Expected setAtRoot of Resolution ${expected.setAtRoot} but got ${resolution.setAtRoot}`,
+        );
     });
     verifyMapOfResolutionSet(expected.resolvedFileToResolution, actual.resolvedFileToResolution, `resolvedFileToResolution`);
     verifyResolutionSet(expected.resolutionsWithFailedLookups, actual.resolutionsWithFailedLookups, `resolutionsWithFailedLookups`);
@@ -366,7 +376,6 @@ export function verifyResolutionCache(
         fileName: ts.Path,
         cache: ts.ModeAwareCache<T> | undefined,
         getResolvedFileName: (resolution: T) => string | undefined,
-        deferWatchingNonRelativeResolution: boolean,
         storeExpected: Map<ts.Path, ts.ModeAwareCache<ts.ResolutionWithFailedLookupLocations>>,
         getProgramResolutions: (name: string, mode: ts.ResolutionMode) => T | undefined,
     ) {
@@ -377,7 +386,7 @@ export function verifyResolutionCache(
         let expectedCache: ts.ModeAwareCache<ts.ResolutionWithFailedLookupLocations> | undefined;
         cache?.forEach((resolved, name, mode) => {
             const resolvedFileName = getResolvedFileName(resolved);
-            const expected = collectResolution(cacheType, fileName, resolved, resolvedFileName, name, mode, deferWatchingNonRelativeResolution);
+            const expected = collectResolution(cacheType, fileName, resolved, resolvedFileName, name, mode);
             if (!expectedCache) storeExpected.set(fileName, expectedCache = ts.createModeAwareCache());
             expectedCache.set(name, mode, expected);
             // Resolution in cache should be same as that is in program
@@ -395,7 +404,6 @@ export function verifyResolutionCache(
         resolvedFileName: string | undefined,
         name: string,
         mode: ts.ResolutionMode,
-        deferWatchingNonRelativeResolution: boolean,
     ): ExpectedResolution {
         const existing = resolutionToRefs.get(resolved);
         let expectedResolution: ExpectedResolution;
@@ -416,7 +424,7 @@ export function verifyResolutionCache(
             expectedToResolution.set(expectedResolution, resolved);
             resolutionToExpected.set(resolved, expectedResolution);
         }
-        expected.watchFailedLookupLocationsOfExternalModuleResolutions(name, expectedResolution, fileName, () => ({ resolvedFileName }), deferWatchingNonRelativeResolution);
+        expected.watchResolution(expectedResolution, fileName, () => ({ resolvedFileName }));
         return expectedResolution;
     }
 

tests/baselines/reference/tscWatch/extends/configDir-template.js

@@ -127,8 +127,6 @@ File '/home/src/projects/myproject/root2/other/sometype2/package.json' does not
 File '/home/src/projects/myproject/root2/other/sometype2/index.d.ts' exists - use it as a name resolution result.
 Resolving real path for '/home/src/projects/myproject/root2/other/sometype2/index.d.ts', result '/home/src/projects/myproject/root2/other/sometype2/index.d.ts'.
 ======== Module name 'other/sometype2' was successfully resolved to '/home/src/projects/myproject/root2/other/sometype2/index.d.ts'. ========
-FileWatcher:: Added:: WatchInfo: /home/src/projects/myproject/root2/other/sometype2/index.d.ts 250 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Source file
-FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 250 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Source file
 DirectoryWatcher:: Added:: WatchInfo: /home/src/projects/myproject/other 1 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Failed Lookup Locations
 Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /home/src/projects/myproject/other 1 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Failed Lookup Locations
 DirectoryWatcher:: Added:: WatchInfo: /home/src/projects/myproject/src 1 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Failed Lookup Locations
@@ -141,6 +139,8 @@ DirectoryWatcher:: Added:: WatchInfo: /home/src/projects/configs 1 {"excludeFile
 Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /home/src/projects/configs 1 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Failed Lookup Locations
 DirectoryWatcher:: Added:: WatchInfo: /home/src/projects/myproject/root2 1 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Failed Lookup Locations
 Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /home/src/projects/myproject/root2 1 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Failed Lookup Locations
+FileWatcher:: Added:: WatchInfo: /home/src/projects/myproject/root2/other/sometype2/index.d.ts 250 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Source file
+FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 250 {"excludeFiles":["/home/src/projects/myproject/main.ts"]} Source file
 ../../../../a/lib/lib.d.ts
   Default library for target 'es5'
 types/sometype.ts