Skip to content

Commit

Permalink
Do not watch tsconfig files from folders that we canot watch
Browse files Browse the repository at this point in the history
Fixes #30818
  • Loading branch information
sheetalkamat committed Jun 7, 2019
1 parent 737cb45 commit a6c72a0
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 76 deletions.
70 changes: 35 additions & 35 deletions src/compiler/resolutionCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,41 @@ namespace ts {
return some(ignoredPaths, searchPath => stringContains(path, searchPath));
}

/**
* Filter out paths like
* "/", "/user", "/user/username", "/user/username/folderAtRoot",
* "c:/", "c:/users", "c:/users/username", "c:/users/username/folderAtRoot", "c:/folderAtRoot"
* @param dirPath
*/
export function canWatchDirectory(dirPath: Path) {
const rootLength = getRootLength(dirPath);
if (dirPath.length === rootLength) {
// Ignore "/", "c:/"
return false;
}

const nextDirectorySeparator = dirPath.indexOf(directorySeparator, rootLength);
if (nextDirectorySeparator === -1) {
// ignore "/user", "c:/users" or "c:/folderAtRoot"
return false;
}

if (dirPath.charCodeAt(0) !== CharacterCodes.slash &&
dirPath.substr(rootLength, nextDirectorySeparator).search(/users/i) === -1) {
// Paths like c:/folderAtRoot/subFolder are allowed
return true;
}

for (let searchIndex = nextDirectorySeparator + 1, searchLevels = 2; searchLevels > 0; searchLevels--) {
searchIndex = dirPath.indexOf(directorySeparator, searchIndex) + 1;
if (searchIndex === 0) {
// Folder isnt at expected minimun levels
return false;
}
}
return true;
}

export const maxNumberOfFilesToIterateForInvalidation = 256;

type GetResolutionWithResolvedFileName<T extends ResolutionWithFailedLookupLocations = ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName = ResolutionWithResolvedFileName> =
Expand Down Expand Up @@ -373,41 +408,6 @@ namespace ts {
return endsWith(dirPath, "/node_modules/@types");
}

/**
* Filter out paths like
* "/", "/user", "/user/username", "/user/username/folderAtRoot",
* "c:/", "c:/users", "c:/users/username", "c:/users/username/folderAtRoot", "c:/folderAtRoot"
* @param dirPath
*/
function canWatchDirectory(dirPath: Path) {
const rootLength = getRootLength(dirPath);
if (dirPath.length === rootLength) {
// Ignore "/", "c:/"
return false;
}

const nextDirectorySeparator = dirPath.indexOf(directorySeparator, rootLength);
if (nextDirectorySeparator === -1) {
// ignore "/user", "c:/users" or "c:/folderAtRoot"
return false;
}

if (dirPath.charCodeAt(0) !== CharacterCodes.slash &&
dirPath.substr(rootLength, nextDirectorySeparator).search(/users/i) === -1) {
// Paths like c:/folderAtRoot/subFolder are allowed
return true;
}

for (let searchIndex = nextDirectorySeparator + 1, searchLevels = 2; searchLevels > 0; searchLevels--) {
searchIndex = dirPath.indexOf(directorySeparator, searchIndex) + 1;
if (searchIndex === 0) {
// Folder isnt at expected minimun levels
return false;
}
}
return true;
}

function getDirectoryToWatchFailedLookupLocation(failedLookupLocation: string, failedLookupLocationPath: Path): DirectoryOfFailedLookupWatch | undefined {
if (isInDirectoryPath(rootPath, failedLookupLocationPath)) {
// Ensure failed look up is normalized path
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ namespace ts {
return ExitStatus.Success;
}

const noopFileWatcher: FileWatcher = { close: noop };
export const noopFileWatcher: FileWatcher = { close: noop };

export function createWatchHost(system = sys, reportWatchStatus?: WatchStatusReporter): WatchHost {
const onWatchStatusChange = reportWatchStatus || createWatchStatusReporter(system);
Expand Down
23 changes: 15 additions & 8 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,11 @@ namespace ts.server {

const watches: WatchType[] = [];
if (configFileExistenceInfo.configFileWatcherForRootOfInferredProject) {
watches.push(WatchType.ConfigFileForInferredRoot);
watches.push(
configFileExistenceInfo.configFileWatcherForRootOfInferredProject === noopFileWatcher ?
WatchType.NoopConfigFileForInferredRoot :
WatchType.ConfigFileForInferredRoot
);
}
if (this.configuredProjects.has(canonicalConfigFilePath)) {
watches.push(WatchType.ConfigFile);
Expand All @@ -1349,13 +1353,16 @@ namespace ts.server {
canonicalConfigFilePath: string,
configFileExistenceInfo: ConfigFileExistenceInfo
) {
configFileExistenceInfo.configFileWatcherForRootOfInferredProject = this.watchFactory.watchFile(
this.host,
configFileName,
(_filename, eventKind) => this.onConfigFileChangeForOpenScriptInfo(configFileName, eventKind),
PollingInterval.High,
WatchType.ConfigFileForInferredRoot
);
configFileExistenceInfo.configFileWatcherForRootOfInferredProject =
canWatchDirectory(getDirectoryPath(canonicalConfigFilePath) as Path) ?
this.watchFactory.watchFile(
this.host,
configFileName,
(_filename, eventKind) => this.onConfigFileChangeForOpenScriptInfo(configFileName, eventKind),
PollingInterval.High,
WatchType.ConfigFileForInferredRoot
) :
noopFileWatcher;
this.logConfigFileWatchUpdate(configFileName, canonicalConfigFilePath, configFileExistenceInfo, ConfigFileWatcherStatus.UpdatedCallback);
}

Expand Down
1 change: 1 addition & 0 deletions src/server/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,5 +226,6 @@ namespace ts {
ConfigFileForInferredRoot = "Config file for the inferred project root",
NodeModulesForClosedScriptInfo = "node_modules for closed script infos in them",
MissingSourceMapFile = "Missing source map file",
NoopConfigFileForInferredRoot = "Noop Config file for the inferred project root",
}
}
26 changes: 21 additions & 5 deletions src/testRunner/unittests/tsserver/configFileSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,7 @@ namespace ts.projectSystem {
const project = projectService.inferredProjects[0];
assert.isDefined(project);

const filesToWatch = [libFile.path];
forEachAncestorDirectory(dirOfFile, ancestor => {
filesToWatch.push(combinePaths(ancestor, "tsconfig.json"));
filesToWatch.push(combinePaths(ancestor, "jsconfig.json"));
});
const filesToWatch = [libFile.path, ...getConfigFilesToWatch(dirOfFile)];

checkProjectActualFiles(project, [file.path, libFile.path]);
checkWatchedFiles(host, filesToWatch);
Expand Down Expand Up @@ -170,5 +166,25 @@ namespace ts.projectSystem {
verifyInferredProject(host, projectService);
});
});

describe("should not search and watch config files from directories that cannot be watched", () => {
const root = "/root/teams/VSCode68/Shared Documents/General/jt-ts-test-workspace";
function verifyConfigFileWatch(projectRootPath: string | undefined) {
const path = `${root}/x.js`;
const host = createServerHost([libFile, { path, content: "const x = 10" }], { useCaseSensitiveFileNames: true });
const service = createProjectService(host);
service.openClientFile(path, /*fileContent*/ undefined, /*scriptKind*/ undefined, projectRootPath);
checkNumberOfProjects(service, { inferredProjects: 1 });
checkProjectActualFiles(service.inferredProjects[0], [path, libFile.path]);
checkWatchedFilesDetailed(host, [libFile.path, ...getConfigFilesToWatch(root)], 1);
}

it("when projectRootPath is not present", () => {
verifyConfigFileWatch(/*projectRootPath*/ undefined);
});
it("when projectRootPath is present but file is not from project root", () => {
verifyConfigFileWatch("/a/b");
});
});
});
}
24 changes: 17 additions & 7 deletions src/testRunner/unittests/tsserver/configuredProjects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,22 @@ namespace ts.projectSystem {
});

it("add and then remove a config file in a folder with loose files", () => {
const projectRoot = "/user/username/projects/project";
const configFile: File = {
path: "/a/b/tsconfig.json",
path: `${projectRoot}/tsconfig.json`,
content: `{
"files": ["commonFile1.ts"]
}`
};
const commonFile1: File = {
path: `${projectRoot}/commonFile1.ts`,
content: "let x = 1"
};
const commonFile2: File = {
path: `${projectRoot}/commonFile2.ts`,
content: "let y = 1"
};

const filesWithoutConfig = [libFile, commonFile1, commonFile2];
const host = createServerHost(filesWithoutConfig);

Expand All @@ -100,8 +110,7 @@ namespace ts.projectSystem {
checkProjectActualFiles(projectService.inferredProjects[0], [commonFile1.path, libFile.path]);
checkProjectActualFiles(projectService.inferredProjects[1], [commonFile2.path, libFile.path]);

const configFileLocations = ["/", "/a/", "/a/b/"];
const watchedFiles = flatMap(configFileLocations, location => [location + "tsconfig.json", location + "jsconfig.json"]).concat(libFile.path);
const watchedFiles = getConfigFilesToWatch(projectRoot).concat(libFile.path);
checkWatchedFiles(host, watchedFiles);

// Add a tsconfig file
Expand Down Expand Up @@ -431,20 +440,21 @@ namespace ts.projectSystem {
});

it("open file become a part of configured project if it is referenced from root file", () => {
const projectRoot = "/user/username/projects/project";
const file1 = {
path: "/a/b/f1.ts",
path: `${projectRoot}/a/b/f1.ts`,
content: "export let x = 5"
};
const file2 = {
path: "/a/c/f2.ts",
path: `${projectRoot}/a/c/f2.ts`,
content: `import {x} from "../b/f1"`
};
const file3 = {
path: "/a/c/f3.ts",
path: `${projectRoot}/a/c/f3.ts`,
content: "export let y = 1"
};
const configFile = {
path: "/a/c/tsconfig.json",
path: `${projectRoot}/a/c/tsconfig.json`,
content: JSON.stringify({ compilerOptions: {}, files: ["f2.ts", "f3.ts"] })
};

Expand Down
7 changes: 7 additions & 0 deletions src/testRunner/unittests/tsserver/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,13 @@ namespace ts.projectSystem {
return getRootsToWatchWithAncestorDirectory(currentDirectory, nodeModulesAtTypes);
}

export function getConfigFilesToWatch(folder: string) {
return [
...getRootsToWatchWithAncestorDirectory(folder, "tsconfig.json"),
...getRootsToWatchWithAncestorDirectory(folder, "jsconfig.json")
];
}

export function checkOpenFiles(projectService: server.ProjectService, expectedFiles: File[]) {
checkArray("Open files", arrayFrom(projectService.openFiles.keys(), path => projectService.getScriptInfoForPath(path as Path)!.fileName), expectedFiles.map(file => file.path));
}
Expand Down
22 changes: 11 additions & 11 deletions src/testRunner/unittests/tsserver/inferredProjects.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
namespace ts.projectSystem {
describe("unittests:: tsserver:: Inferred projects", () => {
it("create inferred project", () => {
const projectRoot = "/user/username/projects/project";
const appFile: File = {
path: "/a/b/c/app.ts",
path: `${projectRoot}/app.ts`,
content: `
import {f} from "./module"
console.log(f)
`
};

const moduleFile: File = {
path: "/a/b/c/module.d.ts",
path: `${projectRoot}/module.d.ts`,
content: `export let x: number`
};
const host = createServerHost([appFile, moduleFile, libFile]);
Expand All @@ -24,20 +25,19 @@ namespace ts.projectSystem {
const project = projectService.inferredProjects[0];

checkArray("inferred project", project.getFileNames(), [appFile.path, libFile.path, moduleFile.path]);
const configFileLocations = ["/a/b/c/", "/a/b/", "/a/", "/"];
const configFiles = flatMap(configFileLocations, location => [location + "tsconfig.json", location + "jsconfig.json"]);
checkWatchedFiles(host, configFiles.concat(libFile.path, moduleFile.path));
checkWatchedDirectories(host, ["/a/b/c"], /*recursive*/ false);
checkWatchedDirectories(host, [combinePaths(getDirectoryPath(appFile.path), nodeModulesAtTypes)], /*recursive*/ true);
checkWatchedFiles(host, getConfigFilesToWatch(projectRoot).concat(libFile.path, moduleFile.path));
checkWatchedDirectories(host, [projectRoot], /*recursive*/ false);
checkWatchedDirectories(host, [combinePaths(projectRoot, nodeModulesAtTypes)], /*recursive*/ true);
});

it("should use only one inferred project if 'useOneInferredProject' is set", () => {
const projectRoot = "/user/username/projects/project";
const file1 = {
path: "/a/b/main.ts",
path: `${projectRoot}/a/b/main.ts`,
content: "let x =1;"
};
const configFile: File = {
path: "/a/b/tsconfig.json",
path: `${projectRoot}/a/b/tsconfig.json`,
content: `{
"compilerOptions": {
"target": "es6"
Expand All @@ -46,12 +46,12 @@ namespace ts.projectSystem {
}`
};
const file2 = {
path: "/a/c/main.ts",
path: `${projectRoot}/a/c/main.ts`,
content: "let x =1;"
};

const file3 = {
path: "/a/d/main.ts",
path: `${projectRoot}/a/d/main.ts`,
content: "let x =1;"
};

Expand Down
6 changes: 2 additions & 4 deletions src/testRunner/unittests/tsserver/resolutionCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -818,10 +818,8 @@ namespace ts.projectSystem {
verifyTrace(resolutionTrace, expectedTrace);

const currentDirectory = getDirectoryPath(file1.path);
const watchedFiles = mapDefined(files, f => f === file1 || f.path.indexOf("/node_modules/") !== -1 ? undefined : f.path);
forEachAncestorDirectory(currentDirectory, d => {
watchedFiles.push(combinePaths(d, "tsconfig.json"), combinePaths(d, "jsconfig.json"));
});
const watchedFiles = mapDefined(files, f => f === file1 || f.path.indexOf("/node_modules/") !== -1 ? undefined : f.path)
.concat(getConfigFilesToWatch(`${projectLocation}/product/src`));
const watchedRecursiveDirectories = getTypeRootsFromLocation(currentDirectory).concat([
`${currentDirectory}/node_modules`, `${currentDirectory}/feature`, `${projectLocation}/product/${nodeModules}`,
`${projectLocation}/${nodeModules}`, `${projectLocation}/product/test/${nodeModules}`,
Expand Down
18 changes: 13 additions & 5 deletions src/testRunner/unittests/tsserver/typingsInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -986,14 +986,15 @@ namespace ts.projectSystem {
});

it("should redo resolution that resolved to '.js' file after typings are installed", () => {
const projects = `/user/username/projects`;
const file: TestFSWithWatch.File = {
path: "/a/b/app.js",
path: `${projects}/a/b/app.js`,
content: `
import * as commander from "commander";`
};
const cachePath = "/a/cache";
const cachePath = `${projects}/a/cache`;
const commanderJS: TestFSWithWatch.File = {
path: "/node_modules/commander/index.js",
path: `${projects}/node_modules/commander/index.js`,
content: "module.exports = 0",
};

Expand All @@ -1013,10 +1014,17 @@ namespace ts.projectSystem {
const service = createProjectService(host, { typingsInstaller: installer });
service.openClientFile(file.path);

checkWatchedFiles(host, [...flatMap(["/a/b", "/a", ""], x => [x + "/tsconfig.json", x + "/jsconfig.json"]), "/a/lib/lib.d.ts"]);
checkWatchedFiles(host, [...getConfigFilesToWatch(getDirectoryPath(file.path)), "/a/lib/lib.d.ts"]);
checkWatchedDirectories(host, [], /*recursive*/ false);
// Does not include cachePath because that is handled by typingsInstaller
checkWatchedDirectories(host, ["/node_modules", "/a/b/node_modules", "/a/b/node_modules/@types", "/a/b/bower_components"], /*recursive*/ true);
checkWatchedDirectories(host, [
`${projects}/node_modules`,
`${projects}/a/node_modules`,
`${projects}/a/b/node_modules`,
`${projects}/a/node_modules/@types`,
`${projects}/a/b/node_modules/@types`,
`${projects}/a/b/bower_components`
], /*recursive*/ true);

service.checkNumberOfProjects({ inferredProjects: 1 });
checkProjectActualFiles(service.inferredProjects[0], [file.path, commanderJS.path]);
Expand Down

0 comments on commit a6c72a0

Please sign in to comment.