Fix the incorrect resolution watching ref management for symlinks

Author: sheetalkamat

src/compiler/resolutionCache.ts

@@ -54,6 +54,7 @@ import {
     normalizePath,
     PackageId,
     packageIdToString,
+    PackageJsonInfo,
     parseNodeModuleFromPath,
     Path,
     PathPathComponents,
@@ -213,7 +214,7 @@ export interface FileWatcherOfAffectingLocation {
     watcher: FileWatcher;
     resolutions: number;
     files: number;
-    paths: Set<string>;
+    symlinks: Set<string> | undefined;
 }
 
 /** @internal */
@@ -723,12 +724,11 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
             }
         });
         fileWatchesOfAffectingLocations.forEach((watcher, path) => {
-            if (watcher.files === 0 && watcher.resolutions === 0) {
+            if (watcher.files === 0 && watcher.resolutions === 0 && !watcher.symlinks?.size) {
                 fileWatchesOfAffectingLocations.delete(path);
                 watcher.watcher.close();
             }
         });
-
         hasChangedAutomaticTypeDirectiveNames = false;
     }
 
@@ -1057,51 +1057,63 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
             return;
         }
         let locationToWatch = affectingLocation;
+        let isSymlink = false;
+        let symlinkWatcher: FileWatcherOfAffectingLocation | undefined;
         if (resolutionHost.realpath) {
             locationToWatch = resolutionHost.realpath(affectingLocation);
             if (affectingLocation !== locationToWatch) {
-                const fileWatcher = fileWatchesOfAffectingLocations.get(locationToWatch);
-                if (fileWatcher) {
-                    if (forResolution) fileWatcher.resolutions++;
-                    else fileWatcher.files++;
-                    fileWatcher.paths.add(affectingLocation);
-                    fileWatchesOfAffectingLocations.set(affectingLocation, fileWatcher);
-                    return;
-                }
+                isSymlink = true;
+                symlinkWatcher = fileWatchesOfAffectingLocations.get(locationToWatch);
             }
         }
-        const paths = new Set<string>();
-        paths.add(locationToWatch);
-        let actualWatcher = canWatchAffectingLocation(resolutionHost.toPath(locationToWatch)) ?
-            resolutionHost.watchAffectingFileLocation(locationToWatch, (fileName, eventKind) => {
-                cachedDirectoryStructureHost?.addOrDeleteFile(fileName, resolutionHost.toPath(locationToWatch), eventKind);
-                const packageJsonMap = moduleResolutionCache.getPackageJsonInfoCache().getInternalMap();
-                paths.forEach(path => {
-                    if (watcher.resolutions) (affectingPathChecks ??= new Set()).add(path);
-                    if (watcher.files) (affectingPathChecksForFile ??= new Set()).add(path);
-                    packageJsonMap?.delete(resolutionHost.toPath(path));
-                });
-                resolutionHost.scheduleInvalidateResolutionsOfFailedLookupLocations();
-            }) : noopFileWatcher;
-        const watcher: FileWatcherOfAffectingLocation = {
-            watcher: actualWatcher !== noopFileWatcher ? {
-                close: () => {
-                    actualWatcher.close();
-                    // Ensure when watching symlinked package.json, we can close the actual file watcher only once
-                    actualWatcher = noopFileWatcher;
-                }
-            } : actualWatcher,
-            resolutions: forResolution ? 1 : 0,
-            files: forResolution ? 0 : 1,
-            paths,
-        };
-        fileWatchesOfAffectingLocations.set(locationToWatch, watcher);
-        if (affectingLocation !== locationToWatch) {
+
+        const resolutions = forResolution ? 1 : 0;
+        const files = forResolution ? 0 : 1;
+        if (!isSymlink || !symlinkWatcher) {
+            const watcher: FileWatcherOfAffectingLocation = {
+                watcher: canWatchAffectingLocation(resolutionHost.toPath(locationToWatch)) ?
+                    resolutionHost.watchAffectingFileLocation(locationToWatch, (fileName, eventKind) => {
+                        cachedDirectoryStructureHost?.addOrDeleteFile(fileName, resolutionHost.toPath(locationToWatch), eventKind);
+                        invalidateAffectingFileWatcher(locationToWatch, moduleResolutionCache.getPackageJsonInfoCache().getInternalMap());
+                        resolutionHost.scheduleInvalidateResolutionsOfFailedLookupLocations();
+                    }) : noopFileWatcher,
+                resolutions: isSymlink ? 0 : resolutions,
+                files: isSymlink ? 0 : files,
+                symlinks: undefined,
+            };
+            fileWatchesOfAffectingLocations.set(locationToWatch, watcher);
+            if (isSymlink) symlinkWatcher = watcher;
+        }
+        if (isSymlink) {
+            Debug.assert(!!symlinkWatcher);
+            const watcher: FileWatcherOfAffectingLocation = {
+                watcher: {
+                    close: () => {
+                        const symlinkWatcher = fileWatchesOfAffectingLocations.get(locationToWatch);
+                        // Close symlink watcher if no ref
+                        if (symlinkWatcher?.symlinks?.delete(affectingLocation) && !symlinkWatcher.symlinks.size && !symlinkWatcher.resolutions && !symlinkWatcher.files) {
+                            fileWatchesOfAffectingLocations.delete(locationToWatch);
+                            symlinkWatcher.watcher.close();
+                        }
+                    },
+                },
+                resolutions,
+                files,
+                symlinks: undefined,
+            };
             fileWatchesOfAffectingLocations.set(affectingLocation, watcher);
-            paths.add(affectingLocation);
+            (symlinkWatcher.symlinks ??= new Set()).add(affectingLocation);
         }
     }
 
+    function invalidateAffectingFileWatcher(path: string, packageJsonMap: Map<Path, PackageJsonInfo | boolean> | undefined) {
+        const watcher = fileWatchesOfAffectingLocations.get(path);
+        if (watcher?.resolutions) (affectingPathChecks ??= new Set()).add(path);
+        if (watcher?.files) (affectingPathChecksForFile ??= new Set()).add(path);
+        watcher?.symlinks?.forEach(path => invalidateAffectingFileWatcher(path, packageJsonMap));
+        packageJsonMap?.delete(resolutionHost.toPath(path));
+    }
+
     function watchFailedLookupLocationOfNonRelativeModuleResolutions(resolutions: ResolutionWithFailedLookupLocations[], name: string) {
         const program = resolutionHost.getCurrentProgram();
         if (!program || !program.getTypeChecker().tryFindAmbientModuleWithoutAugmentations(name)) {

src/harness/incrementalUtils.ts

@@ -385,7 +385,7 @@ function verifyResolutionCache(
     ) {
         ts.Debug.assert(expected?.resolutions === actual?.resolutions, `${projectName}:: ${caption}:: resolutions`);
         ts.Debug.assert(expected?.files === actual?.files, `${projectName}:: ${caption}:: files`);
-        verifySet(expected?.paths, actual?.paths, `${projectName}:: ${caption}:: paths`);
+        verifySet(expected?.symlinks, actual?.symlinks, `${projectName}:: ${caption}:: symlinks`);
     }
 }