Skip to content

Commit db9620d

Browse files
sheetalkamatsheetalkamat
committed
Use watch recursive directories instead of watchFile for node_modules and bower components
1 parent d64f248 commit db9620d

5 files changed

Lines changed: 151 additions & 38 deletions

File tree

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ namespace ts.projectSystem {
1313
export import checkArray = TestFSWithWatch.checkArray;
1414
export import libFile = TestFSWithWatch.libFile;
1515
export import checkWatchedFiles = TestFSWithWatch.checkWatchedFiles;
16-
import checkWatchedDirectories = TestFSWithWatch.checkWatchedDirectories;
16+
export import checkWatchedFilesDetailed = TestFSWithWatch.checkWatchedFilesDetailed;
17+
export import checkWatchedDirectories = TestFSWithWatch.checkWatchedDirectories;
18+
export import checkWatchedDirectoriesDetailed = TestFSWithWatch.checkWatchedDirectoriesDetailed;
1719
import safeList = TestFSWithWatch.safeList;
1820

1921
export const customTypesMap = {
@@ -7821,8 +7823,8 @@ namespace ts.projectSystem {
78217823

78227824
checkWatchedDirectories(host, emptyArray, /*recursive*/ true);
78237825

7824-
TestFSWithWatch.checkMultiMapKeyCount("watchedFiles", host.watchedFiles, expectedWatchedFiles);
7825-
TestFSWithWatch.checkMultiMapKeyCount("watchedDirectories", host.watchedDirectories, expectedWatchedDirectories);
7826+
checkWatchedFilesDetailed(host, expectedWatchedFiles);
7827+
checkWatchedDirectoriesDetailed(host, expectedWatchedDirectories, /*recursive*/ false);
78267828
checkProjectActualFiles(project, fileNames);
78277829
}
78287830
}

‎src/harness/unittests/typingsInstaller.ts‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,29 @@ namespace ts.projectSystem {
141141
checkNumberOfProjects(projectService, { configuredProjects: 1 });
142142
const p = configuredProjectAt(projectService, 0);
143143
checkProjectActualFiles(p, [file1.path, tsconfig.path]);
144-
checkWatchedFiles(host, [tsconfig.path, libFile.path, packageJson.path, "/a/b/bower_components", "/a/b/node_modules"]);
144+
145+
const expectedWatchedFiles = createMap<number>();
146+
expectedWatchedFiles.set(tsconfig.path, 1); // tsserver
147+
expectedWatchedFiles.set(libFile.path, 1); // tsserver
148+
expectedWatchedFiles.set(packageJson.path, 1); // typing installer
149+
checkWatchedFilesDetailed(host, expectedWatchedFiles);
150+
151+
checkWatchedDirectories(host, emptyArray, /*recursive*/ false);
152+
153+
const expectedWatchedDirectoriesRecursive = createMap<number>();
154+
expectedWatchedDirectoriesRecursive.set("/a/b", 2); // TypingInstaller and wild card
155+
expectedWatchedDirectoriesRecursive.set("/a/b/node_modules/@types", 1); // type root watch
156+
checkWatchedDirectoriesDetailed(host, expectedWatchedDirectoriesRecursive, /*recursive*/ true);
145157

146158
installer.installAll(/*expectedCount*/ 1);
147159

148160
checkNumberOfProjects(projectService, { configuredProjects: 1 });
149161
host.checkTimeoutQueueLengthAndRun(2);
150162
checkProjectActualFiles(p, [file1.path, jquery.path, tsconfig.path]);
151163
// should not watch jquery
152-
checkWatchedFiles(host, [tsconfig.path, libFile.path, packageJson.path, "/a/b/bower_components", "/a/b/node_modules"]);
164+
checkWatchedFilesDetailed(host, expectedWatchedFiles);
165+
checkWatchedDirectories(host, emptyArray, /*recursive*/ false);
166+
checkWatchedDirectoriesDetailed(host, expectedWatchedDirectoriesRecursive, /*recursive*/ true);
153167
});
154168

155169
it("inferred project (typings installed)", () => {
@@ -827,7 +841,17 @@ namespace ts.projectSystem {
827841
checkNumberOfProjects(projectService, { configuredProjects: 1 });
828842
const p = configuredProjectAt(projectService, 0);
829843
checkProjectActualFiles(p, [app.path, jsconfig.path]);
830-
checkWatchedFiles(host, [jsconfig.path, "/bower_components", "/node_modules", libFile.path]);
844+
845+
const watchedFilesExpected = createMap<number>();
846+
watchedFilesExpected.set(jsconfig.path, 1); // project files
847+
watchedFilesExpected.set(libFile.path, 1); // project files
848+
checkWatchedFilesDetailed(host, watchedFilesExpected);
849+
850+
checkWatchedDirectories(host, emptyArray, /*recursive*/ false);
851+
852+
const watchedRecursiveDirectoriesExpected = createMap<number>();
853+
watchedRecursiveDirectoriesExpected.set("/", 2); // wild card + type installer
854+
checkWatchedDirectoriesDetailed(host, watchedRecursiveDirectoriesExpected, /*recursive*/ true);
831855

832856
installer.installAll(/*expectedCount*/ 1);
833857

‎src/harness/virtualFileSystemWithWatch.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,18 @@ interface Array<T> {}`
179179
checkMapKeys("watchedFiles", host.watchedFiles, expectedFiles);
180180
}
181181

182-
export function checkWatchedDirectories(host: TestServerHost, expectedDirectories: string[], recursive = false) {
182+
export function checkWatchedFilesDetailed(host: TestServerHost, expectedFiles: Map<number>) {
183+
checkMultiMapKeyCount("watchedFiles", host.watchedFiles, expectedFiles);
184+
}
185+
186+
export function checkWatchedDirectories(host: TestServerHost, expectedDirectories: string[], recursive: boolean) {
183187
checkMapKeys(`watchedDirectories${recursive ? " recursive" : ""}`, recursive ? host.watchedDirectoriesRecursive : host.watchedDirectories, expectedDirectories);
184188
}
185189

190+
export function checkWatchedDirectoriesDetailed(host: TestServerHost, expectedDirectories: Map<number>, recursive: boolean) {
191+
checkMultiMapKeyCount(`watchedDirectories${recursive ? " recursive" : ""}`, recursive ? host.watchedDirectoriesRecursive : host.watchedDirectories, expectedDirectories);
192+
}
193+
186194
export function checkOutputContains(host: TestServerHost, expected: ReadonlyArray<string>) {
187195
const mapExpected = arrayToSet(expected);
188196
const mapSeen = createMap<true>();

‎src/server/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,10 @@ declare namespace ts.server {
119119

120120
/* @internal */
121121
export interface InstallTypingHost extends JsTyping.TypingResolutionHost {
122+
useCaseSensitiveFileNames: boolean;
122123
writeFile(path: string, content: string): void;
123124
createDirectory(path: string): void;
124125
watchFile?(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher;
126+
watchDirectory?(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher;
125127
}
126128
}

‎src/server/typingsInstaller/typingsInstaller.ts‎

Lines changed: 108 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ namespace ts.server.typingsInstaller {
6464
onRequestCompleted: RequestCompletedAction;
6565
}
6666

67+
function isPackageOrBowerJson(fileName: string) {
68+
const base = getBaseFileName(fileName);
69+
return base === "package.json" || base === "bower.json";
70+
}
71+
72+
function isInNodeModulesOrBowerComponents(f: string) {
73+
return stringContains(f, "/node_modules/") || stringContains(f, "/bower_components/");
74+
}
75+
6776
type ProjectWatchers = Map<FileWatcher> & { isInvoked?: boolean; };
6877

6978
export abstract class TypingsInstaller {
@@ -73,6 +82,7 @@ namespace ts.server.typingsInstaller {
7382
private readonly projectWatchers = createMap<ProjectWatchers>();
7483
private safeList: JsTyping.SafeList | undefined;
7584
readonly pendingRunRequests: PendingRequest[] = [];
85+
private readonly toCanonicalFileName: GetCanonicalFileName;
7686

7787
private installRunCount = 1;
7888
private inFlightRequestCount = 0;
@@ -86,6 +96,7 @@ namespace ts.server.typingsInstaller {
8696
private readonly typesMapLocation: Path,
8797
private readonly throttleLimit: number,
8898
protected readonly log = nullLog) {
99+
this.toCanonicalFileName = createGetCanonicalFileName(installTypingHost.useCaseSensitiveFileNames);
89100
if (this.log.isEnabled()) {
90101
this.log.writeLine(`Global cache location '${globalCachePath}', safe file path '${safeListPath}', types map path ${typesMapLocation}`);
91102
}
@@ -147,7 +158,7 @@ namespace ts.server.typingsInstaller {
147158
}
148159

149160
// start watching files
150-
this.watchFiles(req.projectName, discoverTypingsResult.filesToWatch);
161+
this.watchFiles(req.projectName, discoverTypingsResult.filesToWatch, req.projectRootPath);
151162

152163
// install typings
153164
if (discoverTypingsResult.newTypingNames.length) {
@@ -367,51 +378,117 @@ namespace ts.server.typingsInstaller {
367378
}
368379
}
369380

370-
private watchFiles(projectName: string, files: string[]) {
381+
private watchFiles(projectName: string, files: string[], projectRootPath: Path) {
371382
if (!files.length) {
372383
// shut down existing watchers
373384
this.closeWatchers(projectName);
374385
return;
375386
}
376387

377388
let watchers = this.projectWatchers.get(projectName);
389+
const toRemove = createMap<FileWatcher>();
378390
if (!watchers) {
379391
watchers = createMap();
380392
this.projectWatchers.set(projectName, watchers);
381393
}
394+
else {
395+
copyEntries(watchers, toRemove);
396+
}
382397

383-
watchers.isInvoked = false;
384398
// handler should be invoked once for the entire set of files since it will trigger full rediscovery of typings
399+
watchers.isInvoked = false;
400+
385401
const isLoggingEnabled = this.log.isEnabled();
386-
mutateMap(
387-
watchers,
388-
arrayToSet(files),
389-
{
390-
// Watch the missing files
391-
createNewValue: file => {
392-
if (isLoggingEnabled) {
393-
this.log.writeLine(`FileWatcher:: Added:: WatchInfo: ${file}`);
394-
}
395-
const watcher = this.installTypingHost.watchFile(file, (f, eventKind) => {
396-
if (isLoggingEnabled) {
397-
this.log.writeLine(`FileWatcher:: Triggered with ${f} eventKind: ${FileWatcherEventKind[eventKind]}:: WatchInfo: ${file}:: handler is already invoked '${watchers.isInvoked}'`);
398-
}
399-
if (!watchers.isInvoked) {
400-
watchers.isInvoked = true;
401-
this.sendResponse({ projectName, kind: ActionInvalidate });
402-
}
403-
}, /*pollingInterval*/ 2000);
404-
return isLoggingEnabled ? {
405-
close: () => {
406-
this.log.writeLine(`FileWatcher:: Closed:: WatchInfo: ${file}`);
407-
}
408-
} : watcher;
409-
},
410-
// Files that are no longer missing (e.g. because they are no longer required)
411-
// should no longer be watched.
412-
onDeleteValue: closeFileWatcher
402+
const createProjectWatcher = (path: string, createWatch: (path: string) => FileWatcher) => {
403+
toRemove.delete(path);
404+
if (watchers.has(path)) {
405+
return;
406+
}
407+
408+
watchers.set(path, createWatch(path));
409+
};
410+
const createProjectFileWatcher = (file: string): FileWatcher => {
411+
if (isLoggingEnabled) {
412+
this.log.writeLine(`FileWatcher:: Added:: WatchInfo: ${file}`);
413+
}
414+
const watcher = this.installTypingHost.watchFile(file, (f, eventKind) => {
415+
if (isLoggingEnabled) {
416+
this.log.writeLine(`FileWatcher:: Triggered with ${f} eventKind: ${FileWatcherEventKind[eventKind]}:: WatchInfo: ${file}:: handler is already invoked '${watchers.isInvoked}'`);
417+
}
418+
if (!watchers.isInvoked) {
419+
watchers.isInvoked = true;
420+
this.sendResponse({ projectName, kind: ActionInvalidate });
421+
}
422+
}, /*pollingInterval*/ 2000);
423+
424+
return isLoggingEnabled ? {
425+
close: () => {
426+
this.log.writeLine(`FileWatcher:: Closed:: WatchInfo: ${file}`);
427+
watcher.close();
428+
}
429+
} : watcher;
430+
};
431+
const createProjectDirectoryWatcher = (dir: string): FileWatcher => {
432+
if (isLoggingEnabled) {
433+
this.log.writeLine(`DirectoryWatcher:: Added:: WatchInfo: ${dir} recursive`);
434+
}
435+
const watcher = this.installTypingHost.watchDirectory(dir, f => {
436+
if (isLoggingEnabled) {
437+
this.log.writeLine(`DirectoryWatcher:: Triggered with ${f} :: WatchInfo: ${dir} recursive :: handler is already invoked '${watchers.isInvoked}'`);
438+
}
439+
if (watchers.isInvoked) {
440+
return;
441+
}
442+
f = this.toCanonicalFileName(f);
443+
if (isPackageOrBowerJson(f) && f !== this.toCanonicalFileName(combinePaths(this.globalCachePath, "package.json"))) {
444+
watchers.isInvoked = true;
445+
this.sendResponse({ projectName, kind: ActionInvalidate });
446+
}
447+
}, /*recursive*/ true);
448+
449+
return isLoggingEnabled ? {
450+
close: () => {
451+
this.log.writeLine(`DirectoryWatcher:: Closed:: WatchInfo: ${dir} recursive`);
452+
watcher.close();
453+
}
454+
} : watcher;
455+
};
456+
457+
// Create watches from list of files
458+
for (const file of files) {
459+
const filePath = this.toCanonicalFileName(file);
460+
if (isPackageOrBowerJson(filePath)) {
461+
// package.json or bower.json exists, watch the file to detect changes and update typings
462+
createProjectWatcher(filePath, createProjectFileWatcher);
463+
continue;
413464
}
414-
);
465+
466+
// path in projectRoot, watch project root
467+
if (containsPath(projectRootPath, filePath, projectRootPath, !this.installTypingHost.useCaseSensitiveFileNames)) {
468+
createProjectWatcher(projectRootPath, createProjectDirectoryWatcher);
469+
continue;
470+
}
471+
472+
// path in global cache, watch global cache
473+
if (containsPath(this.globalCachePath, filePath, projectRootPath, !this.installTypingHost.useCaseSensitiveFileNames)) {
474+
createProjectWatcher(this.globalCachePath, createProjectDirectoryWatcher);
475+
continue;
476+
}
477+
478+
// Get path without node_modules and bower_components
479+
let pathToWatch = getDirectoryPath(filePath);
480+
while (isInNodeModulesOrBowerComponents(pathToWatch)) {
481+
pathToWatch = getDirectoryPath(pathToWatch);
482+
}
483+
484+
createProjectWatcher(pathToWatch, createProjectDirectoryWatcher);
485+
}
486+
487+
// Remove unused watches
488+
toRemove.forEach((watch, path) => {
489+
watch.close();
490+
watchers.delete(path);
491+
});
415492
}
416493

417494
private createSetTypings(request: DiscoverTypings, typings: string[]): SetTypings {

0 commit comments

Comments
 (0)