Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a6c72a0

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

10 files changed

+123-76Lines changed: 123 additions & 76 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/compiler/resolutionCache.ts‎

Copy file name to clipboardExpand all lines: src/compiler/resolutionCache.ts
+35-35Lines 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
Collapse file

‎src/compiler/watch.ts‎

Copy file name to clipboardExpand all lines: src/compiler/watch.ts
+1-1Lines 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);
Collapse file

‎src/server/editorServices.ts‎

Copy file name to clipboardExpand all lines: src/server/editorServices.ts
+15-8Lines 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

Collapse file

‎src/server/utilities.ts‎

Copy file name to clipboardExpand all lines: src/server/utilities.ts
+1Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: src/testRunner/unittests/tsserver/configFileSearch.ts
+21-5Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: src/testRunner/unittests/tsserver/configuredProjects.ts
+17-7Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: src/testRunner/unittests/tsserver/helpers.ts
+7Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: src/testRunner/unittests/tsserver/inferredProjects.ts
+11-11Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: src/testRunner/unittests/tsserver/resolutionCache.ts
+2-4Lines 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}`,
Collapse file

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

Copy file name to clipboardExpand all lines: src/testRunner/unittests/tsserver/typingsInstaller.ts
+13-5Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.