When directory watcher is invoked with any file from node_modules package, invalidate for file paths in that package (microsoft#43974)

Co-authored-by: Daniel Rosenwasser <[email protected]>

Author: sheetalkamat

src/compiler/moduleNameResolver.ts

@@ -1150,7 +1150,7 @@ namespace ts {
             }
             const resolvedFromFile = loadModuleFromFile(extensions, candidate, onlyRecordFailures, state);
             if (resolvedFromFile) {
-                const packageDirectory = considerPackageJson ? parseNodeModuleFromPath(resolvedFromFile) : undefined;
+                const packageDirectory = considerPackageJson ? parseNodeModuleFromPath(resolvedFromFile.path) : undefined;
                 const packageInfo = packageDirectory ? getPackageJsonInfo(packageDirectory, /*onlyRecordFailures*/ false, state) : undefined;
                 return withPackageId(packageInfo, resolvedFromFile);
             }
@@ -1184,8 +1184,9 @@ namespace ts {
      *   For `/node_modules/@types/foo/bar/index.d.ts` this is packageDirectory: "@types/foo"
      *   For `/node_modules/foo/bar/index.d.ts` this is packageDirectory: "foo"
      */
-    function parseNodeModuleFromPath(resolved: PathAndExtension): string | undefined {
-        const path = normalizePath(resolved.path);
+    /* @internal */
+    export function parseNodeModuleFromPath(resolved: string): string | undefined {
+        const path = normalizePath(resolved);
         const idx = path.lastIndexOf(nodeModulesPathPart);
         if (idx === -1) {
             return undefined;

src/compiler/resolutionCache.ts

@@ -153,9 +153,9 @@ namespace ts {
         const resolvedFileToResolution = createMultiMap<ResolutionWithFailedLookupLocations>();
 
         let hasChangedAutomaticTypeDirectiveNames = false;
-        const failedLookupChecks: Path[] = [];
-        const startsWithPathChecks: Path[] = [];
-        const isInDirectoryChecks: Path[] = [];
+        let failedLookupChecks: Path[] | undefined;
+        let startsWithPathChecks: Set<Path> | undefined;
+        let isInDirectoryChecks: Path[] | undefined;
 
         const getCurrentDirectory = memoize(() => resolutionHost.getCurrentDirectory!()); // TODO: GH#18217
         const cachedDirectoryStructureHost = resolutionHost.getCachedDirectoryStructureHost();
@@ -248,9 +248,9 @@ namespace ts {
             resolvedTypeReferenceDirectives.clear();
             resolvedFileToResolution.clear();
             resolutionsWithFailedLookups.length = 0;
-            failedLookupChecks.length = 0;
-            startsWithPathChecks.length = 0;
-            isInDirectoryChecks.length = 0;
+            failedLookupChecks = undefined;
+            startsWithPathChecks = undefined;
+            isInDirectoryChecks = undefined;
             // perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update
             // (between startCachingPerDirectoryResolution and finishCachingPerDirectoryResolution)
             clearPerDirectoryResolutions();
@@ -766,7 +766,7 @@ namespace ts {
             if (isCreatingWatchedDirectory) {
                 // Watching directory is created
                 // Invalidate any resolution has failed lookup in this directory
-                isInDirectoryChecks.push(fileOrDirectoryPath);
+                (isInDirectoryChecks ||= []).push(fileOrDirectoryPath);
             }
             else {
                 // If something to do with folder/file starting with "." in node_modules folder, skip it
@@ -785,8 +785,8 @@ namespace ts {
                 if (isNodeModulesAtTypesDirectory(fileOrDirectoryPath) || isNodeModulesDirectory(fileOrDirectoryPath) ||
                     isNodeModulesAtTypesDirectory(dirOfFileOrDirectory) || isNodeModulesDirectory(dirOfFileOrDirectory)) {
                     // Invalidate any resolution from this directory
-                    failedLookupChecks.push(fileOrDirectoryPath);
-                    startsWithPathChecks.push(fileOrDirectoryPath);
+                    (failedLookupChecks ||= []).push(fileOrDirectoryPath);
+                    (startsWithPathChecks ||= new Set()).add(fileOrDirectoryPath);
                 }
                 else {
                     if (!isPathWithDefaultFailedLookupExtension(fileOrDirectoryPath) && !customFailedLookupPaths.has(fileOrDirectoryPath)) {
@@ -797,30 +797,36 @@ namespace ts {
                         return false;
                     }
                     // Resolution need to be invalidated if failed lookup location is same as the file or directory getting created
-                    failedLookupChecks.push(fileOrDirectoryPath);
+                    (failedLookupChecks ||= []).push(fileOrDirectoryPath);
+
+                    // If the invalidated file is from a node_modules package, invalidate everything else
+                    // in the package since we might not get notifications for other files in the package.
+                    // This hardens our logic against unreliable file watchers.
+                    const packagePath = parseNodeModuleFromPath(fileOrDirectoryPath);
+                    if (packagePath) (startsWithPathChecks ||= new Set()).add(packagePath as Path);
                 }
             }
             resolutionHost.scheduleInvalidateResolutionsOfFailedLookupLocations();
         }
 
         function invalidateResolutionsOfFailedLookupLocations() {
-            if (!failedLookupChecks.length && !startsWithPathChecks.length && !isInDirectoryChecks.length) {
+            if (!failedLookupChecks && !startsWithPathChecks && !isInDirectoryChecks) {
                 return false;
             }
 
             const invalidated = invalidateResolutions(resolutionsWithFailedLookups, canInvalidateFailedLookupResolution);
-            failedLookupChecks.length = 0;
-            startsWithPathChecks.length = 0;
-            isInDirectoryChecks.length = 0;
+            failedLookupChecks = undefined;
+            startsWithPathChecks = undefined;
+            isInDirectoryChecks = undefined;
             return invalidated;
         }
 
         function canInvalidateFailedLookupResolution(resolution: ResolutionWithFailedLookupLocations) {
             return resolution.failedLookupLocations.some(location => {
                 const locationPath = resolutionHost.toPath(location);
                 return contains(failedLookupChecks, locationPath) ||
-                    startsWithPathChecks.some(fileOrDirectoryPath => startsWith(locationPath, fileOrDirectoryPath)) ||
-                    isInDirectoryChecks.some(fileOrDirectoryPath => isInDirectoryPath(fileOrDirectoryPath, locationPath));
+                    firstDefinedIterator(startsWithPathChecks?.keys() || emptyIterator, fileOrDirectoryPath => startsWith(locationPath, fileOrDirectoryPath) ? true : undefined) ||
+                    isInDirectoryChecks?.some(fileOrDirectoryPath => isInDirectoryPath(fileOrDirectoryPath, locationPath));
             });
         }
 

src/testRunner/unittests/tscWatch/resolutionCache.ts

@@ -449,5 +449,84 @@ declare namespace myapp {
             },
             changes: emptyArray
         });
+
+        describe("works when installing something in node_modules or @types when there is no notification from fs for index file", () => {
+            function getNodeAtTypes() {
+                const nodeAtTypesIndex: File = {
+                    path: `${projectRoot}/node_modules/@types/node/index.d.ts`,
+                    content: `/// <reference path="base.d.ts" />`
+                };
+                const nodeAtTypesBase: File = {
+                    path: `${projectRoot}/node_modules/@types/node/base.d.ts`,
+                    content: `// Base definitions for all NodeJS modules that are not specific to any version of TypeScript:
+/// <reference path="ts3.6/base.d.ts" />`
+                };
+                const nodeAtTypes36Base: File = {
+                    path: `${projectRoot}/node_modules/@types/node/ts3.6/base.d.ts`,
+                    content: `/// <reference path="../globals.d.ts" />`
+                };
+                const nodeAtTypesGlobals: File = {
+                    path: `${projectRoot}/node_modules/@types/node/globals.d.ts`,
+                    content: `declare var process: NodeJS.Process;
+declare namespace NodeJS {
+    interface Process {
+        on(msg: string): void;
+    }
+}`
+                };
+                return { nodeAtTypesIndex, nodeAtTypesBase, nodeAtTypes36Base, nodeAtTypesGlobals };
+            }
+            verifyTscWatch({
+                scenario,
+                subScenario: "works when installing something in node_modules or @types when there is no notification from fs for index file",
+                commandLineArgs: ["--w", `--extendedDiagnostics`],
+                sys: () => {
+                    const file: File = {
+                        path: `${projectRoot}/worker.ts`,
+                        content: `process.on("uncaughtException");`
+                    };
+                    const tsconfig: File = {
+                        path: `${projectRoot}/tsconfig.json`,
+                        content: "{}"
+                    };
+                    const { nodeAtTypesIndex, nodeAtTypesBase, nodeAtTypes36Base, nodeAtTypesGlobals } = getNodeAtTypes();
+                    return createWatchedSystem([file, libFile, tsconfig, nodeAtTypesIndex, nodeAtTypesBase, nodeAtTypes36Base, nodeAtTypesGlobals], { currentDirectory: projectRoot });
+                },
+                changes: [
+                    {
+                        caption: "npm ci step one: remove all node_modules files",
+                        change: sys => sys.deleteFolder(`${projectRoot}/node_modules/@types`, /*recursive*/ true),
+                        timeouts: runQueuedTimeoutCallbacks,
+                    },
+                    {
+                        caption: `npm ci step two: create atTypes but something else in the @types folder`,
+                        change: sys => sys.ensureFileOrFolder({
+                            path: `${projectRoot}/node_modules/@types/mocha/index.d.ts`,
+                            content: `export const foo = 10;`
+                        }),
+                        timeouts: runQueuedTimeoutCallbacks
+                    },
+                    {
+                        caption: `npm ci step three: create atTypes node folder`,
+                        change: sys => sys.ensureFileOrFolder({ path: `${projectRoot}/node_modules/@types/node` }),
+                        timeouts: runQueuedTimeoutCallbacks
+                    },
+                    {
+                        caption: `npm ci step four: create atTypes write all the files but dont invoke watcher for index.d.ts`,
+                        change: sys => {
+                            const { nodeAtTypesIndex, nodeAtTypesBase, nodeAtTypes36Base, nodeAtTypesGlobals } = getNodeAtTypes();
+                            sys.ensureFileOrFolder(nodeAtTypesBase);
+                            sys.ensureFileOrFolder(nodeAtTypesIndex, /*ignoreWatchInvokedWithTriggerAsFileCreate*/ true);
+                            sys.ensureFileOrFolder(nodeAtTypes36Base, /*ignoreWatchInvokedWithTriggerAsFileCreate*/ true);
+                            sys.ensureFileOrFolder(nodeAtTypesGlobals, /*ignoreWatchInvokedWithTriggerAsFileCreate*/ true);
+                        },
+                        timeouts: sys => {
+                            sys.runQueuedTimeoutCallbacks(); // update failed lookups
+                            sys.runQueuedTimeoutCallbacks(); // actual program update
+                        },
+                    },
+                ]
+            });
+        });
     });
 }