Skip to content

Keep scriptInfo and project alive even after file delete till next file open #57492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/harness/projectServiceStateLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import {
AutoImportProviderProject,
AuxiliaryProject,
ConfiguredProject,
isBackgroundProject,
isConfiguredProject,
LogLevel,
Expand All @@ -32,6 +33,7 @@ interface ProjectData {
isClosed: ReturnType<Project["isClosed"]>;
isOrphan: ReturnType<Project["isOrphan"]>;
noOpenRef: boolean;
deferredClose: ConfiguredProject["deferredClose"];
documentPositionMappers: SourceMapper["documentPositionMappers"];
autoImportProviderHost: Project["autoImportProviderHost"];
noDtsResolutionProject: Project["noDtsResolutionProject"];
Expand All @@ -46,6 +48,7 @@ interface ScriptInfoData {
open: ReturnType<ScriptInfo["isScriptOpen"]>;
version: ReturnType<TextStorage["getVersion"]>;
pendingReloadFromDisk: TextStorage["pendingReloadFromDisk"];
deferredDelete: ScriptInfo["deferredDelete"];
sourceMapFilePath: Exclude<ScriptInfo["sourceMapFilePath"], SourceMapFileWatcher> | SourceMapFileWatcherData | undefined;
declarationInfoPath: ScriptInfo["declarationInfoPath"];
sourceInfos: ScriptInfo["sourceInfos"];
Expand Down Expand Up @@ -116,6 +119,7 @@ export function patchServiceForStateBaseline(service: ProjectService) {
projectDiff = printProperty(PrintPropertyWhen.TruthyOrChangedOrNew, data, "isClosed", project.isClosed(), projectDiff, projectPropertyLogs);
projectDiff = printProperty(PrintPropertyWhen.TruthyOrChangedOrNew, data, "isOrphan", !isBackgroundProject(project) && project.isOrphan(), projectDiff, projectPropertyLogs);
projectDiff = printProperty(PrintPropertyWhen.TruthyOrChangedOrNew, data, "noOpenRef", isConfiguredProject(project) && !project.hasOpenRef(), projectDiff, projectPropertyLogs);
projectDiff = printProperty(PrintPropertyWhen.TruthyOrChangedOrNew, data, "deferredClose", isConfiguredProject(project) && project.deferredClose, projectDiff, projectPropertyLogs);
projectDiff = printMapPropertyValue(
PrintPropertyWhen.Changed,
data?.documentPositionMappers,
Expand Down Expand Up @@ -146,6 +150,7 @@ export function patchServiceForStateBaseline(service: ProjectService) {
isClosed: project.isClosed(),
isOrphan: !isBackgroundProject(project) && project.isOrphan(),
noOpenRef: isConfiguredProject(project) && !project.hasOpenRef(),
deferredClose: isConfiguredProject(project) && project.deferredClose,
autoImportProviderHost: project.autoImportProviderHost,
noDtsResolutionProject: project.noDtsResolutionProject,
originalConfiguredProjects: project.originalConfiguredProjects && new Set(project.originalConfiguredProjects),
Expand All @@ -166,6 +171,7 @@ export function patchServiceForStateBaseline(service: ProjectService) {
infoDiff = printProperty(PrintPropertyWhen.Changed, data, "open", isOpen, infoDiff, infoPropertyLogs);
infoDiff = printProperty(PrintPropertyWhen.Always, data, "version", info.textStorage.getVersion(), infoDiff, infoPropertyLogs);
infoDiff = printProperty(PrintPropertyWhen.TruthyOrChangedOrNew, data, "pendingReloadFromDisk", info.textStorage.pendingReloadFromDisk, infoDiff, infoPropertyLogs);
infoDiff = printProperty(PrintPropertyWhen.TruthyOrChangedOrNew, data, "deferredDelete", info.deferredDelete, infoDiff, infoPropertyLogs);
infoDiff = printScriptInfoSourceMapFilePath(data, info, infoDiff, infoPropertyLogs);
infoDiff = printProperty(PrintPropertyWhen.DefinedOrChangedOrNew, data, "declarationInfoPath", info.declarationInfoPath, infoDiff, infoPropertyLogs);
infoDiff = printSetPropertyValueWorker(PrintPropertyWhen.DefinedOrChangedOrNew, data?.sourceInfos, "sourceInfos", info.sourceInfos, infoDiff, infoPropertyLogs, identity);
Expand Down Expand Up @@ -200,6 +206,7 @@ export function patchServiceForStateBaseline(service: ProjectService) {
sourceInfos: info.sourceInfos && new Set(info.sourceInfos),
documentPositionMapper: info.documentPositionMapper,
containingProjects: new Set(info.containingProjects),
deferredDelete: info.deferredDelete,
}),
);
}
Expand Down
471 changes: 295 additions & 176 deletions src/server/editorServices.ts

Large diffs are not rendered by default.

45 changes: 42 additions & 3 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,12 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
}

private getOrCreateScriptInfoAndAttachToProject(fileName: string) {
const scriptInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost);
const scriptInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(
fileName,
this.currentDirectory,
this.directoryStructureHost,
/*deferredDeleteOk*/ false,
);
if (scriptInfo) {
const existingValue = this.rootFilesMap.get(scriptInfo.path);
if (existingValue && existingValue.info !== scriptInfo) {
Expand All @@ -678,7 +683,12 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
getScriptVersion(filename: string) {
// Don't attach to the project if version is asked

const info = this.projectService.getOrCreateScriptInfoNotOpenedByClient(filename, this.currentDirectory, this.directoryStructureHost);
const info = this.projectService.getOrCreateScriptInfoNotOpenedByClient(
filename,
this.currentDirectory,
this.directoryStructureHost,
/*deferredDeleteOk*/ false,
);
return (info && info.getLatestVersion())!; // TODO: GH#18217
}

Expand Down Expand Up @@ -1298,6 +1308,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
}
}

/** @internal */
markAsDirty() {
if (!this.dirty) {
this.projectStateVersion++;
Expand Down Expand Up @@ -1658,7 +1669,12 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
// by the host for files in the program when the program is retrieved above but
// the program doesn't contain external files so this must be done explicitly.
inserted => {
const scriptInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(inserted, this.currentDirectory, this.directoryStructureHost);
const scriptInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(
inserted,
this.currentDirectory,
this.directoryStructureHost,
/*deferredDeleteOk*/ false,
);
scriptInfo?.attachToProject(this);
},
removed => this.detachScriptInfoFromProject(removed),
Expand Down Expand Up @@ -2584,6 +2600,7 @@ export class AutoImportProviderProject extends Project {
return !some(this.rootFileNames);
}

/** @internal */
override isOrphan() {
return true;
}
Expand Down Expand Up @@ -2619,6 +2636,7 @@ export class AutoImportProviderProject extends Project {
return !!this.rootFileNames?.length;
}

/** @internal */
override markAsDirty() {
this.rootFileNames = undefined;
super.markAsDirty();
Expand Down Expand Up @@ -2713,6 +2731,9 @@ export class ConfiguredProject extends Project {
/** @internal */
skipConfigDiagEvent?: true;

/** @internal */
deferredClose?: boolean;

/** @internal */
constructor(
configFileName: NormalizedPath,
Expand Down Expand Up @@ -2773,6 +2794,7 @@ export class ConfiguredProject extends Project {
* @returns: true if set of files in the project stays the same and false - otherwise.
*/
override updateGraph(): boolean {
if (this.deferredClose) return false;
const isInitialLoad = this.isInitialLoadPending();
this.isInitialLoadPending = returnFalse;
const updateLevel = this.pendingUpdateLevel;
Expand Down Expand Up @@ -2892,6 +2914,12 @@ export class ConfiguredProject extends Project {
super.close();
}

/** @internal */
override markAsDirty() {
if (this.deferredClose) return;
super.markAsDirty();
}

/** @internal */
addExternalProjectReference() {
this.externalProjectRefCount++;
Expand Down Expand Up @@ -2941,6 +2969,7 @@ export class ConfiguredProject extends Project {
}

const configFileExistenceInfo = this.projectService.configFileExistenceInfoCache.get(this.canonicalConfigFilePath)!;
if (this.deferredClose) return !!configFileExistenceInfo.openFilesImpactedByConfigFile?.size;
if (this.projectService.hasPendingProjectUpdate(this)) {
// If there is pending update for this project,
// we dont know if this project would be needed by any of the open files impacted by this config file
Expand All @@ -2966,6 +2995,11 @@ export class ConfiguredProject extends Project {
) || false;
}

/** @internal */
override isOrphan(): boolean {
return !!this.deferredClose;
}

/** @internal */
hasExternalProjectRef() {
return !!this.externalProjectRefCount;
Expand Down Expand Up @@ -3023,3 +3057,8 @@ export function isExternalProject(project: Project): project is ExternalProject
export function isBackgroundProject(project: Project): project is AutoImportProviderProject | AuxiliaryProject {
return project.projectKind === ProjectKind.AutoImportProvider || project.projectKind === ProjectKind.Auxiliary;
}

/** @internal */
export function isProjectDeferredClose(project: Project): project is ConfiguredProject {
return isConfiguredProject(project) && !!project.deferredClose;
}
21 changes: 19 additions & 2 deletions src/server/scriptInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
IScriptSnapshot,
isString,
LineInfo,
missingFileModifiedTime,
orderedRemoveItem,
Path,
ScriptKind,
Expand All @@ -44,6 +45,7 @@ import {
isConfiguredProject,
isExternalProject,
isInferredProject,
isProjectDeferredClose,
maxFileSize,
NormalizedPath,
Project,
Expand Down Expand Up @@ -180,6 +182,14 @@ export class TextStorage {
const reloaded = this.reload(newText);
this.fileSize = fileSize; // NB: after reload since reload clears it
this.ownFileText = !tempFileName || tempFileName === this.info.fileName;
// In case we update this text before mTime gets updated to present file modified time
// because its schedule to do that later, update the mTime so we dont re-update the text
// Eg. with npm ci where file gets created and editor calls say get error request before
// the timeout to update the file stamps in node_modules is run
// Test:: watching npm install in codespaces where workspaces folder is hosted at root
if (this.ownFileText && this.info.mTime === missingFileModifiedTime.getTime()) {
this.info.mTime = (this.host.getModifiedTime!(this.info.fileName) || missingFileModifiedTime).getTime();
}
return reloaded;
}

Expand Down Expand Up @@ -398,6 +408,9 @@ export class ScriptInfo {
/** @internal */
documentPositionMapper?: DocumentPositionMapper | false;

/** @internal */
deferredDelete?: boolean;

constructor(
private readonly host: ServerHost,
readonly fileName: NormalizedPath,
Expand Down Expand Up @@ -567,7 +580,10 @@ export class ScriptInfo {
case 0:
return Errors.ThrowNoProject();
case 1:
return ensurePrimaryProjectKind(this.containingProjects[0]);
return ensurePrimaryProjectKind(
!isProjectDeferredClose(this.containingProjects[0]) ?
this.containingProjects[0] : undefined,
);
default:
// If this file belongs to multiple projects, below is the order in which default project is used
// - for open script info, its default configured project during opening is default if info is part of it
Expand All @@ -583,6 +599,7 @@ export class ScriptInfo {
for (let index = 0; index < this.containingProjects.length; index++) {
const project = this.containingProjects[index];
if (isConfiguredProject(project)) {
if (project.deferredClose) continue;
if (!project.isSourceOfProjectReferenceRedirect(this.fileName)) {
// If we havent found default configuredProject and
// its not the last one, find it and use that one if there
Expand Down Expand Up @@ -676,7 +693,7 @@ export class ScriptInfo {
}

isOrphan() {
return !forEach(this.containingProjects, p => !p.isOrphan());
return this.deferredDelete || !forEach(this.containingProjects, p => !p.isOrphan());
}

/** @internal */
Expand Down
1 change: 1 addition & 0 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,7 @@ export class Session<TMessage = string> implements EventSender {
fileNameToSearch,
noDtsProject.currentDirectory,
noDtsProject.directoryStructureHost,
/*deferredDeleteOk*/ false,
);
if (!info) continue;
if (!noDtsProject.containsScriptInfo(info)) {
Expand Down
Loading