Skip to content

Commit

Permalink
Handle if project for open file will get recollected because of pendi…
Browse files Browse the repository at this point in the history 
…ng cleanup from closed script info (#50908)

* Handle if project for open file will get recollected because of pending cleanup from closed script info
Fixes #50868

* Rename
  • Loading branch information
@sheetalkamat
sheetalkamat committed Sep 26, 2022
1 parent c81bf4d commit 09368bc
Show file tree
Hide file tree
Showing 6 changed files with 966 additions and 46 deletions.
18 changes: 10 additions & 8 deletions src/server/editorServices.ts
View file
Expand Up @@ -1167,20 +1167,22 @@ namespace ts.server {
}

/* @internal */
tryGetDefaultProjectForFile(fileName: NormalizedPath): Project | undefined {
const scriptInfo = this.getScriptInfoForNormalizedPath(fileName);
tryGetDefaultProjectForFile(fileNameOrScriptInfo: NormalizedPath | ScriptInfo): Project | undefined {
const scriptInfo = isString(fileNameOrScriptInfo) ? this.getScriptInfoForNormalizedPath(fileNameOrScriptInfo) : fileNameOrScriptInfo;
return scriptInfo && !scriptInfo.isOrphan() ? scriptInfo.getDefaultProject() : undefined;
}

/* @internal */
ensureDefaultProjectForFile(fileName: NormalizedPath): Project {
return this.tryGetDefaultProjectForFile(fileName) || this.doEnsureDefaultProjectForFile(fileName);
ensureDefaultProjectForFile(fileNameOrScriptInfo: NormalizedPath | ScriptInfo): Project {
return this.tryGetDefaultProjectForFile(fileNameOrScriptInfo) || this.doEnsureDefaultProjectForFile(fileNameOrScriptInfo);
}

private doEnsureDefaultProjectForFile(fileName: NormalizedPath): Project {
private doEnsureDefaultProjectForFile(fileNameOrScriptInfo: NormalizedPath | ScriptInfo): Project {
this.ensureProjectStructuresUptoDate();
const scriptInfo = this.getScriptInfoForNormalizedPath(fileName);
return scriptInfo ? scriptInfo.getDefaultProject() : (this.logErrorForScriptInfoNotFound(fileName), Errors.ThrowNoProject());
const scriptInfo = isString(fileNameOrScriptInfo) ? this.getScriptInfoForNormalizedPath(fileNameOrScriptInfo) : fileNameOrScriptInfo;
return scriptInfo ?
scriptInfo.getDefaultProject() :
(this.logErrorForScriptInfoNotFound(isString(fileNameOrScriptInfo) ? fileNameOrScriptInfo : fileNameOrScriptInfo.fileName), Errors.ThrowNoProject());
}

getScriptInfoEnsuringProjectsUptoDate(uncheckedFileName: string) {
Expand Down Expand Up @@ -3648,7 +3650,7 @@ namespace ts.server {
return;
}

const project = scriptInfo.getDefaultProject();
const project = this.ensureDefaultProjectForFile(scriptInfo);
if (!project.languageServiceEnabled) {
return;
}
Expand Down
12 changes: 5 additions & 7 deletions src/server/session.ts
View file
Expand Up @@ -1719,6 +1719,10 @@ namespace ts.server {
this.projectService.logErrorForScriptInfoNotFound(args.file);
return Errors.ThrowNoProject();
}
else if (!getScriptInfoEnsuringProjectsUptoDate) {
// Ensure there are containing projects are present
this.projectService.ensureDefaultProjectForFile(scriptInfo);
}
projects = scriptInfo.containingProjects;
symLinkedProjects = this.projectService.getSymlinkedProjects(scriptInfo);
}
Expand Down Expand Up @@ -1867,13 +1871,7 @@ namespace ts.server {
}

private getFileAndLanguageServiceForSyntacticOperation(args: protocol.FileRequestArgs) {
// Since this is syntactic operation, there should always be project for the file
// throw if we dont get project
const file = toNormalizedPath(args.file);
const project = this.getProject(args.projectFileName) || this.projectService.ensureDefaultProjectForFile(file);
if (!project) {
return Errors.ThrowNoProject();
}
const { file, project } = this.getFileAndProject(args);
return {
file,
languageService: project.getLanguageService(/*ensureSynchronized*/ false)
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/unittests/tsserver/helpers.ts
View file
Expand Up @@ -95,6 +95,7 @@ namespace ts.projectSystem {
function msg(s: string, type = server.Msg.Err, write: (s: string) => void) {
s = `[${nowString()}] ${s}`;
if (!inGroup || firstInGroup) s = padStringRight(type + " " + seq.toString(), " ") + s;
if (Debug.isDebugging) console.log(s);

This comment has been minimized.

Sign in to view

Copy link
@weswigham

weswigham Sep 28, 2022

Member

This log is absolutely spamming my console when I run a simple runtests-parallel - Debug.isDebugging is set when tests are run from the vscode integrated terminal because Debug.isDebugging is keyed off the presence of the VSCODE_INSPECTOR_OPTIONS environment variable, which is set for any execution from the vscode integrated terminal, not just the vscode-launched debug runs.

If we want to keep logs like this, I think we need to change how ts.Debug.isDebugging is set.
write(s);
if (!inGroup) seq++;
}
Expand Down
110 changes: 79 additions & 31 deletions src/testRunner/unittests/tsserver/projects.ts
View file
Expand Up @@ -1610,39 +1610,87 @@ namespace ts.projectSystem {
checkNumberOfInferredProjects(projectService, 0);
});

it("file opened is in configured project that will be removed", () => {
const testsConfig: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig.json`,
content: "{}"
};
const testsFile: File = {
path: `${tscWatch.projectRoot}/playground/tests.ts`,
content: `export function foo() {}`
};
const innerFile: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig-json/tests/spec.ts`,
content: `export function bar() { }`
};
const innerConfig: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig-json/tsconfig.json`,
content: JSON.stringify({
include: ["./src"]
describe("file opened is in configured project that will be removed", () => {
function runOnTs<T extends server.protocol.Request>(scenario: string, getRequest: (innerFile: File) => Partial<T>) {
it(scenario, () => {
const testsConfig: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig.json`,
content: "{}"
};
const testsFile: File = {
path: `${tscWatch.projectRoot}/playground/tests.ts`,
content: `export function foo() {}`
};
const innerFile: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig-json/tests/spec.ts`,
content: `export function bar() { }`
};
const innerConfig: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig-json/tsconfig.json`,
content: JSON.stringify({
include: ["./src"]
})
};
const innerSrcFile: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig-json/src/src.ts`,
content: `export function foobar() { }`
};
const host = createServerHost([testsConfig, testsFile, innerFile, innerConfig, innerSrcFile, libFile]);
const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) });
openFilesForSession([testsFile], session);
closeFilesForSession([testsFile], session);
openFilesForSession([innerFile], session);
session.executeCommandSeq(getRequest(innerFile));
baselineTsserverLogs("projects", scenario, session);
});
}
runOnTs<protocol.OutliningSpansRequest>(
"file opened is in configured project that will be removed",
innerFile => ({
command: protocol.CommandTypes.GetOutliningSpans,
arguments: { file: innerFile.path }
})
};
const innerSrcFile: File = {
path: `${tscWatch.projectRoot}/playground/tsconfig-json/src/src.ts`,
content: `export function foobar() { }`
};
const host = createServerHost([testsConfig, testsFile, innerFile, innerConfig, innerSrcFile, libFile]);
const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) });
openFilesForSession([testsFile], session);
closeFilesForSession([testsFile], session);
openFilesForSession([innerFile], session);
session.executeCommandSeq<protocol.OutliningSpansRequest>({
command: protocol.CommandTypes.GetOutliningSpans,
arguments: { file: innerFile.path }
);

runOnTs<protocol.ReferencesRequest>(
"references on file opened is in configured project that will be removed",
innerFile => ({
command: protocol.CommandTypes.References,
arguments: protocolFileLocationFromSubstring(innerFile, "bar")
})
);

it("js file opened is in configured project that will be removed", () => {
const rootConfig: File = {
path: `${tscWatch.projectRoot}/tsconfig.json`,
content: JSON.stringify({ compilerOptions: { allowJs: true } })
};
const mocksFile: File = {
path: `${tscWatch.projectRoot}/mocks/cssMock.js`,
content: `function foo() { }`
};
const innerFile: File = {
path: `${tscWatch.projectRoot}/apps/editor/scripts/createConfigVariable.js`,
content: `function bar() { }`
};
const innerConfig: File = {
path: `${tscWatch.projectRoot}/apps/editor/tsconfig.json`,
content: JSON.stringify({
extends: "../../tsconfig.json",
include: ["./src"],
})
};
const innerSrcFile: File = {
path: `${tscWatch.projectRoot}/apps/editor/src/src.js`,
content: `function fooBar() { }`
};
const host = createServerHost([rootConfig, mocksFile, innerFile, innerConfig, innerSrcFile, libFile]);
const session = createSession(host, { canUseEvents: true, logger: createLoggerWithInMemoryLogs(host) });
openFilesForSession([mocksFile], session);
closeFilesForSession([mocksFile], session);
openFilesForSession([innerFile], session);
baselineTsserverLogs("projects", "js file opened is in configured project that will be removed", session);
});
baselineTsserverLogs("projects", "file opened is in configured project that will be removed", session);
});
});
}

0 comments on commit 09368bc

Please sign in to comment.