Skip to content

Commit b071a86

Browse files
sheetalkamatsheetalkamat
committed
More work on feedback from PR
1 parent f723beb commit b071a86

3 files changed

Lines changed: 51 additions & 50 deletions

File tree

‎src/server/editorServices.ts‎

Lines changed: 41 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,8 +1289,7 @@ namespace ts.server {
12891289
}
12901290

12911291
const configFilePath = project instanceof server.ConfiguredProject && project.getConfigFilePath();
1292-
const base = getBaseFileName(configFilePath);
1293-
return base === "tsconfig.json" || base === "jsconfig.json" ? base : "other";
1292+
return getBaseConfigFileName(configFilePath) || "other";
12941293
}
12951294

12961295
function convertTypeAcquisition({ enable, include, exclude }: TypeAcquisition): ProjectInfoTypeAcquisitionData {
@@ -1344,13 +1343,14 @@ namespace ts.server {
13441343

13451344
private updateNonInferredProjectFiles<T>(project: ExternalProject | ConfiguredProject, newUncheckedFiles: T[], propertyReader: FilePropertyReader<T>, clientFileName?: string) {
13461345
const projectRootFilesMap = project.getRootFilesMap();
1347-
const newRootScriptInfoMap: Map<ProjectRoot> = createMap<ProjectRoot>();
1346+
const newRootScriptInfoMap = createMap<ProjectRoot>();
13481347

13491348
for (const f of newUncheckedFiles) {
13501349
const newRootFile = propertyReader.getFileName(f);
13511350
const normalizedPath = toNormalizedPath(newRootFile);
13521351
let scriptInfo: ScriptInfo | NormalizedPath;
13531352
let path: Path;
1353+
// Use the project's lsHost so that it can use caching instead of reaching to disk for the query
13541354
if (!project.lsHost.fileExists(newRootFile)) {
13551355
path = normalizedPathToPath(normalizedPath, this.currentDirectory, this.toCanonicalFileName);
13561356
const existingValue = projectRootFilesMap.get(path);
@@ -1378,7 +1378,7 @@ namespace ts.server {
13781378
newRootScriptInfoMap.set(path, scriptInfo);
13791379
}
13801380

1381-
// project's root file map size is always going to be larger than new roots map
1381+
// project's root file map size is always going to be same or larger than new roots map
13821382
// as we have already all the new files to the project
13831383
if (projectRootFilesMap.size > newRootScriptInfoMap.size) {
13841384
projectRootFilesMap.forEach((value, path) => {
@@ -1388,12 +1388,14 @@ namespace ts.server {
13881388
}
13891389
else {
13901390
projectRootFilesMap.delete(path);
1391-
project.markAsDirty();
13921391
}
13931392
}
13941393
});
13951394
}
1396-
project.markAsDirty(); // Just to ensure that even if root files dont change, the changes to the non root file are picked up
1395+
1396+
// Just to ensure that even if root files dont change, the changes to the non root file are picked up,
1397+
// mark the project as dirty unconditionally
1398+
project.markAsDirty();
13971399
}
13981400

13991401
private updateNonInferredProject<T>(project: ExternalProject | ConfiguredProject, newUncheckedFiles: T[], propertyReader: FilePropertyReader<T>, newOptions: CompilerOptions, newTypeAcquisition: TypeAcquisition, compileOnSave: boolean, configFileErrors: Diagnostic[]) {
@@ -1408,38 +1410,22 @@ namespace ts.server {
14081410

14091411
/**
14101412
* Read the config file of the project again and update the project
1411-
* @param project
14121413
*/
14131414
/* @internal */
14141415
reloadConfiguredProject(project: ConfiguredProject) {
14151416
// At this point, there is no reason to not have configFile in the host
1416-
1417-
// note: the returned "success" is true does not mean the "configFileErrors" is empty.
1418-
// because we might have tolerated the errors and kept going. So always return the configFileErrors
1419-
// regardless the "success" here is true or not.
14201417
const host = project.getCachedServerHost();
1418+
1419+
// Clear the cache since we are reloading the project from disk
14211420
host.clearCache();
14221421
const configFileName = project.getConfigFilePath();
14231422
this.logger.info(`Reloading configured project ${configFileName}`);
1424-
const { projectOptions, configFileErrors, configFileSpecs } = this.convertConfigFileContentToProjectOptions(configFileName, host);
1425-
project.configFileSpecs = configFileSpecs;
1426-
this.updateConfiguredProject(project, projectOptions, configFileErrors);
1427-
1428-
if (!this.eventHandler) {
1429-
return;
1430-
}
14311423

1432-
this.eventHandler(<ConfigFileDiagEvent>{
1433-
eventName: ConfigFileDiagEvent,
1434-
data: { configFileName, diagnostics: project.getGlobalProjectErrors() || [], triggerFile: configFileName }
1435-
});
1436-
}
1424+
// Read updated contents from disk
1425+
const { projectOptions, configFileErrors, configFileSpecs } = this.convertConfigFileContentToProjectOptions(configFileName, host);
14371426

1438-
/**
1439-
* Updates the configured project with updated config file contents
1440-
* @param project
1441-
*/
1442-
private updateConfiguredProject(project: ConfiguredProject, projectOptions: ProjectOptions, configFileErrors: Diagnostic[]) {
1427+
// Update the project
1428+
project.configFileSpecs = configFileSpecs;
14431429
if (this.exceededTotalSizeLimitForNonTsFiles(project.canonicalConfigFilePath, projectOptions.compilerOptions, projectOptions.files, fileNamePropertyReader)) {
14441430
project.disableLanguageService();
14451431
project.stopWatchingWildCards(WatcherCloseReason.ProjectReloadHitMaxSize);
@@ -1451,6 +1437,15 @@ namespace ts.server {
14511437
project.watchTypeRoots();
14521438
}
14531439
this.updateNonInferredProject(project, projectOptions.files, fileNamePropertyReader, projectOptions.compilerOptions, projectOptions.typeAcquisition, projectOptions.compileOnSave, configFileErrors);
1440+
1441+
if (!this.eventHandler) {
1442+
return;
1443+
}
1444+
1445+
this.eventHandler(<ConfigFileDiagEvent>{
1446+
eventName: ConfigFileDiagEvent,
1447+
data: { configFileName, diagnostics: project.getGlobalProjectErrors() || [], triggerFile: configFileName }
1448+
});
14541449
}
14551450

14561451
/*@internal*/
@@ -1587,8 +1582,8 @@ namespace ts.server {
15871582
}
15881583
if (args.extraFileExtensions) {
15891584
this.hostConfiguration.extraFileExtensions = args.extraFileExtensions;
1590-
// We need to update the projects because of we might interprete more/less files
1591-
// depending on whether extra files extenstions are either added or removed
1585+
// We need to update the project structures again as it is possible that existing
1586+
// project structure could have more or less files depending on extensions permitted
15921587
this.reloadProjects();
15931588
this.logger.info("Host file extension mappings updated");
15941589
}
@@ -1664,7 +1659,7 @@ namespace ts.server {
16641659
* If the there is no existing project it just opens the configured project for the config file
16651660
*/
16661661
private reloadConfiguredsProjectForFiles(openFiles: ScriptInfo[], delayReload: boolean) {
1667-
const mapUpdatedProjects = createMap<true>();
1662+
const updatedProjects = createMap<true>();
16681663
// try to reload config file for all open files
16691664
for (const info of openFiles) {
16701665
// This tries to search for a tsconfig.json for the given file. If we found it,
@@ -1673,32 +1668,35 @@ namespace ts.server {
16731668
// otherwise we create a new one.
16741669
const configFileName = this.getConfigFileNameForFile(info);
16751670
if (configFileName) {
1676-
let project = this.findConfiguredProjectByProjectName(configFileName);
1671+
const project = this.findConfiguredProjectByProjectName(configFileName);
16771672
if (!project) {
1678-
project = this.createConfiguredProject(configFileName, info.fileName);
1679-
mapUpdatedProjects.set(configFileName, true);
1673+
this.createConfiguredProject(configFileName, info.fileName);
1674+
updatedProjects.set(configFileName, true);
16801675
}
1681-
else if (!mapUpdatedProjects.has(configFileName)) {
1676+
else if (!updatedProjects.has(configFileName)) {
16821677
if (delayReload) {
16831678
project.pendingReload = true;
16841679
this.delayUpdateProjectGraph(project);
16851680
}
16861681
else {
16871682
this.reloadConfiguredProject(project);
16881683
}
1689-
mapUpdatedProjects.set(configFileName, true);
1684+
updatedProjects.set(configFileName, true);
16901685
}
16911686
}
16921687
}
16931688
}
16941689

16951690
/**
1696-
* - script info can be never migrate to state - root file in inferred project, this is only a starting point
1697-
* - if script info has more that one containing projects - it is not a root file in inferred project because:
1698-
* - references in inferred project supercede the root part
1699-
* - root/reference in non-inferred project beats root in inferred project
1691+
* Remove the root of inferred project if script info is part of another project
17001692
*/
17011693
private removeRootOfInferredProjectIfNowPartOfOtherProject(info: ScriptInfo) {
1694+
// If the script info is root of inferred project, it could only be first containing project
1695+
// since info is added to inferred project and made root only when there are no other projects containing it
1696+
// So even if it is root of the inferred project and after project structure updates its now part
1697+
// of multiple project it needs to be removed from that inferred project because:
1698+
// - references in inferred project supercede the root part
1699+
// - root / reference in non - inferred project beats root in inferred project
17021700
if (info.containingProjects.length > 1 &&
17031701
info.containingProjects[0].projectKind === ProjectKind.Inferred &&
17041702
info.containingProjects[0].isRoot(info)) {
@@ -1728,8 +1726,8 @@ namespace ts.server {
17281726
if (info.containingProjects.length === 0) {
17291727
this.assignScriptInfoToInferredProject(info);
17301728
}
1731-
// Or remove the root of inferred project if is referenced in more than one projects
17321729
else {
1730+
// Or remove the root of inferred project if is referenced in more than one projects
17331731
this.removeRootOfInferredProjectIfNowPartOfOtherProject(info);
17341732
}
17351733
}
@@ -1848,7 +1846,7 @@ namespace ts.server {
18481846
if (!this.changedFiles) {
18491847
this.changedFiles = [scriptInfo];
18501848
}
1851-
else if (this.changedFiles.indexOf(scriptInfo) < 0) {
1849+
else if (!contains(this.changedFiles, scriptInfo)) {
18521850
this.changedFiles.push(scriptInfo);
18531851
}
18541852
}
@@ -2023,8 +2021,7 @@ namespace ts.server {
20232021
const rootFiles: protocol.ExternalFile[] = [];
20242022
for (const file of proj.rootFiles) {
20252023
const normalized = toNormalizedPath(file.fileName);
2026-
const baseFileName = getBaseFileName(normalized);
2027-
if (baseFileName === "tsconfig.json" || baseFileName === "jsconfig.json") {
2024+
if (getBaseConfigFileName(normalized)) {
20282025
if (this.host.fileExists(normalized)) {
20292026
(tsConfigFiles || (tsConfigFiles = [])).push(normalized);
20302027
}

‎src/server/project.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -487,13 +487,12 @@ namespace ts.server {
487487

488488
// add a root file to project
489489
addRoot(info: ScriptInfo) {
490-
if (!this.isRoot(info)) {
491-
this.rootFiles.push(info);
492-
this.rootFilesMap.set(info.path, info);
493-
info.attachToProject(this);
490+
Debug.assert(!this.isRoot(info));
491+
this.rootFiles.push(info);
492+
this.rootFilesMap.set(info.path, info);
493+
info.attachToProject(this);
494494

495-
this.markAsDirty();
496-
}
495+
this.markAsDirty();
497496
}
498497

499498
// add a root file to project

‎src/server/utilities.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,11 @@ namespace ts.server {
227227

228228
/* @internal */
229229
namespace ts.server {
230+
export function getBaseConfigFileName(configFilePath: NormalizedPath): "tsconfig.json" | "jsconfig.json" | undefined {
231+
const base = getBaseFileName(configFilePath);
232+
return base === "tsconfig.json" || base === "jsconfig.json" ? base : undefined;
233+
}
234+
230235
export function insertSorted<T>(array: SortedArray<T>, insert: T, compare: Comparer<T>): void {
231236
if (array.length === 0) {
232237
array.push(insert);

0 commit comments

Comments
 (0)