Skip to content

Commit a6c72a0

Browse files
sheetalkamatsheetalkamat
committed
Do not watch tsconfig files from folders that we canot watch
Fixes microsoft#30818
1 parent 737cb45 commit a6c72a0

File tree

10 files changed

+123
-76
lines changed

10 files changed

+123
-76
lines changed

‎src/compiler/resolutionCache.ts‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,41 @@ namespace ts {
7676
return some(ignoredPaths, searchPath => stringContains(path, searchPath));
7777
}
7878

79+
/**
80+
* Filter out paths like
81+
* "/", "/user", "/user/username", "/user/username/folderAtRoot",
82+
* "c:/", "c:/users", "c:/users/username", "c:/users/username/folderAtRoot", "c:/folderAtRoot"
83+
* @param dirPath
84+
*/
85+
export function canWatchDirectory(dirPath: Path) {
86+
const rootLength = getRootLength(dirPath);
87+
if (dirPath.length === rootLength) {
88+
// Ignore "/", "c:/"
89+
return false;
90+
}
91+
92+
const nextDirectorySeparator = dirPath.indexOf(directorySeparator, rootLength);
93+
if (nextDirectorySeparator === -1) {
94+
// ignore "/user", "c:/users" or "c:/folderAtRoot"
95+
return false;
96+
}
97+
98+
if (dirPath.charCodeAt(0) !== CharacterCodes.slash &&
99+
dirPath.substr(rootLength, nextDirectorySeparator).search(/users/i) === -1) {
100+
// Paths like c:/folderAtRoot/subFolder are allowed
101+
return true;
102+
}
103+
104+
for (let searchIndex = nextDirectorySeparator + 1, searchLevels = 2; searchLevels > 0; searchLevels--) {
105+
searchIndex = dirPath.indexOf(directorySeparator, searchIndex) + 1;
106+
if (searchIndex === 0) {
107+
// Folder isnt at expected minimun levels
108+
return false;
109+
}
110+
}
111+
return true;
112+
}
113+
79114
export const maxNumberOfFilesToIterateForInvalidation = 256;
80115

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

376-
/**
377-
* Filter out paths like
378-
* "/", "/user", "/user/username", "/user/username/folderAtRoot",
379-
* "c:/", "c:/users", "c:/users/username", "c:/users/username/folderAtRoot", "c:/folderAtRoot"
380-
* @param dirPath
381-
*/
382-
function canWatchDirectory(dirPath: Path) {
383-
const rootLength = getRootLength(dirPath);
384-
if (dirPath.length === rootLength) {
385-
// Ignore "/", "c:/"
386-
return false;
387-
}
388-
389-
const nextDirectorySeparator = dirPath.indexOf(directorySeparator, rootLength);
390-
if (nextDirectorySeparator === -1) {
391-
// ignore "/user", "c:/users" or "c:/folderAtRoot"
392-
return false;
393-
}
394-
395-
if (dirPath.charCodeAt(0) !== CharacterCodes.slash &&
396-
dirPath.substr(rootLength, nextDirectorySeparator).search(/users/i) === -1) {
397-
// Paths like c:/folderAtRoot/subFolder are allowed
398-
return true;
399-
}
400-
401-
for (let searchIndex = nextDirectorySeparator + 1, searchLevels = 2; searchLevels > 0; searchLevels--) {
402-
searchIndex = dirPath.indexOf(directorySeparator, searchIndex) + 1;
403-
if (searchIndex === 0) {
404-
// Folder isnt at expected minimun levels
405-
return false;
406-
}
407-
}
408-
return true;
409-
}
410-
411411
function getDirectoryToWatchFailedLookupLocation(failedLookupLocation: string, failedLookupLocationPath: Path): DirectoryOfFailedLookupWatch | undefined {
412412
if (isInDirectoryPath(rootPath, failedLookupLocationPath)) {
413413
// Ensure failed look up is normalized path

‎src/compiler/watch.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ namespace ts {
214214
return ExitStatus.Success;
215215
}
216216

217-
const noopFileWatcher: FileWatcher = { close: noop };
217+
export const noopFileWatcher: FileWatcher = { close: noop };
218218

219219
export function createWatchHost(system = sys, reportWatchStatus?: WatchStatusReporter): WatchHost {
220220
const onWatchStatusChange = reportWatchStatus || createWatchStatusReporter(system);

‎src/server/editorServices.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,7 +1333,11 @@ namespace ts.server {
13331333

13341334
const watches: WatchType[] = [];
13351335
if (configFileExistenceInfo.configFileWatcherForRootOfInferredProject) {
1336-
watches.push(WatchType.ConfigFileForInferredRoot);
1336+
watches.push(
1337+
configFileExistenceInfo.configFileWatcherForRootOfInferredProject === noopFileWatcher ?
1338+
WatchType.NoopConfigFileForInferredRoot :
1339+
WatchType.ConfigFileForInferredRoot
1340+
);
13371341
}
13381342
if (this.configuredProjects.has(canonicalConfigFilePath)) {
13391343
watches.push(WatchType.ConfigFile);
@@ -1349,13 +1353,16 @@ namespace ts.server {
13491353
canonicalConfigFilePath: string,
13501354
configFileExistenceInfo: ConfigFileExistenceInfo
13511355
) {
1352-
configFileExistenceInfo.configFileWatcherForRootOfInferredProject = this.watchFactory.watchFile(
1353-
this.host,
1354-
configFileName,
1355-
(_filename, eventKind) => this.onConfigFileChangeForOpenScriptInfo(configFileName, eventKind),
1356-
PollingInterval.High,
1357-
WatchType.ConfigFileForInferredRoot
1358-
);
1356+
configFileExistenceInfo.configFileWatcherForRootOfInferredProject =
1357+
canWatchDirectory(getDirectoryPath(canonicalConfigFilePath) as Path) ?
1358+
this.watchFactory.watchFile(
1359+
this.host,
1360+
configFileName,
1361+
(_filename, eventKind) => this.onConfigFileChangeForOpenScriptInfo(configFileName, eventKind),
1362+
PollingInterval.High,
1363+
WatchType.ConfigFileForInferredRoot
1364+
) :
1365+
noopFileWatcher;
13591366
this.logConfigFileWatchUpdate(configFileName, canonicalConfigFilePath, configFileExistenceInfo, ConfigFileWatcherStatus.UpdatedCallback);
13601367
}
13611368

‎src/server/utilities.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,5 +226,6 @@ namespace ts {
226226
ConfigFileForInferredRoot = "Config file for the inferred project root",
227227
NodeModulesForClosedScriptInfo = "node_modules for closed script infos in them",
228228
MissingSourceMapFile = "Missing source map file",
229+
NoopConfigFileForInferredRoot = "Noop Config file for the inferred project root",
229230
}
230231
}

‎src/testRunner/unittests/tsserver/configFileSearch.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,7 @@ namespace ts.projectSystem {
132132
const project = projectService.inferredProjects[0];
133133
assert.isDefined(project);
134134

135-
const filesToWatch = [libFile.path];
136-
forEachAncestorDirectory(dirOfFile, ancestor => {
137-
filesToWatch.push(combinePaths(ancestor, "tsconfig.json"));
138-
filesToWatch.push(combinePaths(ancestor, "jsconfig.json"));
139-
});
135+
const filesToWatch = [libFile.path, ...getConfigFilesToWatch(dirOfFile)];
140136

141137
checkProjectActualFiles(project, [file.path, libFile.path]);
142138
checkWatchedFiles(host, filesToWatch);
@@ -170,5 +166,25 @@ namespace ts.projectSystem {
170166
verifyInferredProject(host, projectService);
171167
});
172168
});
169+
170+
describe("should not search and watch config files from directories that cannot be watched", () => {
171+
const root = "/root/teams/VSCode68/Shared Documents/General/jt-ts-test-workspace";
172+
function verifyConfigFileWatch(projectRootPath: string | undefined) {
173+
const path = `${root}/x.js`;
174+
const host = createServerHost([libFile, { path, content: "const x = 10" }], { useCaseSensitiveFileNames: true });
175+
const service = createProjectService(host);
176+
service.openClientFile(path, /*fileContent*/ undefined, /*scriptKind*/ undefined, projectRootPath);
177+
checkNumberOfProjects(service, { inferredProjects: 1 });
178+
checkProjectActualFiles(service.inferredProjects[0], [path, libFile.path]);
179+
checkWatchedFilesDetailed(host, [libFile.path, ...getConfigFilesToWatch(root)], 1);
180+
}
181+
182+
it("when projectRootPath is not present", () => {
183+
verifyConfigFileWatch(/*projectRootPath*/ undefined);
184+
});
185+
it("when projectRootPath is present but file is not from project root", () => {
186+
verifyConfigFileWatch("/a/b");
187+
});
188+
});
173189
});
174190
}

‎src/testRunner/unittests/tsserver/configuredProjects.ts‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,22 @@ namespace ts.projectSystem {
8282
});
8383

8484
it("add and then remove a config file in a folder with loose files", () => {
85+
const projectRoot = "/user/username/projects/project";
8586
const configFile: File = {
86-
path: "/a/b/tsconfig.json",
87+
path: `${projectRoot}/tsconfig.json`,
8788
content: `{
8889
"files": ["commonFile1.ts"]
8990
}`
9091
};
92+
const commonFile1: File = {
93+
path: `${projectRoot}/commonFile1.ts`,
94+
content: "let x = 1"
95+
};
96+
const commonFile2: File = {
97+
path: `${projectRoot}/commonFile2.ts`,
98+
content: "let y = 1"
99+
};
100+
91101
const filesWithoutConfig = [libFile, commonFile1, commonFile2];
92102
const host = createServerHost(filesWithoutConfig);
93103

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

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

107116
// Add a tsconfig file
@@ -431,20 +440,21 @@ namespace ts.projectSystem {
431440
});
432441

433442
it("open file become a part of configured project if it is referenced from root file", () => {
443+
const projectRoot = "/user/username/projects/project";
434444
const file1 = {
435-
path: "/a/b/f1.ts",
445+
path: `${projectRoot}/a/b/f1.ts`,
436446
content: "export let x = 5"
437447
};
438448
const file2 = {
439-
path: "/a/c/f2.ts",
449+
path: `${projectRoot}/a/c/f2.ts`,
440450
content: `import {x} from "../b/f1"`
441451
};
442452
const file3 = {
443-
path: "/a/c/f3.ts",
453+
path: `${projectRoot}/a/c/f3.ts`,
444454
content: "export let y = 1"
445455
};
446456
const configFile = {
447-
path: "/a/c/tsconfig.json",
457+
path: `${projectRoot}/a/c/tsconfig.json`,
448458
content: JSON.stringify({ compilerOptions: {}, files: ["f2.ts", "f3.ts"] })
449459
};
450460

‎src/testRunner/unittests/tsserver/helpers.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,13 @@ namespace ts.projectSystem {
477477
return getRootsToWatchWithAncestorDirectory(currentDirectory, nodeModulesAtTypes);
478478
}
479479

480+
export function getConfigFilesToWatch(folder: string) {
481+
return [
482+
...getRootsToWatchWithAncestorDirectory(folder, "tsconfig.json"),
483+
...getRootsToWatchWithAncestorDirectory(folder, "jsconfig.json")
484+
];
485+
}
486+
480487
export function checkOpenFiles(projectService: server.ProjectService, expectedFiles: File[]) {
481488
checkArray("Open files", arrayFrom(projectService.openFiles.keys(), path => projectService.getScriptInfoForPath(path as Path)!.fileName), expectedFiles.map(file => file.path));
482489
}

‎src/testRunner/unittests/tsserver/inferredProjects.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
namespace ts.projectSystem {
22
describe("unittests:: tsserver:: Inferred projects", () => {
33
it("create inferred project", () => {
4+
const projectRoot = "/user/username/projects/project";
45
const appFile: File = {
5-
path: "/a/b/c/app.ts",
6+
path: `${projectRoot}/app.ts`,
67
content: `
78
import {f} from "./module"
89
console.log(f)
910
`
1011
};
1112

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

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

3433
it("should use only one inferred project if 'useOneInferredProject' is set", () => {
34+
const projectRoot = "/user/username/projects/project";
3535
const file1 = {
36-
path: "/a/b/main.ts",
36+
path: `${projectRoot}/a/b/main.ts`,
3737
content: "let x =1;"
3838
};
3939
const configFile: File = {
40-
path: "/a/b/tsconfig.json",
40+
path: `${projectRoot}/a/b/tsconfig.json`,
4141
content: `{
4242
"compilerOptions": {
4343
"target": "es6"
@@ -46,12 +46,12 @@ namespace ts.projectSystem {
4646
}`
4747
};
4848
const file2 = {
49-
path: "/a/c/main.ts",
49+
path: `${projectRoot}/a/c/main.ts`,
5050
content: "let x =1;"
5151
};
5252

5353
const file3 = {
54-
path: "/a/d/main.ts",
54+
path: `${projectRoot}/a/d/main.ts`,
5555
content: "let x =1;"
5656
};
5757

‎src/testRunner/unittests/tsserver/resolutionCache.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -818,10 +818,8 @@ namespace ts.projectSystem {
818818
verifyTrace(resolutionTrace, expectedTrace);
819819

820820
const currentDirectory = getDirectoryPath(file1.path);
821-
const watchedFiles = mapDefined(files, f => f === file1 || f.path.indexOf("/node_modules/") !== -1 ? undefined : f.path);
822-
forEachAncestorDirectory(currentDirectory, d => {
823-
watchedFiles.push(combinePaths(d, "tsconfig.json"), combinePaths(d, "jsconfig.json"));
824-
});
821+
const watchedFiles = mapDefined(files, f => f === file1 || f.path.indexOf("/node_modules/") !== -1 ? undefined : f.path)
822+
.concat(getConfigFilesToWatch(`${projectLocation}/product/src`));
825823
const watchedRecursiveDirectories = getTypeRootsFromLocation(currentDirectory).concat([
826824
`${currentDirectory}/node_modules`, `${currentDirectory}/feature`, `${projectLocation}/product/${nodeModules}`,
827825
`${projectLocation}/${nodeModules}`, `${projectLocation}/product/test/${nodeModules}`,

‎src/testRunner/unittests/tsserver/typingsInstaller.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -986,14 +986,15 @@ namespace ts.projectSystem {
986986
});
987987

988988
it("should redo resolution that resolved to '.js' file after typings are installed", () => {
989+
const projects = `/user/username/projects`;
989990
const file: TestFSWithWatch.File = {
990-
path: "/a/b/app.js",
991+
path: `${projects}/a/b/app.js`,
991992
content: `
992993
import * as commander from "commander";`
993994
};
994-
const cachePath = "/a/cache";
995+
const cachePath = `${projects}/a/cache`;
995996
const commanderJS: TestFSWithWatch.File = {
996-
path: "/node_modules/commander/index.js",
997+
path: `${projects}/node_modules/commander/index.js`,
997998
content: "module.exports = 0",
998999
};
9991000

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

1016-
checkWatchedFiles(host, [...flatMap(["/a/b", "/a", ""], x => [x + "/tsconfig.json", x + "/jsconfig.json"]), "/a/lib/lib.d.ts"]);
1017+
checkWatchedFiles(host, [...getConfigFilesToWatch(getDirectoryPath(file.path)), "/a/lib/lib.d.ts"]);
10171018
checkWatchedDirectories(host, [], /*recursive*/ false);
10181019
// Does not include cachePath because that is handled by typingsInstaller
1019-
checkWatchedDirectories(host, ["/node_modules", "/a/b/node_modules", "/a/b/node_modules/@types", "/a/b/bower_components"], /*recursive*/ true);
1020+
checkWatchedDirectories(host, [
1021+
`${projects}/node_modules`,
1022+
`${projects}/a/node_modules`,
1023+
`${projects}/a/b/node_modules`,
1024+
`${projects}/a/node_modules/@types`,
1025+
`${projects}/a/b/node_modules/@types`,
1026+
`${projects}/a/b/bower_components`
1027+
], /*recursive*/ true);
10201028

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

0 commit comments

Comments
 (0)