Skip to content

Commit bc7786b

Browse files
authored
Ensure that file watcher is closed only once for affected file locations that share watcher because of different names but same real path (microsoft#50150)
* Add test where clearing affected files watcher that also is shared by real path causes Debug failure * Ensure that file watcher is closed only once for affected file locations that share watcher because of different names but same real path * Lift up package json map
1 parent d6d2643 commit bc7786b

File tree

5 files changed

+188
-15
lines changed

5 files changed

+188
-15
lines changed

src/compiler/resolutionCache.ts

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,6 @@ namespace ts {
360360
if (watcher.files === 0 && watcher.resolutions === 0) {
361361
fileWatchesOfAffectingLocations.delete(path);
362362
watcher.watcher.close();
363-
// Ensure when watching symlinked package.json, we can close the actual file watcher only once
364-
watcher.watcher = noopFileWatcher;
365363
}
366364
});
367365

@@ -771,17 +769,25 @@ namespace ts {
771769
}
772770
const paths = new Set<string>();
773771
paths.add(locationToWatch);
772+
let actualWatcher = canWatchDirectoryOrFile(resolutionHost.toPath(locationToWatch)) ?
773+
resolutionHost.watchAffectingFileLocation(locationToWatch, (fileName, eventKind) => {
774+
cachedDirectoryStructureHost?.addOrDeleteFile(fileName, resolutionHost.toPath(locationToWatch), eventKind);
775+
const packageJsonMap = moduleResolutionCache.getPackageJsonInfoCache().getInternalMap();
776+
paths.forEach(path => {
777+
if (watcher.resolutions) (affectingPathChecks ??= new Set()).add(path);
778+
if (watcher.files) (affectingPathChecksForFile ??= new Set()).add(path);
779+
packageJsonMap?.delete(resolutionHost.toPath(path));
780+
});
781+
resolutionHost.scheduleInvalidateResolutionsOfFailedLookupLocations();
782+
}) : noopFileWatcher;
774783
const watcher: FileWatcherOfAffectingLocation = {
775-
watcher: canWatchDirectoryOrFile(resolutionHost.toPath(locationToWatch)) ?
776-
resolutionHost.watchAffectingFileLocation(locationToWatch, (fileName, eventKind) => {
777-
cachedDirectoryStructureHost?.addOrDeleteFile(fileName, resolutionHost.toPath(locationToWatch), eventKind);
778-
paths.forEach(path => {
779-
if (watcher.resolutions) (affectingPathChecks ??= new Set()).add(path);
780-
if (watcher.files) (affectingPathChecksForFile ??= new Set()).add(path);
781-
moduleResolutionCache.getPackageJsonInfoCache().getInternalMap()?.delete(resolutionHost.toPath(path));
782-
});
783-
resolutionHost.scheduleInvalidateResolutionsOfFailedLookupLocations();
784-
}) : noopFileWatcher,
784+
watcher: actualWatcher !== noopFileWatcher ? {
785+
close: () => {
786+
actualWatcher.close();
787+
// Ensure when watching symlinked package.json, we can close the actual file watcher only once
788+
actualWatcher = noopFileWatcher;
789+
}
790+
} : actualWatcher,
785791
resolutions: forResolution ? 1 : 0,
786792
files: forResolution ? 0 : 1,
787793
paths,

src/harness/virtualFileSystemWithWatch.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,14 @@ interface Array<T> { length: number; [n: number]: T; }`
116116

117117
function createWatcher<T>(map: MultiMap<Path, T>, path: Path, callback: T): FileWatcher {
118118
map.add(path, callback);
119-
return { close: () => map.remove(path, callback) };
119+
let closed = false;
120+
return {
121+
close: () => {
122+
Debug.assert(!closed);
123+
map.remove(path, callback);
124+
closed = true;
125+
}
126+
};
120127
}
121128

122129
export function getDiffInKeys<T>(map: ESMap<string, T>, expectedKeys: readonly string[]) {

src/testRunner/unittests/tsserver/symLinks.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,16 @@ new C();`
131131
host.runQueuedTimeoutCallbacks(); // Actual update
132132

133133
verifyGetErrRequest({ session, host, files: [recognizersDateTimeSrcFile] });
134+
135+
// Change config file's module resolution affecting option
136+
const config = JSON.parse(host.readFile(recognizerDateTimeTsconfigPath)!);
137+
host.writeFile(recognizerDateTimeTsconfigPath, JSON.stringify({
138+
...config,
139+
compilerOptions: { ...config.compilerOptions, resolveJsonModule: true }
140+
}));
141+
host.runQueuedTimeoutCallbacks(); // Scheduled invalidation of resolutions
142+
host.runQueuedTimeoutCallbacks(); // Actual update
143+
134144
baselineTsserverLogs("symLinks", `module resolution${withPathMapping ? " with path mapping" : ""} when project compiles from sources`, session);
135145
});
136146

tests/baselines/reference/tsserver/symLinks/module-resolution-when-project-compiles-from-sources.js

Lines changed: 74 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)