Skip to content

Commit 0526ff5

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request microsoft#23484 from Microsoft/typingInstallerWatch
Use watch recursive directories instead of watchFile for node_modules and bower components
2 parents 54ad9b1 + 56b618b commit 0526ff5

5 files changed

Lines changed: 153 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: 110 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,20 @@ 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 getDirectoryExcludingNodeModulesOrBowerComponents(f: string) {
73+
const indexOfNodeModules = f.indexOf("/node_modules/");
74+
const indexOfBowerComponents = f.indexOf("/bower_components/");
75+
const subStrLength = indexOfNodeModules === -1 || indexOfBowerComponents === -1 ?
76+
Math.max(indexOfNodeModules, indexOfBowerComponents) :
77+
Math.min(indexOfNodeModules, indexOfBowerComponents);
78+
return subStrLength === -1 ? f : f.substr(0, subStrLength);
79+
}
80+
6781
type ProjectWatchers = Map<FileWatcher> & { isInvoked?: boolean; };
6882

6983
export abstract class TypingsInstaller {
@@ -73,6 +87,8 @@ namespace ts.server.typingsInstaller {
7387
private readonly projectWatchers = createMap<ProjectWatchers>();
7488
private safeList: JsTyping.SafeList | undefined;
7589
readonly pendingRunRequests: PendingRequest[] = [];
90+
private readonly toCanonicalFileName: GetCanonicalFileName;
91+
private readonly globalCacheCanonicalPackageJsonPath: string;
7692

7793
private installRunCount = 1;
7894
private inFlightRequestCount = 0;
@@ -86,6 +102,8 @@ namespace ts.server.typingsInstaller {
86102
private readonly typesMapLocation: Path,
87103
private readonly throttleLimit: number,
88104
protected readonly log = nullLog) {
105+
this.toCanonicalFileName = createGetCanonicalFileName(installTypingHost.useCaseSensitiveFileNames);
106+
this.globalCacheCanonicalPackageJsonPath = combinePaths(this.toCanonicalFileName(globalCachePath), "package.json");
89107
if (this.log.isEnabled()) {
90108
this.log.writeLine(`Global cache location '${globalCachePath}', safe file path '${safeListPath}', types map path ${typesMapLocation}`);
91109
}
@@ -147,7 +165,7 @@ namespace ts.server.typingsInstaller {
147165
}
148166

149167
// start watching files
150-
this.watchFiles(req.projectName, discoverTypingsResult.filesToWatch);
168+
this.watchFiles(req.projectName, discoverTypingsResult.filesToWatch, req.projectRootPath);
151169

152170
// install typings
153171
if (discoverTypingsResult.newTypingNames.length) {
@@ -367,51 +385,112 @@ namespace ts.server.typingsInstaller {
367385
}
368386
}
369387

370-
private watchFiles(projectName: string, files: string[]) {
388+
private watchFiles(projectName: string, files: string[], projectRootPath: Path) {
371389
if (!files.length) {
372390
// shut down existing watchers
373391
this.closeWatchers(projectName);
374392
return;
375393
}
376394

377395
let watchers = this.projectWatchers.get(projectName);
396+
const toRemove = createMap<FileWatcher>();
378397
if (!watchers) {
379398
watchers = createMap();
380399
this.projectWatchers.set(projectName, watchers);
381400
}
401+
else {
402+
copyEntries(watchers, toRemove);
403+
}
382404

383-
watchers.isInvoked = false;
384405
// handler should be invoked once for the entire set of files since it will trigger full rediscovery of typings
406+
watchers.isInvoked = false;
407+
385408
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
409+
const createProjectWatcher = (path: string, createWatch: (path: string) => FileWatcher) => {
410+
toRemove.delete(path);
411+
if (watchers.has(path)) {
412+
return;
413+
}
414+
415+
watchers.set(path, createWatch(path));
416+
};
417+
const createProjectFileWatcher = (file: string): FileWatcher => {
418+
if (isLoggingEnabled) {
419+
this.log.writeLine(`FileWatcher:: Added:: WatchInfo: ${file}`);
420+
}
421+
const watcher = this.installTypingHost.watchFile(file, (f, eventKind) => {
422+
if (isLoggingEnabled) {
423+
this.log.writeLine(`FileWatcher:: Triggered with ${f} eventKind: ${FileWatcherEventKind[eventKind]}:: WatchInfo: ${file}:: handler is already invoked '${watchers.isInvoked}'`);
424+
}
425+
if (!watchers.isInvoked) {
426+
watchers.isInvoked = true;
427+
this.sendResponse({ projectName, kind: ActionInvalidate });
428+
}
429+
}, /*pollingInterval*/ 2000);
430+
431+
return isLoggingEnabled ? {
432+
close: () => {
433+
this.log.writeLine(`FileWatcher:: Closed:: WatchInfo: ${file}`);
434+
watcher.close();
435+
}
436+
} : watcher;
437+
};
438+
const createProjectDirectoryWatcher = (dir: string): FileWatcher => {
439+
if (isLoggingEnabled) {
440+
this.log.writeLine(`DirectoryWatcher:: Added:: WatchInfo: ${dir} recursive`);
441+
}
442+
const watcher = this.installTypingHost.watchDirectory(dir, f => {
443+
if (isLoggingEnabled) {
444+
this.log.writeLine(`DirectoryWatcher:: Triggered with ${f} :: WatchInfo: ${dir} recursive :: handler is already invoked '${watchers.isInvoked}'`);
445+
}
446+
if (watchers.isInvoked) {
447+
return;
448+
}
449+
f = this.toCanonicalFileName(f);
450+
if (f !== this.globalCacheCanonicalPackageJsonPath && isPackageOrBowerJson(f)) {
451+
watchers.isInvoked = true;
452+
this.sendResponse({ projectName, kind: ActionInvalidate });
453+
}
454+
}, /*recursive*/ true);
455+
456+
return isLoggingEnabled ? {
457+
close: () => {
458+
this.log.writeLine(`DirectoryWatcher:: Closed:: WatchInfo: ${dir} recursive`);
459+
watcher.close();
460+
}
461+
} : watcher;
462+
};
463+
464+
// Create watches from list of files
465+
for (const file of files) {
466+
const filePath = this.toCanonicalFileName(file);
467+
if (isPackageOrBowerJson(filePath)) {
468+
// package.json or bower.json exists, watch the file to detect changes and update typings
469+
createProjectWatcher(filePath, createProjectFileWatcher);
470+
continue;
471+
}
472+
473+
// path in projectRoot, watch project root
474+
if (containsPath(projectRootPath, filePath, projectRootPath, !this.installTypingHost.useCaseSensitiveFileNames)) {
475+
createProjectWatcher(projectRootPath, createProjectDirectoryWatcher);
476+
continue;
477+
}
478+
479+
// path in global cache, watch global cache
480+
if (containsPath(this.globalCachePath, filePath, projectRootPath, !this.installTypingHost.useCaseSensitiveFileNames)) {
481+
createProjectWatcher(this.globalCachePath, createProjectDirectoryWatcher);
482+
continue;
413483
}
414-
);
484+
485+
// Get path without node_modules and bower_components
486+
createProjectWatcher(getDirectoryExcludingNodeModulesOrBowerComponents(getDirectoryPath(filePath)), createProjectDirectoryWatcher);
487+
}
488+
489+
// Remove unused watches
490+
toRemove.forEach((watch, path) => {
491+
watch.close();
492+
watchers.delete(path);
493+
});
415494
}
416495

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

0 commit comments

Comments
 (0)