When watching failed lookups, watch packageDir if its a symlink otherwise the path we use to watch (#58139)

Author: sheetalkamat

src/compiler/resolutionCache.ts

@@ -18,6 +18,7 @@ import {
     getDirectoryPath,
     getFallbackOptions,
     getNormalizedAbsolutePath,
+    getRelativePathFromDirectory,
     getRelativePathToDirectoryOrUrl,
     getRootLength,
     getStringComparer,
@@ -607,15 +608,17 @@ function createDirectoryWatcherSupportingRecursive({
         watcher: FileWatcher;
         childWatches: ChildWatches;
         refCount: number;
+        targetWatcher: ChildDirectoryWatcher | undefined;
+        links: Set<string> | undefined;
     }
 
-    const cache = new Map<string, HostDirectoryWatcher>();
+    const cache = new Map<Path, HostDirectoryWatcher>();
     const callbackCache = createMultiMap<Path, { dirName: string; callback: DirectoryWatcherCallback; }>();
     const cacheToUpdateChildWatches = new Map<Path, { dirName: string; options: WatchOptions | undefined; fileNames: string[]; }>();
     let timerToUpdateChildWatches: any;
 
     const filePathComparer = getStringComparer(!useCaseSensitiveFileNames);
-    const toCanonicalFilePath = createGetCanonicalFileName(useCaseSensitiveFileNames);
+    const toCanonicalFilePath = createGetCanonicalFileName(useCaseSensitiveFileNames) as (fileName: string) => Path;
 
     return (dirName, callback, recursive, options) =>
         recursive ?
@@ -625,8 +628,13 @@ function createDirectoryWatcherSupportingRecursive({
     /**
      * Create the directory watcher for the dirPath.
      */
-    function createDirectoryWatcher(dirName: string, options: WatchOptions | undefined, callback?: DirectoryWatcherCallback): ChildDirectoryWatcher {
-        const dirPath = toCanonicalFilePath(dirName) as Path;
+    function createDirectoryWatcher(
+        dirName: string,
+        options: WatchOptions | undefined,
+        callback?: DirectoryWatcherCallback,
+        link?: string,
+    ): ChildDirectoryWatcher {
+        const dirPath = toCanonicalFilePath(dirName);
         let directoryWatcher = cache.get(dirPath);
         if (directoryWatcher) {
             directoryWatcher.refCount++;
@@ -640,7 +648,7 @@ function createDirectoryWatcherSupportingRecursive({
 
                         if (options?.synchronousWatchDirectory) {
                             // Call the actual callback
-                            invokeCallbacks(dirPath, fileName);
+                            if (!cache.get(dirPath)?.targetWatcher) invokeCallbacks(dirName, dirPath, fileName);
 
                             // Iterate through existing children and update the watches if needed
                             updateChildWatches(dirName, dirPath, options);
@@ -654,11 +662,15 @@ function createDirectoryWatcherSupportingRecursive({
                 ),
                 refCount: 1,
                 childWatches: emptyArray,
+                targetWatcher: undefined,
+                links: undefined,
             };
             cache.set(dirPath, directoryWatcher);
             updateChildWatches(dirName, dirPath, options);
         }
 
+        if (link) (directoryWatcher.links ??= new Set()).add(link);
+
         const callbackToAdd = callback && { dirName, callback };
         if (callbackToAdd) {
             callbackCache.add(dirPath, callbackToAdd);
@@ -669,21 +681,24 @@ function createDirectoryWatcherSupportingRecursive({
             close: () => {
                 const directoryWatcher = Debug.checkDefined(cache.get(dirPath));
                 if (callbackToAdd) callbackCache.remove(dirPath, callbackToAdd);
+                if (link) directoryWatcher.links?.delete(link);
                 directoryWatcher.refCount--;
 
                 if (directoryWatcher.refCount) return;
 
                 cache.delete(dirPath);
+                directoryWatcher.links = undefined;
                 closeFileWatcherOf(directoryWatcher);
+                closeTargetWatcher(directoryWatcher);
                 directoryWatcher.childWatches.forEach(closeFileWatcher);
             },
         };
     }
 
     type InvokeMap = Map<Path, string[] | true>;
-    function invokeCallbacks(dirPath: Path, fileName: string): void;
-    function invokeCallbacks(dirPath: Path, invokeMap: InvokeMap, fileNames: string[] | undefined): void;
-    function invokeCallbacks(dirPath: Path, fileNameOrInvokeMap: string | InvokeMap, fileNames?: string[]) {
+    function invokeCallbacks(dirName: string, dirPath: Path, fileName: string): void;
+    function invokeCallbacks(dirName: string, dirPath: Path, invokeMap: InvokeMap, fileNames: string[] | undefined): void;
+    function invokeCallbacks(dirName: string, dirPath: Path, fileNameOrInvokeMap: string | InvokeMap, fileNames?: string[]) {
         let fileName: string | undefined;
         let invokeMap: InvokeMap | undefined;
         if (isString(fileNameOrInvokeMap)) {
@@ -715,6 +730,15 @@ function createDirectoryWatcherSupportingRecursive({
                 }
             }
         });
+        cache.get(dirPath)?.links?.forEach(link => {
+            const toPathInLink = (fileName: string) => combinePaths(link, getRelativePathFromDirectory(dirName, fileName, toCanonicalFilePath));
+            if (invokeMap) {
+                invokeCallbacks(link, toCanonicalFilePath(link), invokeMap, fileNames?.map(toPathInLink));
+            }
+            else {
+                invokeCallbacks(link, toCanonicalFilePath(link), toPathInLink(fileName!));
+            }
+        });
     }
 
     function nonSyncUpdateChildWatches(dirName: string, dirPath: Path, fileName: string, options: WatchOptions | undefined) {
@@ -727,7 +751,8 @@ function createDirectoryWatcherSupportingRecursive({
         }
 
         // Call the actual callbacks and remove child watches
-        invokeCallbacks(dirPath, fileName);
+        invokeCallbacks(dirName, dirPath, fileName);
+        closeTargetWatcher(parentWatcher);
         removeChildWatches(parentWatcher);
     }
 
@@ -760,7 +785,7 @@ function createDirectoryWatcherSupportingRecursive({
             // Because the child refresh is fresh, we would need to invalidate whole root directory being watched
             // to ensure that all the changes are reflected at this time
             const hasChanges = updateChildWatches(dirName, dirPath, options);
-            invokeCallbacks(dirPath, invokeMap, hasChanges ? undefined : fileNames);
+            if (!cache.get(dirPath)?.targetWatcher) invokeCallbacks(dirName, dirPath, invokeMap, hasChanges ? undefined : fileNames);
         }
 
         sysLog(`sysLog:: invokingWatchers:: Elapsed:: ${timestamp() - start}ms:: ${cacheToUpdateChildWatches.size}`);
@@ -792,24 +817,46 @@ function createDirectoryWatcherSupportingRecursive({
         }
     }
 
+    function closeTargetWatcher(watcher: HostDirectoryWatcher | undefined) {
+        if (watcher?.targetWatcher) {
+            watcher.targetWatcher.close();
+            watcher.targetWatcher = undefined;
+        }
+    }
+
     function updateChildWatches(parentDir: string, parentDirPath: Path, options: WatchOptions | undefined) {
         // Iterate through existing children and update the watches if needed
         const parentWatcher = cache.get(parentDirPath);
         if (!parentWatcher) return false;
+        const target = normalizePath(realpath(parentDir));
+        let hasChanges;
         let newChildWatches: ChildDirectoryWatcher[] | undefined;
-        const hasChanges = enumerateInsertsAndDeletes<string, ChildDirectoryWatcher>(
-            fileSystemEntryExists(parentDir, FileSystemEntryKind.Directory) ? mapDefined(getAccessibleSortedChildDirectories(parentDir), child => {
-                const childFullName = getNormalizedAbsolutePath(child, parentDir);
-                // Filter our the symbolic link directories since those arent included in recursive watch
-                // which is same behaviour when recursive: true is passed to fs.watch
-                return !isIgnoredPath(childFullName, options) && filePathComparer(childFullName, normalizePath(realpath(childFullName))) === Comparison.EqualTo ? childFullName : undefined;
-            }) : emptyArray,
-            parentWatcher.childWatches,
-            (child, childWatcher) => filePathComparer(child, childWatcher.dirName),
-            createAndAddChildDirectoryWatcher,
-            closeFileWatcher,
-            addChildDirectoryWatcher,
-        );
+        if (filePathComparer(target, parentDir) === Comparison.EqualTo) {
+            // if (parentWatcher.target) closeFileWatcher
+            hasChanges = enumerateInsertsAndDeletes<string, ChildDirectoryWatcher>(
+                fileSystemEntryExists(parentDir, FileSystemEntryKind.Directory) ? mapDefined(getAccessibleSortedChildDirectories(parentDir), child => {
+                    const childFullName = getNormalizedAbsolutePath(child, parentDir);
+                    // Filter our the symbolic link directories since those arent included in recursive watch
+                    // which is same behaviour when recursive: true is passed to fs.watch
+                    return !isIgnoredPath(childFullName, options) && filePathComparer(childFullName, normalizePath(realpath(childFullName))) === Comparison.EqualTo ? childFullName : undefined;
+                }) : emptyArray,
+                parentWatcher.childWatches,
+                (child, childWatcher) => filePathComparer(child, childWatcher.dirName),
+                createAndAddChildDirectoryWatcher,
+                closeFileWatcher,
+                addChildDirectoryWatcher,
+            );
+        }
+        else if (parentWatcher.targetWatcher && filePathComparer(target, parentWatcher.targetWatcher.dirName) === Comparison.EqualTo) {
+            hasChanges = false;
+            Debug.assert(parentWatcher.childWatches === emptyArray);
+        }
+        else {
+            closeTargetWatcher(parentWatcher);
+            parentWatcher.targetWatcher = createDirectoryWatcher(target, options, /*callback*/ undefined, parentDir);
+            parentWatcher.childWatches.forEach(closeFileWatcher);
+            hasChanges = true;
+        }
         parentWatcher.childWatches = newChildWatches || emptyArray;
         return hasChanges;
 

src/compiler/sys.ts

@@ -271,6 +271,8 @@ export function verifyResolutionCache(
     verifyResolutionSet(expected.resolutionsWithOnlyAffectingLocations, actual.resolutionsWithOnlyAffectingLocations, `resolutionsWithOnlyAffectingLocations`);
     verifyDirectoryWatchesOfFailedLookups(expected.directoryWatchesOfFailedLookups, actual.directoryWatchesOfFailedLookups);
     verifyFileWatchesOfAffectingLocations(expected.fileWatchesOfAffectingLocations, actual.fileWatchesOfAffectingLocations);
+    verifyPackageDirWatchers(expected.packageDirWatchers, actual.packageDirWatchers);
+    verifyDirPathToSymlinkPackageRefCount(expected.dirPathToSymlinkPackageRefCount, actual.dirPathToSymlinkPackageRefCount);
 
     // Stop watching resolutions to verify everything gets closed.
     expected.startCachingPerDirectoryResolution();
@@ -368,11 +370,17 @@ export function verifyResolutionCache(
     }
 
     function verifyDirectoryWatchesOfFailedLookups(expected: Map<string, ts.DirectoryWatchesOfFailedLookup>, actual: Map<string, ts.DirectoryWatchesOfFailedLookup>) {
-        verifyMap(expected, actual, (expected, actual, caption) => {
-            ts.Debug.assert(expected?.refCount === actual?.refCount, `${projectName}:: ${caption}:: refCount`);
-            ts.Debug.assert(!!expected?.refCount, `${projectName}:: ${caption}:: expected refCount to be non zero`);
-            ts.Debug.assert(expected?.nonRecursive === actual?.nonRecursive, `${projectName}:: ${caption}:: nonRecursive`);
-        }, "directoryWatchesOfFailedLookups");
+        verifyMap(expected, actual, verifyDirectoryWatchesOfFailedLookup, "directoryWatchesOfFailedLookups");
+    }
+
+    function verifyDirectoryWatchesOfFailedLookup(
+        expected: ts.DirectoryWatchesOfFailedLookup | undefined,
+        actual: ts.DirectoryWatchesOfFailedLookup | undefined,
+        caption: string,
+    ) {
+        ts.Debug.assert(expected?.refCount === actual?.refCount, `${projectName}:: ${caption}:: refCount`);
+        ts.Debug.assert(!!expected?.refCount, `${projectName}:: ${caption}:: expected refCount to be non zero`);
+        ts.Debug.assert(expected?.nonRecursive === actual?.nonRecursive, `${projectName}:: ${caption}:: nonRecursive`);
     }
 
     function verifyFileWatchesOfAffectingLocations(
@@ -391,6 +399,40 @@ export function verifyResolutionCache(
         ts.Debug.assert(expected?.files === actual?.files, `${projectName}:: ${caption}:: files`);
         verifySet(expected?.symlinks, actual?.symlinks, `${projectName}:: ${caption}:: symlinks`);
     }
+
+    function verifyPackageDirWatchers(
+        expected: Map<ts.Path, ts.PackageDirWatcher>,
+        actual: Map<ts.Path, ts.PackageDirWatcher>,
+    ) {
+        verifyMap(expected, actual, verifyPackageDirWatcher, "packageDirWatchers");
+    }
+
+    function verifyPackageDirWatcher(
+        expected: ts.PackageDirWatcher | undefined,
+        actual: ts.PackageDirWatcher | undefined,
+        caption: string,
+    ) {
+        ts.Debug.assert(expected?.isSymlink === actual?.isSymlink, `${projectName}:: ${caption}:: isSymlink`);
+        verifyMap(expected?.dirPathToWatcher, actual?.dirPathToWatcher, verfiyDirPathToWatcherOfPackageDirWatcher, `${projectName}:: ${caption}:: dirPathToWatcher`);
+    }
+
+    function verfiyDirPathToWatcherOfPackageDirWatcher(
+        expected: ts.DirPathToWatcherOfPackageDirWatcher | undefined,
+        actual: ts.DirPathToWatcherOfPackageDirWatcher | undefined,
+        caption: string,
+    ) {
+        ts.Debug.assert(expected?.refCount === actual?.refCount, `${projectName}:: ${caption}:: refCount`);
+        verifyDirectoryWatchesOfFailedLookup(expected?.watcher, actual?.watcher, `${projectName}:: ${caption}:: directoryWatchesOfFailedLookup`);
+    }
+
+    function verifyDirPathToSymlinkPackageRefCount(
+        expected: Map<ts.Path, number>,
+        actual: Map<ts.Path, number>,
+    ) {
+        verifyMap(expected, actual, (expected, actual, caption) => {
+            ts.Debug.assert(expected === actual, `${projectName}:: ${caption}`);
+        }, "dirPathToSymlinkPackageRefCount");
+    }
 }
 
 function verifyMap<Key extends string, Expected, Actual>(

src/harness/incrementalUtils.ts

@@ -134,6 +134,7 @@ import "./unittests/tscWatch/projectsWithReferences";
 import "./unittests/tscWatch/resolutionCache";
 import "./unittests/tscWatch/resolveJsonModuleWithIncremental";
 import "./unittests/tscWatch/sourceOfProjectReferenceRedirect";
+import "./unittests/tscWatch/symlinks";
 import "./unittests/tscWatch/watchApi";
 import "./unittests/tscWatch/watchEnvironment";
 import "./unittests/tsserver/applyChangesToOpenFiles";

src/testRunner/tests.ts

@@ -55,34 +55,44 @@ describe("unittests:: canWatch::", () => {
         scenario: string,
         forPath: "node_modules" | "node_modules/@types" | "",
     ) {
-        baselineCanWatch(
-            scenario,
-            () => `Determines whether to watch given failed lookup location (file that didnt exist) when resolving module.\r\nIt also determines the directory to watch and whether to watch it recursively or not.`,
-            (paths, longestPathLength, baseline) => {
-                const recursive = "Recursive";
-                const maxLength = longestPathLength + ts.combinePaths(forPath, "dir/subdir/somefile.d.ts").length;
-                const maxLengths = [maxLength, maxLength, recursive.length] as const;
-                baselineCanWatchForRoot(paths, baseline, (rootPathCompoments, root) => {
-                    pushHeader(baseline, ["Location", "getDirectoryToWatchFailedLookupLocation", recursive], maxLengths);
-                    paths.forEach(path => {
-                        baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "somefile.d.ts"), root, rootPathCompoments, maxLengths);
-                        baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "dir/somefile.d.ts"), root, rootPathCompoments, maxLengths);
-                        baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "dir/subdir/somefile.d.ts"), root, rootPathCompoments, maxLengths);
+        ["file", "dir", "subDir"].forEach(type => {
+            baselineCanWatch(
+                `${scenario}In${type}`,
+                () => `Determines whether to watch given failed lookup location (file that didnt exist) when resolving module.\r\nIt also determines the directory to watch and whether to watch it recursively or not.`,
+                (paths, longestPathLength, baseline) => {
+                    const recursive = "Recursive";
+                    const maxLength = longestPathLength + ts.combinePaths(forPath, "dir/subdir/somefile.d.ts").length;
+                    const maxLengths = [maxLength, maxLength, recursive.length, maxLength] as const;
+                    baselineCanWatchForRoot(paths, baseline, (rootPathCompoments, root) => {
+                        pushHeader(baseline, ["Location", "getDirectoryToWatchFailedLookupLocation", recursive, "Location if not symlink"], maxLengths);
+                        paths.forEach(path => {
+                            let subPath;
+                            switch (type) {
+                                case "file":
+                                    subPath = "somefile.d.ts";
+                                    break;
+                                case "dir":
+                                    subPath = "dir/somefile.d.ts";
+                                    break;
+                                case "subDir":
+                                    subPath = "dir/subdir/somefile.d.ts";
+                                    break;
+                            }
+                            const testPath = combinePaths(path, forPath, subPath);
+                            const result = ts.getDirectoryToWatchFailedLookupLocation(
+                                testPath,
+                                testPath,
+                                root,
+                                root,
+                                rootPathCompoments,
+                                ts.returnUndefined,
+                            );
+                            pushRow(baseline, [testPath, result ? result.packageDir ?? result.dir : "", result ? `${!result.nonRecursive}` : "", result?.packageDir ? result.dir : ""], maxLengths);
+                        });
                     });
-                });
-                function baselineGetDirectoryToWatchFailedLookupLocation(path: ts.Path, root: ts.Path, rootPathCompoments: Readonly<ts.PathPathComponents>, maxLengths: readonly number[]) {
-                    const result = ts.getDirectoryToWatchFailedLookupLocation(
-                        path,
-                        path,
-                        root,
-                        root,
-                        rootPathCompoments,
-                        ts.returnUndefined,
-                    );
-                    pushRow(baseline, [path, result ? result.dir : "", result ? `${!result.nonRecursive}` : ""], maxLengths);
-                }
-            },
-        );
+                },
+            );
+        });
     }
 
     baselineCanWatch(