Skip to content

Commit

Permalink
Merge pull request #19058 from Microsoft/whenWatchesFail
Browse files Browse the repository at this point in the history
Swallow the directory watcher exceptions and ignore them
  • Loading branch information
sheetalkamat authored Oct 11, 2017
2 parents d0168af + 52d7c72 commit a7fa187
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, "-");
}

/**
Expand Down
8 changes: 6 additions & 2 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,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 {
Expand Down Expand Up @@ -1917,8 +1917,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 {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/declarationEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1226,7 +1226,7 @@ namespace ts {
// check if numeric literal is a decimal literal that was originally written with a dot
const text = getLiteralTextOfNode(<LiteralExpression>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
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
15 changes: 6 additions & 9 deletions src/compiler/resolutionCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,17 +329,14 @@ namespace ts {
let dir = getDirectoryPath(getNormalizedAbsolutePath(failedLookupLocation, getCurrentDirectory()));
let dirPath = getDirectoryPath(failedLookupLocationPath);

// If the directory is node_modules use it to watch
if (isNodeModulesDirectory(dirPath)) {
return { dir, dirPath };
// If directory path contains node module, get the most parent node_modules directory for watching
while (stringContains(dirPath, "/node_modules/")) {
dir = getDirectoryPath(dir);
dirPath = getDirectoryPath(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);
}
// If the directory is node_modules use it to watch
if (isNodeModulesDirectory(dirPath)) {
return { dir, dirPath };
}

Expand Down
37 changes: 37 additions & 0 deletions src/harness/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2397,6 +2397,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", () => {
Expand Down
2 changes: 1 addition & 1 deletion src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,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"));
Expand Down
22 changes: 19 additions & 3 deletions src/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -753,10 +753,23 @@ namespace ts.server {
const sys = <ServerHost>ts.sys;
// use watchGuard process on Windows when node version is 4 or later
const useWatchGuard = process.platform === "win32" && getNodeMajorVersion() >= 4;
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(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<boolean>();
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);
Expand Down Expand Up @@ -790,14 +803,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"));
Expand Down
2 changes: 1 addition & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/typingsInstaller/nodeTypingsInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
4 changes: 2 additions & 2 deletions src/services/pathCompletions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit a7fa187

Please sign in to comment.