From b9592d4186ac04ba9690ae6d2f86697ddcf820a9 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 9 Oct 2017 15:59:27 -0700 Subject: [PATCH 1/4] Use the parent most node_modules directory for module resolution failed lookup locations --- src/compiler/resolutionCache.ts | 14 +++---- .../unittests/tsserverProjectSystem.ts | 37 +++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index aecc6989891..25545c0efbc 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -323,19 +323,17 @@ namespace ts { let dir = getDirectoryPath(getNormalizedAbsolutePath(failedLookupLocation, getCurrentDirectory())); let dirPath = getDirectoryPath(failedLookupLocationPath); + // If directory path contains node module, get the most parent node_modules directory for watching + while (dirPath.indexOf("/node_modules/") !== -1) { + dir = getDirectoryPath(dir); + dirPath = getDirectoryPath(dirPath); + } + // If the directory is node_modules use it to watch if (isNodeModulesDirectory(dirPath)) { return { dir, dirPath }; } - // If directory path contains node module, get the node_modules directory for watching - if (dirPath.indexOf("/node_modules/") !== -1) { - while (!isNodeModulesDirectory(dirPath)) { - dir = getDirectoryPath(dir); - dirPath = getDirectoryPath(dirPath); - } - return { dir, dirPath }; - } // Use some ancestor of the root directory if (rootPath !== undefined) { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 06e2beeaaa6..2f875456e06 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -2399,6 +2399,43 @@ namespace ts.projectSystem { checkWatchedDirectories(host, watchedRecursiveDirectories, /*recursive*/ true); }); + + it("Failed lookup locations are uses parent most node_modules directory", () => { + const file1: FileOrFolder = { + path: "/a/b/src/file1.ts", + content: 'import { classc } from "module1"' + }; + const module1: FileOrFolder = { + path: "/a/b/node_modules/module1/index.d.ts", + content: `import { class2 } from "module2"; + export classc { method2a(): class2; }` + }; + const module2: FileOrFolder = { + path: "/a/b/node_modules/module2/index.d.ts", + content: "export class2 { method2() { return 10; } }" + }; + const module3: FileOrFolder = { + path: "/a/b/node_modules/module/node_modules/module3/index.d.ts", + content: "export class3 { method2() { return 10; } }" + }; + const configFile: FileOrFolder = { + path: "/a/b/src/tsconfig.json", + content: JSON.stringify({ files: [file1.path] }) + }; + const files = [file1, module1, module2, module3, configFile, libFile]; + const host = createServerHost(files); + const projectService = createProjectService(host); + projectService.openClientFile(file1.path); + checkNumberOfProjects(projectService, { configuredProjects: 1 }); + const project = projectService.configuredProjects.get(configFile.path); + assert.isDefined(project); + checkProjectActualFiles(project, [file1.path, libFile.path, module1.path, module2.path, configFile.path]); + checkWatchedFiles(host, [libFile.path, module1.path, module2.path, configFile.path]); + checkWatchedDirectories(host, [], /*recursive*/ false); + const watchedRecursiveDirectories = getTypeRootsFromLocation("/a/b/src"); + watchedRecursiveDirectories.push("/a/b/src", "/a/b/node_modules"); + checkWatchedDirectories(host, watchedRecursiveDirectories, /*recursive*/ true); + }); }); describe("Proper errors", () => { From aa22c56282021e19fe546d2f65f650836f826e3b Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 9 Oct 2017 18:03:05 -0700 Subject: [PATCH 2/4] Swallow the directory watcher exceptions --- src/server/server.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/server/server.ts b/src/server/server.ts index 6fab1a3d08a..7917f6fb544 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -753,10 +753,21 @@ namespace ts.server { const sys = ts.sys; // use watchGuard process on Windows when node version is 4 or later const useWatchGuard = process.platform === "win32" && getNodeMajorVersion() >= 4; + const originalWatchDirectory = sys.watchDirectory; + const noopWatcher: FileWatcher = { close: noop }; + function watchDirectorySwallowingException(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher { + try { + return originalWatchDirectory.call(sys, path, callback, recursive); + } + catch (e) { + logger.info(`Exception when creating directory watcher: ${e.message}`); + return noopWatcher; + } + } + if (useWatchGuard) { const currentDrive = extractWatchDirectoryCacheKey(sys.resolvePath(sys.getCurrentDirectory()), /*currentDriveKey*/ undefined); const statusCache = createMap(); - const originalWatchDirectory = sys.watchDirectory; sys.watchDirectory = function (path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher { const cacheKey = extractWatchDirectoryCacheKey(path, currentDrive); let status = cacheKey && statusCache.get(cacheKey); @@ -790,14 +801,17 @@ namespace ts.server { } if (status) { // this drive is safe to use - call real 'watchDirectory' - return originalWatchDirectory.call(sys, path, callback, recursive); + return watchDirectorySwallowingException(path, callback, recursive); } else { // this drive is unsafe - return no-op watcher - return { close() { } }; + return noopWatcher; } }; } + else { + sys.watchDirectory = watchDirectorySwallowingException; + } // Override sys.write because fs.writeSync is not reliable on Node 4 sys.write = (s: string) => writeMessage(new Buffer(s, "utf8")); From e30a66d22f913b824427fd1323dfd82af20c8a76 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 10 Oct 2017 17:08:47 -0700 Subject: [PATCH 3/4] Add utitlity for stringContains --- src/compiler/checker.ts | 2 +- src/compiler/core.ts | 8 ++++++-- src/compiler/declarationEmitter.ts | 2 +- src/compiler/emitter.ts | 2 +- src/compiler/moduleNameResolver.ts | 2 +- src/compiler/resolutionCache.ts | 3 +-- src/server/editorServices.ts | 2 +- src/server/session.ts | 2 +- src/server/typingsInstaller/nodeTypingsInstaller.ts | 2 +- src/services/pathCompletions.ts | 4 ++-- src/services/refactors/extractSymbol.ts | 2 +- src/services/services.ts | 2 +- 12 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d47a77a7440..ab61d91ef15 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -14012,7 +14012,7 @@ namespace ts { */ function isUnhyphenatedJsxName(name: string | __String) { // - is the only character supported in JSX attribute names that isn't valid in JavaScript identifiers - return (name as string).indexOf("-") < 0; + return !stringContains(name as string, "-"); } /** diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 45a4a04b6ab..442a262bbe2 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1663,7 +1663,7 @@ namespace ts { } export function isUrl(path: string) { - return path && !isRootedDiskPath(path) && path.indexOf("://") !== -1; + return path && !isRootedDiskPath(path) && stringContains(path, "://"); } export function pathIsRelative(path: string): boolean { @@ -1932,8 +1932,12 @@ namespace ts { return expectedPos >= 0 && str.indexOf(suffix, expectedPos) === expectedPos; } + export function stringContains(str: string, substring: string): boolean { + return str.indexOf(substring) !== -1; + } + export function hasExtension(fileName: string): boolean { - return getBaseFileName(fileName).indexOf(".") >= 0; + return stringContains(getBaseFileName(fileName), "."); } export function fileExtensionIs(path: string, extension: string): boolean { diff --git a/src/compiler/declarationEmitter.ts b/src/compiler/declarationEmitter.ts index 3de20915029..48b97b048e9 100644 --- a/src/compiler/declarationEmitter.ts +++ b/src/compiler/declarationEmitter.ts @@ -172,7 +172,7 @@ namespace ts { function hasInternalAnnotation(range: CommentRange) { const comment = currentText.substring(range.pos, range.end); - return comment.indexOf("@internal") >= 0; + return stringContains(comment, "@internal"); } function stripInternal(node: Node) { diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 8083b3841c8..da6c0e0fca8 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1226,7 +1226,7 @@ namespace ts { // check if numeric literal is a decimal literal that was originally written with a dot const text = getLiteralTextOfNode(expression); return !expression.numericLiteralFlags - && text.indexOf(tokenToString(SyntaxKind.DotToken)) < 0; + && !stringContains(text, tokenToString(SyntaxKind.DotToken)); } else if (isPropertyAccessExpression(expression) || isElementAccessExpression(expression)) { // check if constant enum value is integer diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 560beb39557..ac83dd41311 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1061,7 +1061,7 @@ namespace ts { export function getPackageNameFromAtTypesDirectory(mangledName: string): string { const withoutAtTypePrefix = removePrefix(mangledName, "@types/"); if (withoutAtTypePrefix !== mangledName) { - return withoutAtTypePrefix.indexOf(mangledScopedPackageSeparator) !== -1 ? + return stringContains(withoutAtTypePrefix, mangledScopedPackageSeparator) ? "@" + withoutAtTypePrefix.replace(mangledScopedPackageSeparator, ts.directorySeparator) : withoutAtTypePrefix; } diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index 25545c0efbc..3f3955b294f 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -324,7 +324,7 @@ namespace ts { let dirPath = getDirectoryPath(failedLookupLocationPath); // If directory path contains node module, get the most parent node_modules directory for watching - while (dirPath.indexOf("/node_modules/") !== -1) { + while (stringContains(dirPath, "/node_modules/")) { dir = getDirectoryPath(dir); dirPath = getDirectoryPath(dirPath); } @@ -334,7 +334,6 @@ namespace ts { return { dir, dirPath }; } - // Use some ancestor of the root directory if (rootPath !== undefined) { while (!isInDirectoryPath(dirPath, rootPath)) { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 2916fb60c57..c683ff0080a 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1205,7 +1205,7 @@ namespace ts.server { projectRootPath?: NormalizedPath) { let searchPath = asNormalizedPath(getDirectoryPath(info.fileName)); - while (!projectRootPath || searchPath.indexOf(projectRootPath) >= 0) { + while (!projectRootPath || stringContains(searchPath, projectRootPath)) { const canonicalSearchPath = normalizedPathToPath(searchPath, this.currentDirectory, this.toCanonicalFileName); const tsconfigFileName = asNormalizedPath(combinePaths(searchPath, "tsconfig.json")); let result = action(tsconfigFileName, combinePaths(canonicalSearchPath, "tsconfig.json")); diff --git a/src/server/session.ts b/src/server/session.ts index 57dac7ec799..df9c77b9005 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1608,7 +1608,7 @@ namespace ts.server { } // No need to analyze lib.d.ts - const fileNamesInProject = fileNames.filter(value => value.indexOf("lib.d.ts") < 0); + const fileNamesInProject = fileNames.filter(value => !stringContains(value, "lib.d.ts")); if (fileNamesInProject.length === 0) { return; } diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 6a31a114d23..f5d9b866376 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -88,7 +88,7 @@ namespace ts.server.typingsInstaller { this.npmPath = npmLocation !== undefined ? npmLocation : getDefaultNPMLocation(process.argv[0]); // If the NPM path contains spaces and isn't wrapped in quotes, do so. - if (this.npmPath.indexOf(" ") !== -1 && this.npmPath[0] !== `"`) { + if (stringContains(this.npmPath, " ") && this.npmPath[0] !== `"`) { this.npmPath = `"${this.npmPath}"`; } if (this.log.isEnabled()) { diff --git a/src/services/pathCompletions.ts b/src/services/pathCompletions.ts index c41ed798b1d..e3bf9deac89 100644 --- a/src/services/pathCompletions.ts +++ b/src/services/pathCompletions.ts @@ -195,7 +195,7 @@ namespace ts.Completions.PathCompletions { const normalizedPrefixDirectory = getDirectoryPath(normalizedPrefix); const normalizedPrefixBase = getBaseFileName(normalizedPrefix); - const fragmentHasPath = fragment.indexOf(directorySeparator) !== -1; + const fragmentHasPath = stringContains(fragment, directorySeparator); // Try and expand the prefix to include any path from the fragment so that we can limit the readDirectory call const expandedPrefixDirectory = fragmentHasPath ? combinePaths(normalizedPrefixDirectory, normalizedPrefixBase + getDirectoryPath(fragment)) : normalizedPrefixDirectory; @@ -235,7 +235,7 @@ namespace ts.Completions.PathCompletions { function enumeratePotentialNonRelativeModules(fragment: string, scriptPath: string, options: CompilerOptions, typeChecker: TypeChecker, host: LanguageServiceHost): string[] { // Check If this is a nested module - const isNestedModule = fragment.indexOf(directorySeparator) !== -1; + const isNestedModule = stringContains(fragment, directorySeparator); const moduleNameFragment = isNestedModule ? fragment.substr(0, fragment.lastIndexOf(directorySeparator)) : undefined; // Get modules that the type checker picked up diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index c16a290a30f..124a1f720bd 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -660,7 +660,7 @@ namespace ts.refactor.extractSymbol { function getUniqueName(baseName: string, fileText: string): string { let nameText = baseName; - for (let i = 1; fileText.indexOf(nameText) !== -1; i++) { + for (let i = 1; stringContains(fileText, nameText); i++) { nameText = `${baseName}_${i}`; } return nameText; diff --git a/src/services/services.ts b/src/services/services.ts index bc733a4e2fe..6bdc96d8b4d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1952,7 +1952,7 @@ namespace ts { function isNodeModulesFile(path: string): boolean { const node_modulesFolderName = "/node_modules/"; - return path.indexOf(node_modulesFolderName) !== -1; + return stringContains(path, node_modulesFolderName); } } From 52d7c7278d7b6d3995777980e73466d630b53837 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 10 Oct 2017 17:14:32 -0700 Subject: [PATCH 4/4] Add comment about swallowing exception --- src/server/server.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/server/server.ts b/src/server/server.ts index 7917f6fb544..f24251ee6ac 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -753,11 +753,13 @@ namespace ts.server { const sys = ts.sys; // use watchGuard process on Windows when node version is 4 or later const useWatchGuard = process.platform === "win32" && getNodeMajorVersion() >= 4; - const originalWatchDirectory = sys.watchDirectory; + const originalWatchDirectory: ServerHost["watchDirectory"] = sys.watchDirectory.bind(sys); const noopWatcher: FileWatcher = { close: noop }; + // This is the function that catches the exceptions when watching directory, and yet lets project service continue to function + // Eg. on linux the number of watches are limited and one could easily exhaust watches and the exception ENOSPC is thrown when creating watcher at that point function watchDirectorySwallowingException(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher { try { - return originalWatchDirectory.call(sys, path, callback, recursive); + return originalWatchDirectory(path, callback, recursive); } catch (e) { logger.info(`Exception when creating directory watcher: ${e.message}`);