Skip to content

Commit 7753487

Browse files
sheetalkamatsheetalkamat
authored
Handle delayed or missed watches for project updates (microsoft#59625)
1 parent 195203e commit 7753487

13 files changed

+2203
-247
lines changed

‎src/compiler/watchUtilities.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
FileWatcherCallback,
2222
FileWatcherEventKind,
2323
find,
24+
forEachAncestorDirectory,
2425
getAllowJSCompilerOption,
2526
getBaseFileName,
2627
getDirectoryPath,
@@ -291,6 +292,13 @@ export function createCachedDirectoryStructureHost(host: DirectoryStructureHost,
291292
return host.realpath ? host.realpath(s) : s;
292293
}
293294

295+
function clearFirstAncestorEntry(fileOrDirectoryPath: Path) {
296+
forEachAncestorDirectory(
297+
getDirectoryPath(fileOrDirectoryPath),
298+
ancestor => cachedReadDirectoryResult.delete(ensureTrailingDirectorySeparator(ancestor)) ? true : undefined,
299+
);
300+
}
301+
294302
function addOrDeleteFileOrDirectory(fileOrDirectory: string, fileOrDirectoryPath: Path) {
295303
const existingResult = getCachedFileSystemEntries(fileOrDirectoryPath);
296304
if (existingResult !== undefined) {
@@ -302,6 +310,7 @@ export function createCachedDirectoryStructureHost(host: DirectoryStructureHost,
302310

303311
const parentResult = getCachedFileSystemEntriesForBaseDir(fileOrDirectoryPath);
304312
if (!parentResult) {
313+
clearFirstAncestorEntry(fileOrDirectoryPath);
305314
return undefined;
306315
}
307316

@@ -339,6 +348,9 @@ export function createCachedDirectoryStructureHost(host: DirectoryStructureHost,
339348
if (parentResult) {
340349
updateFilesOfFileSystemEntry(parentResult, getBaseNameOfFileName(fileName), eventKind === FileWatcherEventKind.Created);
341350
}
351+
else {
352+
clearFirstAncestorEntry(filePath);
353+
}
342354
}
343355

344356
function updateFilesOfFileSystemEntry(parentResult: SortedAndCanonicalizedMutableFileSystemEntries, baseName: string, fileExists: boolean): void {

‎src/server/editorServices.ts

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import {
8080
length,
8181
map,
8282
mapDefinedIterator,
83+
memoize,
8384
missingFileModifiedTime,
8485
MultiMap,
8586
noop,
@@ -1912,6 +1913,11 @@ export class ProjectService {
19121913
this.watchPackageJsonFile(file, fileOrDirectoryPath, wildCardWatcher);
19131914
}
19141915

1916+
if (!fsResult?.fileExists) {
1917+
// Ensure we send sourceFileChange
1918+
this.sendSourceFileChange(fileOrDirectoryPath);
1919+
}
1920+
19151921
const configuredProjectForConfig = this.findConfiguredProjectByProjectName(configFileName);
19161922
if (
19171923
isIgnoredFileFromWildCardWatching({
@@ -3838,6 +3844,34 @@ export class ProjectService {
38383844
this.logger.close();
38393845
}
38403846

3847+
private sendSourceFileChange(inPath: Path | undefined) {
3848+
this.filenameToScriptInfo.forEach(info => {
3849+
if (this.openFiles.has(info.path)) return; // Skip open files
3850+
if (!info.fileWatcher) return; // not watched file
3851+
const eventKind = memoize(() =>
3852+
this.host.fileExists(info.fileName) ?
3853+
info.deferredDelete ?
3854+
FileWatcherEventKind.Created :
3855+
FileWatcherEventKind.Changed :
3856+
FileWatcherEventKind.Deleted
3857+
);
3858+
if (inPath) {
3859+
// Skip node modules and files that are not in path
3860+
if (isScriptInfoWatchedFromNodeModules(info) || !info.path.startsWith(inPath)) return;
3861+
// If we are sending delete event and its already deleted, ignore
3862+
if (eventKind() === FileWatcherEventKind.Deleted && info.deferredDelete) return;
3863+
// In change cases, its hard to know if this is marked correctly across files and projects, so just send the event
3864+
this.logger.info(`Invoking sourceFileChange on ${info.fileName}:: ${eventKind()}`);
3865+
}
3866+
3867+
// Handle as if file is changed or deleted
3868+
this.onSourceFileChanged(
3869+
info,
3870+
eventKind(),
3871+
);
3872+
});
3873+
}
3874+
38413875
/**
38423876
* This function rebuilds the project for every file opened by the client
38433877
* This does not reload contents of open files from disk. But we could do that if needed
@@ -3850,19 +3884,7 @@ export class ProjectService {
38503884
// as there is no need to load contents of the files from the disk
38513885

38523886
// Reload script infos
3853-
this.filenameToScriptInfo.forEach(info => {
3854-
if (this.openFiles.has(info.path)) return; // Skip open files
3855-
if (!info.fileWatcher) return; // not watched file
3856-
// Handle as if file is changed or deleted
3857-
this.onSourceFileChanged(
3858-
info,
3859-
this.host.fileExists(info.fileName) ?
3860-
info.deferredDelete ?
3861-
FileWatcherEventKind.Created :
3862-
FileWatcherEventKind.Changed :
3863-
FileWatcherEventKind.Deleted,
3864-
);
3865-
});
3887+
this.sendSourceFileChange(/*inPath*/ undefined);
38663888
// Cancel all project updates since we will be updating them now
38673889
this.pendingProjectUpdates.forEach((_project, projectName) => {
38683890
this.throttledOperations.cancel(projectName);

‎src/testRunner/unittests/helpers/virtualFileSystemWithWatch.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -556,33 +556,33 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
556556
this.addFileOrFolderInFolder(baseFolder, newFile);
557557
}
558558

559-
renameFolder(folderName: string, newFolderName: string) {
559+
renameFolder(folderName: string, newFolderName: string, skipFolderEntryWatches?: true) {
560560
const fullPath = getNormalizedAbsolutePath(folderName, this.currentDirectory);
561561
const path = this.toPath(fullPath);
562562
const folder = this.fs.get(path) as FsFolder;
563563
Debug.assert(!!folder);
564564

565+
const newFullPath = getNormalizedAbsolutePath(newFolderName, this.currentDirectory);
566+
const newFolder = this.toFsFolder(newFullPath);
567+
568+
// Invoke watches for files in the folder as deleted (from old path)
569+
this.renameFolderEntries(folder, newFolder, skipFolderEntryWatches);
570+
565571
// Only remove the folder
566572
this.removeFileOrFolder(folder, /*isRenaming*/ true);
567573

568574
// Add updated folder with new folder name
569-
const newFullPath = getNormalizedAbsolutePath(newFolderName, this.currentDirectory);
570-
const newFolder = this.toFsFolder(newFullPath);
571575
const newPath = newFolder.path;
572-
const basePath = getDirectoryPath(path);
573-
Debug.assert(basePath !== path);
574-
Debug.assert(basePath === getDirectoryPath(newPath));
576+
const basePath = getDirectoryPath(newPath);
577+
this.ensureFileOrFolder({ path: getDirectoryPath(newFullPath) });
575578
const baseFolder = this.fs.get(basePath) as FsFolder;
576579
this.addFileOrFolderInFolder(baseFolder, newFolder);
577-
578-
// Invoke watches for files in the folder as deleted (from old path)
579-
this.renameFolderEntries(folder, newFolder);
580580
}
581581

582-
private renameFolderEntries(oldFolder: FsFolder, newFolder: FsFolder) {
582+
private renameFolderEntries(oldFolder: FsFolder, newFolder: FsFolder, skipWatches: true | undefined) {
583583
for (const entry of oldFolder.entries) {
584584
this.fs.delete(entry.path);
585-
this.invokeFileAndFsWatches(entry.fullPath, FileWatcherEventKind.Deleted, entry.fullPath);
585+
if (!skipWatches) this.invokeFileAndFsWatches(entry.fullPath, FileWatcherEventKind.Deleted, entry.fullPath);
586586

587587
entry.fullPath = combinePaths(newFolder.fullPath, getBaseFileName(entry.fullPath));
588588
entry.path = this.toPath(entry.fullPath);
@@ -591,9 +591,9 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
591591
}
592592
this.fs.set(entry.path, entry);
593593
this.setInode(entry.path);
594-
this.invokeFileAndFsWatches(entry.fullPath, FileWatcherEventKind.Created, entry.fullPath);
594+
if (!skipWatches) this.invokeFileAndFsWatches(entry.fullPath, FileWatcherEventKind.Created, entry.fullPath);
595595
if (isFsFolder(entry)) {
596-
this.renameFolderEntries(entry, entry);
596+
this.renameFolderEntries(entry, entry, skipWatches);
597597
}
598598
}
599599
}

‎src/testRunner/unittests/tsserver/getEditsForFileRename.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import * as ts from "../../_namespaces/ts.js";
2+
import { dedent } from "../../_namespaces/Utils.js";
23
import { jsonToReadableText } from "../helpers.js";
4+
import { libContent } from "../helpers/contents.js";
35
import {
46
baselineTsserverLogs,
57
closeFilesForSession,
68
openFilesForSession,
9+
protocolTextSpanFromSubstring,
710
TestSession,
811
textSpanFromSubstring,
912
verifyGetErrRequest,
@@ -207,4 +210,89 @@ describe("unittests:: tsserver:: getEditsForFileRename", () => {
207210
});
208211
})
209212
);
213+
214+
[true, false].forEach(canUseWatchEvents =>
215+
it(`works when moving file to and from folder${canUseWatchEvents ? " canUseWatchEvents" : ""}`, () => {
216+
const alertText = dedent`
217+
export function alert(message: string) {
218+
console.log(\`ALERT: \${message}\`);
219+
}
220+
`;
221+
const projectRootPath = "/home/src/myprojects/project";
222+
const indexTs = `${projectRootPath}/index.ts`;
223+
const indexFileText = dedent`
224+
import { alert } from '@app/components/whatever/alert';
225+
alert('Hello, world!');
226+
`;
227+
const componentsWhatever = `${projectRootPath}/components/whatever`;
228+
const functionsWhatever = `${projectRootPath}/functions/whatever`;
229+
const host = createServerHost({
230+
"/home/src/myprojects/project/tsconfig.json": jsonToReadableText({
231+
compilerOptions: {
232+
target: "es2016",
233+
module: "commonjs",
234+
paths: {
235+
"@app/*": [
236+
"./*",
237+
],
238+
},
239+
esModuleInterop: true,
240+
forceConsistentCasingInFileNames: true,
241+
strict: true,
242+
skipLibCheck: true,
243+
},
244+
}),
245+
[indexTs]: indexFileText,
246+
[`${componentsWhatever}/alert.ts`]: alertText,
247+
[`${functionsWhatever}/placeholder.txt`]: "",
248+
"/a/lib/lib.es2016.full.d.ts": libContent,
249+
});
250+
const session = new TestSession({ host, canUseWatchEvents, canUseEvents: true });
251+
openFilesForSession([{ file: indexTs, projectRootPath }], session);
252+
host.renameFolder(componentsWhatever, functionsWhatever, /*skipFolderEntryWatches*/ true);
253+
if (canUseWatchEvents) session.invokeWatchChanges();
254+
openFilesForSession([{
255+
file: `${functionsWhatever}/alert.ts`,
256+
content: alertText,
257+
projectRootPath,
258+
}], session);
259+
session.executeCommandSeq<ts.server.protocol.GetEditsForFileRenameRequest>({
260+
command: ts.server.protocol.CommandTypes.GetEditsForFileRename,
261+
arguments: { oldFilePath: componentsWhatever, newFilePath: functionsWhatever },
262+
});
263+
// Apply edit to index.ts
264+
session.executeCommandSeq<ts.server.protocol.UpdateOpenRequest>({
265+
command: ts.server.protocol.CommandTypes.UpdateOpen,
266+
arguments: {
267+
changedFiles: [{
268+
fileName: indexTs,
269+
textChanges: [{
270+
...protocolTextSpanFromSubstring(indexFileText, "@app/components/whatever/alert"),
271+
newText: "@app/functions/whatever/alert",
272+
}],
273+
}],
274+
},
275+
});
276+
host.runQueuedTimeoutCallbacks();
277+
host.renameFolder(functionsWhatever, componentsWhatever, /*skipFolderEntryWatches*/ true);
278+
session.executeCommandSeq<ts.server.protocol.UpdateOpenRequest>({
279+
command: ts.server.protocol.CommandTypes.UpdateOpen,
280+
arguments: {
281+
openFiles: [{
282+
file: `${componentsWhatever}/alert.ts`,
283+
fileContent: alertText,
284+
projectRootPath,
285+
}],
286+
closedFiles: [`${functionsWhatever}/alert.ts`],
287+
},
288+
});
289+
session.executeCommandSeq<ts.server.protocol.GetEditsForFileRenameRequest>({
290+
command: ts.server.protocol.CommandTypes.GetEditsForFileRename,
291+
arguments: { oldFilePath: functionsWhatever, newFilePath: componentsWhatever },
292+
});
293+
if (canUseWatchEvents) session.invokeWatchChanges();
294+
host.runQueuedTimeoutCallbacks();
295+
baselineTsserverLogs("getEditsForFileRename", `works when moving file to and from folder${canUseWatchEvents ? " canUseWatchEvents" : ""}`, session);
296+
})
297+
);
210298
});

‎src/testRunner/unittests/tsserver/projects.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,19 +1375,38 @@ describe("unittests:: tsserver:: projects::", () => {
13751375
host.runQueuedTimeoutCallbacks();
13761376

13771377
// file is deleted but watches are not yet invoked
1378-
const originalFileExists = host.fileExists;
1379-
host.fileExists = s => s === fileA.path ? false : originalFileExists.call(host, s);
1378+
const invokeFileWatcher = host.invokeFileWatcher;
1379+
const fileWatches: Parameters<typeof host.invokeFileWatcher>[] = [];
1380+
host.invokeFileWatcher = (fileFullPath, eventKind, modifiedTime) => {
1381+
fileWatches.push([fileFullPath, eventKind, modifiedTime]);
1382+
};
1383+
const invokeFsWatchesCallbacks = host.invokeFsWatchesCallbacks;
1384+
const fsWatches: Parameters<typeof host.invokeFsWatchesCallbacks>[] = [];
1385+
host.invokeFsWatchesCallbacks = (fullPath, eventName, eventFullPath, useTildeSuffix) => {
1386+
fsWatches.push([fullPath, eventName, eventFullPath, useTildeSuffix]);
1387+
};
1388+
const invokeFsWatchesRecursiveCallbacks = host.invokeFsWatchesRecursiveCallbacks;
1389+
const fsWatchesRecursive: Parameters<typeof host.invokeFsWatchesRecursiveCallbacks>[] = [];
1390+
host.invokeFsWatchesRecursiveCallbacks = (fullPath, eventName, eventFullPath, useTildeSuffix) => {
1391+
fsWatchesRecursive.push([fullPath, eventName, eventFullPath, useTildeSuffix]);
1392+
};
1393+
1394+
host.deleteFile(fileA.path);
1395+
host.ensureFileOrFolder(fileSubA);
13801396
closeFilesForSession([fileA], session);
13811397

13821398
// This should create inferred project since fileSubA not on the disk
13831399
openFile(fileSubA);
13841400

13851401
host.runQueuedTimeoutCallbacks(); // Update configured project and projects for open file
1386-
host.fileExists = originalFileExists;
1402+
host.invokeFileWatcher = invokeFileWatcher;
1403+
host.invokeFsWatchesCallbacks = invokeFsWatchesCallbacks;
1404+
host.invokeFsWatchesRecursiveCallbacks = invokeFsWatchesRecursiveCallbacks;
13871405

13881406
// Actually trigger the file move
1389-
host.deleteFile(fileA.path);
1390-
host.ensureFileOrFolder(fileSubA);
1407+
fileWatches.forEach(args => host.invokeFileWatcher(...args));
1408+
fsWatches.forEach(args => host.invokeFsWatchesCallbacks(...args));
1409+
fsWatchesRecursive.forEach(args => host.invokeFsWatchesRecursiveCallbacks(...args));
13911410

13921411
verifyGetErrRequest({ session, files: [fileB, fileSubA], existingTimeouts: true });
13931412
baselineTsserverLogs("projects", "handles delayed directory watch invoke on file creation", session);

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3318,6 +3318,7 @@ declare namespace ts {
33183318
setHostConfiguration(args: protocol.ConfigureRequestArguments): void;
33193319
private getWatchOptionsFromProjectWatchOptions;
33203320
closeLog(): void;
3321+
private sendSourceFileChange;
33213322
/**
33223323
* This function rebuilds the project for every file opened by the client
33233324
* This does not reload contents of open files from disk. But we could do that if needed

‎tests/baselines/reference/tscWatch/resolutionCache/works-when-renaming-node_modules-folder-that-already-contains-@types-folder.js

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ export {}
8686
//// [/user/username/projects/myproject/node_modules2/@types/qqq/index.d.ts] deleted
8787

8888
Output::
89-
sysLog:: /user/username/projects/myproject/node_modules:: Changing watcher to PresentFileSystemEntryWatcher
9089
sysLog:: /user/username/projects/myproject/node_modules/@types:: Changing watcher to PresentFileSystemEntryWatcher
90+
sysLog:: /user/username/projects/myproject/node_modules:: Changing watcher to PresentFileSystemEntryWatcher
9191

9292

9393
PolledWatches::
@@ -115,14 +115,13 @@ FsWatchesRecursive::
115115
{}
116116

117117
Timeout callback:: count: 2
118-
11: timerToUpdateProgram *new*
119-
13: timerToInvalidateFailedLookupResolutions *new*
118+
7: timerToUpdateProgram *new*
119+
10: timerToInvalidateFailedLookupResolutions *new*
120120

121121
Before running Timeout callback:: count: 2
122-
11: timerToUpdateProgram
123-
13: timerToInvalidateFailedLookupResolutions
122+
7: timerToUpdateProgram
123+
10: timerToInvalidateFailedLookupResolutions
124124

125-
Host is moving to new time
126125
After running Timeout callback:: count: 0
127126
Output::
128127
>> Screen clear
@@ -167,7 +166,7 @@ FsWatchesRecursive::
167166
{}
168167

169168
Timeout callback:: count: 0
170-
13: timerToInvalidateFailedLookupResolutions *deleted*
169+
10: timerToInvalidateFailedLookupResolutions *deleted*
171170

172171

173172
Program root files: [

0 commit comments

Comments
 (0)