From db9620d8f01f1047fe93102bf37527ff30b43c83 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 17 Apr 2018 12:18:49 -0700 Subject: [PATCH 1/3] Use watch recursive directories instead of watchFile for node_modules and bower components --- .../unittests/tsserverProjectSystem.ts | 8 +- src/harness/unittests/typingsInstaller.ts | 30 +++- src/harness/virtualFileSystemWithWatch.ts | 10 +- src/server/types.ts | 2 + .../typingsInstaller/typingsInstaller.ts | 139 ++++++++++++++---- 5 files changed, 151 insertions(+), 38 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 10912b4fb87d8..a35f797eaa1c3 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -13,7 +13,9 @@ namespace ts.projectSystem { export import checkArray = TestFSWithWatch.checkArray; export import libFile = TestFSWithWatch.libFile; export import checkWatchedFiles = TestFSWithWatch.checkWatchedFiles; - import checkWatchedDirectories = TestFSWithWatch.checkWatchedDirectories; + export import checkWatchedFilesDetailed = TestFSWithWatch.checkWatchedFilesDetailed; + export import checkWatchedDirectories = TestFSWithWatch.checkWatchedDirectories; + export import checkWatchedDirectoriesDetailed = TestFSWithWatch.checkWatchedDirectoriesDetailed; import safeList = TestFSWithWatch.safeList; export const customTypesMap = { @@ -7821,8 +7823,8 @@ namespace ts.projectSystem { checkWatchedDirectories(host, emptyArray, /*recursive*/ true); - TestFSWithWatch.checkMultiMapKeyCount("watchedFiles", host.watchedFiles, expectedWatchedFiles); - TestFSWithWatch.checkMultiMapKeyCount("watchedDirectories", host.watchedDirectories, expectedWatchedDirectories); + checkWatchedFilesDetailed(host, expectedWatchedFiles); + checkWatchedDirectoriesDetailed(host, expectedWatchedDirectories, /*recursive*/ false); checkProjectActualFiles(project, fileNames); } } diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index b6d5a20ef9a8d..a8c7d4895d18b 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -141,7 +141,19 @@ namespace ts.projectSystem { checkNumberOfProjects(projectService, { configuredProjects: 1 }); const p = configuredProjectAt(projectService, 0); checkProjectActualFiles(p, [file1.path, tsconfig.path]); - checkWatchedFiles(host, [tsconfig.path, libFile.path, packageJson.path, "/a/b/bower_components", "/a/b/node_modules"]); + + const expectedWatchedFiles = createMap(); + expectedWatchedFiles.set(tsconfig.path, 1); // tsserver + expectedWatchedFiles.set(libFile.path, 1); // tsserver + expectedWatchedFiles.set(packageJson.path, 1); // typing installer + checkWatchedFilesDetailed(host, expectedWatchedFiles); + + checkWatchedDirectories(host, emptyArray, /*recursive*/ false); + + const expectedWatchedDirectoriesRecursive = createMap(); + expectedWatchedDirectoriesRecursive.set("/a/b", 2); // TypingInstaller and wild card + expectedWatchedDirectoriesRecursive.set("/a/b/node_modules/@types", 1); // type root watch + checkWatchedDirectoriesDetailed(host, expectedWatchedDirectoriesRecursive, /*recursive*/ true); installer.installAll(/*expectedCount*/ 1); @@ -149,7 +161,9 @@ namespace ts.projectSystem { host.checkTimeoutQueueLengthAndRun(2); checkProjectActualFiles(p, [file1.path, jquery.path, tsconfig.path]); // should not watch jquery - checkWatchedFiles(host, [tsconfig.path, libFile.path, packageJson.path, "/a/b/bower_components", "/a/b/node_modules"]); + checkWatchedFilesDetailed(host, expectedWatchedFiles); + checkWatchedDirectories(host, emptyArray, /*recursive*/ false); + checkWatchedDirectoriesDetailed(host, expectedWatchedDirectoriesRecursive, /*recursive*/ true); }); it("inferred project (typings installed)", () => { @@ -827,7 +841,17 @@ namespace ts.projectSystem { checkNumberOfProjects(projectService, { configuredProjects: 1 }); const p = configuredProjectAt(projectService, 0); checkProjectActualFiles(p, [app.path, jsconfig.path]); - checkWatchedFiles(host, [jsconfig.path, "/bower_components", "/node_modules", libFile.path]); + + const watchedFilesExpected = createMap(); + watchedFilesExpected.set(jsconfig.path, 1); // project files + watchedFilesExpected.set(libFile.path, 1); // project files + checkWatchedFilesDetailed(host, watchedFilesExpected); + + checkWatchedDirectories(host, emptyArray, /*recursive*/ false); + + const watchedRecursiveDirectoriesExpected = createMap(); + watchedRecursiveDirectoriesExpected.set("/", 2); // wild card + type installer + checkWatchedDirectoriesDetailed(host, watchedRecursiveDirectoriesExpected, /*recursive*/ true); installer.installAll(/*expectedCount*/ 1); diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts index 71b7da2ed142e..3756f435d4371 100644 --- a/src/harness/virtualFileSystemWithWatch.ts +++ b/src/harness/virtualFileSystemWithWatch.ts @@ -179,10 +179,18 @@ interface Array {}` checkMapKeys("watchedFiles", host.watchedFiles, expectedFiles); } - export function checkWatchedDirectories(host: TestServerHost, expectedDirectories: string[], recursive = false) { + export function checkWatchedFilesDetailed(host: TestServerHost, expectedFiles: Map) { + checkMultiMapKeyCount("watchedFiles", host.watchedFiles, expectedFiles); + } + + export function checkWatchedDirectories(host: TestServerHost, expectedDirectories: string[], recursive: boolean) { checkMapKeys(`watchedDirectories${recursive ? " recursive" : ""}`, recursive ? host.watchedDirectoriesRecursive : host.watchedDirectories, expectedDirectories); } + export function checkWatchedDirectoriesDetailed(host: TestServerHost, expectedDirectories: Map, recursive: boolean) { + checkMultiMapKeyCount(`watchedDirectories${recursive ? " recursive" : ""}`, recursive ? host.watchedDirectoriesRecursive : host.watchedDirectories, expectedDirectories); + } + export function checkOutputContains(host: TestServerHost, expected: ReadonlyArray) { const mapExpected = arrayToSet(expected); const mapSeen = createMap(); diff --git a/src/server/types.ts b/src/server/types.ts index d4ddd81c53ef8..184a121522efe 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -119,8 +119,10 @@ declare namespace ts.server { /* @internal */ export interface InstallTypingHost extends JsTyping.TypingResolutionHost { + useCaseSensitiveFileNames: boolean; writeFile(path: string, content: string): void; createDirectory(path: string): void; watchFile?(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher; + watchDirectory?(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher; } } diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index f79564f6f98e1..0bff32958be7f 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -64,6 +64,15 @@ namespace ts.server.typingsInstaller { onRequestCompleted: RequestCompletedAction; } + function isPackageOrBowerJson(fileName: string) { + const base = getBaseFileName(fileName); + return base === "package.json" || base === "bower.json"; + } + + function isInNodeModulesOrBowerComponents(f: string) { + return stringContains(f, "/node_modules/") || stringContains(f, "/bower_components/"); + } + type ProjectWatchers = Map & { isInvoked?: boolean; }; export abstract class TypingsInstaller { @@ -73,6 +82,7 @@ namespace ts.server.typingsInstaller { private readonly projectWatchers = createMap(); private safeList: JsTyping.SafeList | undefined; readonly pendingRunRequests: PendingRequest[] = []; + private readonly toCanonicalFileName: GetCanonicalFileName; private installRunCount = 1; private inFlightRequestCount = 0; @@ -86,6 +96,7 @@ namespace ts.server.typingsInstaller { private readonly typesMapLocation: Path, private readonly throttleLimit: number, protected readonly log = nullLog) { + this.toCanonicalFileName = createGetCanonicalFileName(installTypingHost.useCaseSensitiveFileNames); if (this.log.isEnabled()) { this.log.writeLine(`Global cache location '${globalCachePath}', safe file path '${safeListPath}', types map path ${typesMapLocation}`); } @@ -147,7 +158,7 @@ namespace ts.server.typingsInstaller { } // start watching files - this.watchFiles(req.projectName, discoverTypingsResult.filesToWatch); + this.watchFiles(req.projectName, discoverTypingsResult.filesToWatch, req.projectRootPath); // install typings if (discoverTypingsResult.newTypingNames.length) { @@ -367,7 +378,7 @@ namespace ts.server.typingsInstaller { } } - private watchFiles(projectName: string, files: string[]) { + private watchFiles(projectName: string, files: string[], projectRootPath: Path) { if (!files.length) { // shut down existing watchers this.closeWatchers(projectName); @@ -375,43 +386,109 @@ namespace ts.server.typingsInstaller { } let watchers = this.projectWatchers.get(projectName); + const toRemove = createMap(); if (!watchers) { watchers = createMap(); this.projectWatchers.set(projectName, watchers); } + else { + copyEntries(watchers, toRemove); + } - watchers.isInvoked = false; // handler should be invoked once for the entire set of files since it will trigger full rediscovery of typings + watchers.isInvoked = false; + const isLoggingEnabled = this.log.isEnabled(); - mutateMap( - watchers, - arrayToSet(files), - { - // Watch the missing files - createNewValue: file => { - if (isLoggingEnabled) { - this.log.writeLine(`FileWatcher:: Added:: WatchInfo: ${file}`); - } - const watcher = this.installTypingHost.watchFile(file, (f, eventKind) => { - if (isLoggingEnabled) { - this.log.writeLine(`FileWatcher:: Triggered with ${f} eventKind: ${FileWatcherEventKind[eventKind]}:: WatchInfo: ${file}:: handler is already invoked '${watchers.isInvoked}'`); - } - if (!watchers.isInvoked) { - watchers.isInvoked = true; - this.sendResponse({ projectName, kind: ActionInvalidate }); - } - }, /*pollingInterval*/ 2000); - return isLoggingEnabled ? { - close: () => { - this.log.writeLine(`FileWatcher:: Closed:: WatchInfo: ${file}`); - } - } : watcher; - }, - // Files that are no longer missing (e.g. because they are no longer required) - // should no longer be watched. - onDeleteValue: closeFileWatcher + const createProjectWatcher = (path: string, createWatch: (path: string) => FileWatcher) => { + toRemove.delete(path); + if (watchers.has(path)) { + return; + } + + watchers.set(path, createWatch(path)); + }; + const createProjectFileWatcher = (file: string): FileWatcher => { + if (isLoggingEnabled) { + this.log.writeLine(`FileWatcher:: Added:: WatchInfo: ${file}`); + } + const watcher = this.installTypingHost.watchFile(file, (f, eventKind) => { + if (isLoggingEnabled) { + this.log.writeLine(`FileWatcher:: Triggered with ${f} eventKind: ${FileWatcherEventKind[eventKind]}:: WatchInfo: ${file}:: handler is already invoked '${watchers.isInvoked}'`); + } + if (!watchers.isInvoked) { + watchers.isInvoked = true; + this.sendResponse({ projectName, kind: ActionInvalidate }); + } + }, /*pollingInterval*/ 2000); + + return isLoggingEnabled ? { + close: () => { + this.log.writeLine(`FileWatcher:: Closed:: WatchInfo: ${file}`); + watcher.close(); + } + } : watcher; + }; + const createProjectDirectoryWatcher = (dir: string): FileWatcher => { + if (isLoggingEnabled) { + this.log.writeLine(`DirectoryWatcher:: Added:: WatchInfo: ${dir} recursive`); + } + const watcher = this.installTypingHost.watchDirectory(dir, f => { + if (isLoggingEnabled) { + this.log.writeLine(`DirectoryWatcher:: Triggered with ${f} :: WatchInfo: ${dir} recursive :: handler is already invoked '${watchers.isInvoked}'`); + } + if (watchers.isInvoked) { + return; + } + f = this.toCanonicalFileName(f); + if (isPackageOrBowerJson(f) && f !== this.toCanonicalFileName(combinePaths(this.globalCachePath, "package.json"))) { + watchers.isInvoked = true; + this.sendResponse({ projectName, kind: ActionInvalidate }); + } + }, /*recursive*/ true); + + return isLoggingEnabled ? { + close: () => { + this.log.writeLine(`DirectoryWatcher:: Closed:: WatchInfo: ${dir} recursive`); + watcher.close(); + } + } : watcher; + }; + + // Create watches from list of files + for (const file of files) { + const filePath = this.toCanonicalFileName(file); + if (isPackageOrBowerJson(filePath)) { + // package.json or bower.json exists, watch the file to detect changes and update typings + createProjectWatcher(filePath, createProjectFileWatcher); + continue; } - ); + + // path in projectRoot, watch project root + if (containsPath(projectRootPath, filePath, projectRootPath, !this.installTypingHost.useCaseSensitiveFileNames)) { + createProjectWatcher(projectRootPath, createProjectDirectoryWatcher); + continue; + } + + // path in global cache, watch global cache + if (containsPath(this.globalCachePath, filePath, projectRootPath, !this.installTypingHost.useCaseSensitiveFileNames)) { + createProjectWatcher(this.globalCachePath, createProjectDirectoryWatcher); + continue; + } + + // Get path without node_modules and bower_components + let pathToWatch = getDirectoryPath(filePath); + while (isInNodeModulesOrBowerComponents(pathToWatch)) { + pathToWatch = getDirectoryPath(pathToWatch); + } + + createProjectWatcher(pathToWatch, createProjectDirectoryWatcher); + } + + // Remove unused watches + toRemove.forEach((watch, path) => { + watch.close(); + watchers.delete(path); + }); } private createSetTypings(request: DiscoverTypings, typings: string[]): SetTypings { From 67bb67edf15ace4122253418a339f3c08c68ee3e Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 18 Apr 2018 11:22:02 -0700 Subject: [PATCH 2/3] Cache canonical global cache's package.json path --- src/server/typingsInstaller/typingsInstaller.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 0bff32958be7f..463851e96f45e 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -83,6 +83,7 @@ namespace ts.server.typingsInstaller { private safeList: JsTyping.SafeList | undefined; readonly pendingRunRequests: PendingRequest[] = []; private readonly toCanonicalFileName: GetCanonicalFileName; + private readonly globalCacheCanonicalPackageJsonPath: string; private installRunCount = 1; private inFlightRequestCount = 0; @@ -97,6 +98,7 @@ namespace ts.server.typingsInstaller { private readonly throttleLimit: number, protected readonly log = nullLog) { this.toCanonicalFileName = createGetCanonicalFileName(installTypingHost.useCaseSensitiveFileNames); + this.globalCacheCanonicalPackageJsonPath = combinePaths(this.toCanonicalFileName(globalCachePath), "package.json"); if (this.log.isEnabled()) { this.log.writeLine(`Global cache location '${globalCachePath}', safe file path '${safeListPath}', types map path ${typesMapLocation}`); } @@ -440,7 +442,7 @@ namespace ts.server.typingsInstaller { return; } f = this.toCanonicalFileName(f); - if (isPackageOrBowerJson(f) && f !== this.toCanonicalFileName(combinePaths(this.globalCachePath, "package.json"))) { + if (f !== this.globalCacheCanonicalPackageJsonPath && isPackageOrBowerJson(f)) { watchers.isInvoked = true; this.sendResponse({ projectName, kind: ActionInvalidate }); } From 56b618b9fcb92a1363fcf1cced19062bfd0c3129 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 18 Apr 2018 11:44:28 -0700 Subject: [PATCH 3/3] Use indexOf and substr to exclude node_modules and bowerComponents instead of using loop --- src/server/typingsInstaller/typingsInstaller.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 463851e96f45e..7d8cf02fab08a 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -69,8 +69,13 @@ namespace ts.server.typingsInstaller { return base === "package.json" || base === "bower.json"; } - function isInNodeModulesOrBowerComponents(f: string) { - return stringContains(f, "/node_modules/") || stringContains(f, "/bower_components/"); + function getDirectoryExcludingNodeModulesOrBowerComponents(f: string) { + const indexOfNodeModules = f.indexOf("/node_modules/"); + const indexOfBowerComponents = f.indexOf("/bower_components/"); + const subStrLength = indexOfNodeModules === -1 || indexOfBowerComponents === -1 ? + Math.max(indexOfNodeModules, indexOfBowerComponents) : + Math.min(indexOfNodeModules, indexOfBowerComponents); + return subStrLength === -1 ? f : f.substr(0, subStrLength); } type ProjectWatchers = Map & { isInvoked?: boolean; }; @@ -478,12 +483,7 @@ namespace ts.server.typingsInstaller { } // Get path without node_modules and bower_components - let pathToWatch = getDirectoryPath(filePath); - while (isInNodeModulesOrBowerComponents(pathToWatch)) { - pathToWatch = getDirectoryPath(pathToWatch); - } - - createProjectWatcher(pathToWatch, createProjectDirectoryWatcher); + createProjectWatcher(getDirectoryExcludingNodeModulesOrBowerComponents(getDirectoryPath(filePath)), createProjectDirectoryWatcher); } // Remove unused watches