Skip to content

Commit b155a71

Browse files
sheetalkamatsheetalkamat
committed
Retain the version information of script infos when they are deleted
This helps in having to not restart the versioning, which could potentially have same version but different contents and project could confuse with it
1 parent 0b5b7ef commit b155a71

5 files changed

Lines changed: 148 additions & 21 deletions

File tree

‎src/harness/virtualFileSystemWithWatch.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,13 @@ interface Array<T> {}`
615615
}
616616
}
617617

618+
removeFile(filePath: string) {
619+
const path = this.toFullPath(filePath);
620+
const currentEntry = this.fs.get(path) as FsFile;
621+
Debug.assert(isFsFile(currentEntry));
622+
this.removeFileOrFolder(currentEntry, returnFalse);
623+
}
624+
618625
removeFolder(folderPath: string, recursive?: boolean) {
619626
const path = this.toFullPath(folderPath);
620627
const currentEntry = this.fs.get(path) as FsFolder;

‎src/server/editorServices.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,11 @@ namespace ts.server {
334334
return project.dirty && project.updateGraph();
335335
}
336336

337+
type ScriptInfoOrVersion = ScriptInfo | ScriptInfoVersion;
338+
function isScriptInfoVersion(infoOrVersion: ScriptInfoOrVersion): infoOrVersion is ScriptInfoVersion {
339+
return (infoOrVersion as ScriptInfoVersion).svc !== undefined;
340+
}
341+
337342
export class ProjectService {
338343

339344
/*@internal*/
@@ -345,7 +350,7 @@ namespace ts.server {
345350
/**
346351
* Container of all known scripts
347352
*/
348-
private readonly filenameToScriptInfo = createMap<ScriptInfo>();
353+
private readonly filenameToScriptInfo = createMap<ScriptInfoOrVersion>();
349354
// Set of all '.js' files ever opened.
350355
private readonly allJsFilesForOpenFileTelemetry = createMap<true>();
351356

@@ -884,7 +889,7 @@ namespace ts.server {
884889

885890
project.close();
886891
if (Debug.shouldAssert(AssertionLevel.Normal)) {
887-
this.filenameToScriptInfo.forEach(info => Debug.assert(!info.isAttached(project)));
892+
this.filenameToScriptInfo.forEach(info => Debug.assert(isScriptInfoVersion(info) || !info.isAttached(project)));
888893
}
889894
// Remove the project from pending project updates
890895
this.pendingProjectUpdates.delete(project.getProjectName());
@@ -1021,7 +1026,7 @@ namespace ts.server {
10211026
}
10221027

10231028
private deleteScriptInfo(info: ScriptInfo) {
1024-
this.filenameToScriptInfo.delete(info.path);
1029+
this.filenameToScriptInfo.set(info.path, info.getVersion());
10251030
const realpath = info.getRealpathIfDifferent();
10261031
if (realpath) {
10271032
this.realpathToScriptInfos!.remove(realpath, info); // TODO: GH#18217
@@ -1850,8 +1855,8 @@ namespace ts.server {
18501855
private getOrCreateScriptInfoWorker(fileName: NormalizedPath, currentDirectory: string, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean, hostToQueryFileExistsOn?: { fileExists(path: string): boolean; }) {
18511856
Debug.assert(fileContent === undefined || openedByClient, "ScriptInfo needs to be opened by client to be able to set its user defined content");
18521857
const path = normalizedPathToPath(fileName, currentDirectory, this.toCanonicalFileName);
1853-
let info = this.getScriptInfoForPath(path);
1854-
if (!info) {
1858+
let info = this.filenameToScriptInfo.get(path);
1859+
if (!info || isScriptInfoVersion(info)) {
18551860
const isDynamic = isDynamicFileName(fileName);
18561861
Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nScript info with non-dynamic relative file name can only be open script info`);
18571862
Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nOpen script files with non rooted disk path opened with current directory context cannot have same canonical names`);
@@ -1860,7 +1865,7 @@ namespace ts.server {
18601865
if (!openedByClient && !isDynamic && !(hostToQueryFileExistsOn || this.host).fileExists(fileName)) {
18611866
return;
18621867
}
1863-
info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path); // TODO: GH#18217
1868+
info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path, info); // TODO: GH#18217
18641869
this.filenameToScriptInfo.set(info.path, info);
18651870
if (!openedByClient) {
18661871
this.watchClosedScriptInfo(info);
@@ -1894,7 +1899,8 @@ namespace ts.server {
18941899
}
18951900

18961901
getScriptInfoForPath(fileName: Path) {
1897-
return this.filenameToScriptInfo.get(fileName);
1902+
const info = this.filenameToScriptInfo.get(fileName);
1903+
return info && isScriptInfoVersion(info) ? undefined : info;
18981904
}
18991905

19001906
setHostConfiguration(args: protocol.ConfigureRequestArguments) {
@@ -2145,7 +2151,7 @@ namespace ts.server {
21452151
// It was then postponed to cleanup these script infos so that they can be reused if
21462152
// the file from that old project is reopened because of opening file from here.
21472153
this.filenameToScriptInfo.forEach(info => {
2148-
if (!info.isScriptOpen() && info.isOrphan()) {
2154+
if (!isScriptInfoVersion(info) && !info.isScriptOpen() && info.isOrphan()) {
21492155
// if there are not projects that include this script info - delete it
21502156
this.stopWatchingScriptInfo(info);
21512157
this.deleteScriptInfo(info);

‎src/server/scriptInfo.ts‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
namespace ts.server {
2+
/*@internal*/
3+
export interface ScriptInfoVersion {
4+
svc: number;
5+
text: number;
6+
}
27

38
/* @internal */
49
export class TextStorage {
10+
/*@internal*/
11+
version: ScriptInfoVersion;
12+
513
/**
614
* Generated only on demand (based on edits, or information requested)
715
* The property text is set to undefined when edits happen on the cache
816
*/
917
private svc: ScriptVersionCache | undefined;
10-
private svcVersion = 0;
1118

1219
/**
1320
* Stores the text when there are no changes to the script version cache
@@ -19,7 +26,6 @@ namespace ts.server {
1926
* Line map for the text when there is no script version cache present
2027
*/
2128
private lineMap: number[] | undefined;
22-
private textVersion = 0;
2329

2430
/**
2531
* True if the text is for the file thats open in the editor
@@ -34,13 +40,14 @@ namespace ts.server {
3440
*/
3541
private pendingReloadFromDisk: boolean;
3642

37-
constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath) {
43+
constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath, initialVersion?: ScriptInfoVersion) {
44+
this.version = initialVersion || { svc: 0, text: 0 };
3845
}
3946

4047
public getVersion() {
4148
return this.svc
42-
? `SVC-${this.svcVersion}-${this.svc.getSnapshotVersion()}`
43-
: `Text-${this.textVersion}`;
49+
? `SVC-${this.version.svc}-${this.svc.getSnapshotVersion()}`
50+
: `Text-${this.version.text}`;
4451
}
4552

4653
public hasScriptVersionCache_TestOnly() {
@@ -55,7 +62,7 @@ namespace ts.server {
5562
this.svc = undefined;
5663
this.text = newText;
5764
this.lineMap = undefined;
58-
this.textVersion++;
65+
this.version.text++;
5966
}
6067

6168
public edit(start: number, end: number, newText: string) {
@@ -163,7 +170,7 @@ namespace ts.server {
163170
private switchToScriptVersionCache(): ScriptVersionCache {
164171
if (!this.svc || this.pendingReloadFromDisk) {
165172
this.svc = ScriptVersionCache.fromString(this.getOrLoadText());
166-
this.svcVersion++;
173+
this.version.svc++;
167174
}
168175
return this.svc;
169176
}
@@ -235,10 +242,11 @@ namespace ts.server {
235242
readonly fileName: NormalizedPath,
236243
readonly scriptKind: ScriptKind,
237244
public readonly hasMixedContent: boolean,
238-
readonly path: Path) {
245+
readonly path: Path,
246+
initialVersion?: ScriptInfoVersion) {
239247
this.isDynamic = isDynamicFileName(fileName);
240248

241-
this.textStorage = new TextStorage(host, fileName);
249+
this.textStorage = new TextStorage(host, fileName, initialVersion);
242250
if (hasMixedContent || this.isDynamic) {
243251
this.textStorage.reload("");
244252
this.realpath = this.path;
@@ -248,6 +256,11 @@ namespace ts.server {
248256
: getScriptKindFromFileName(fileName);
249257
}
250258

259+
/*@internal*/
260+
getVersion() {
261+
return this.textStorage.version;
262+
}
263+
251264
/*@internal*/
252265
public isDynamicOrHasMixedContent() {
253266
return this.hasMixedContent || this.isDynamic;

‎src/testRunner/unittests/tsserverProjectSystem.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8830,4 +8830,101 @@ export const x = 10;`
88308830
assert.equal(moduleInfo.cacheSourceFile.sourceFile.text, updatedModuleContent);
88318831
});
88328832
});
8833+
8834+
describe("tsserverProjectSystem syntax operations", () => {
8835+
function navBarFull(session: TestSession, file: File) {
8836+
return JSON.stringify(session.executeCommandSeq<protocol.FileRequest>({
8837+
command: protocol.CommandTypes.NavBarFull,
8838+
arguments: { file: file.path }
8839+
}).response);
8840+
}
8841+
8842+
function openFile(session: TestSession, file: File) {
8843+
session.executeCommandSeq<protocol.OpenRequest>({
8844+
command: protocol.CommandTypes.Open,
8845+
arguments: { file: file.path, fileContent: file.content }
8846+
});
8847+
}
8848+
8849+
it("works when file is removed and added with different content", () => {
8850+
const projectRoot = "/user/username/projects/myproject";
8851+
const app: File = {
8852+
path: `${projectRoot}/app.ts`,
8853+
content: "console.log('Hello world');"
8854+
};
8855+
const unitTest1: File = {
8856+
path: `${projectRoot}/unitTest1.ts`,
8857+
content: `import assert = require('assert');
8858+
8859+
describe("Test Suite 1", () => {
8860+
it("Test A", () => {
8861+
assert.ok(true, "This shouldn't fail");
8862+
});
8863+
8864+
it("Test B", () => {
8865+
assert.ok(1 === 1, "This shouldn't fail");
8866+
assert.ok(false, "This should fail");
8867+
});
8868+
});`
8869+
};
8870+
const tsconfig: File = {
8871+
path: `${projectRoot}/tsconfig.json`,
8872+
content: "{}"
8873+
};
8874+
const files = [app, libFile, tsconfig];
8875+
const host = createServerHost(files);
8876+
const session = createSession(host);
8877+
const service = session.getProjectService();
8878+
openFile(session, app);
8879+
8880+
checkNumberOfProjects(service, { configuredProjects: 1 });
8881+
const project = service.configuredProjects.get(tsconfig.path)!;
8882+
const expectedFilesWithoutUnitTest1 = files.map(f => f.path);
8883+
checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
8884+
8885+
host.writeFile(unitTest1.path, unitTest1.content);
8886+
host.runQueuedTimeoutCallbacks();
8887+
const expectedFilesWithUnitTest1 = expectedFilesWithoutUnitTest1.concat(unitTest1.path);
8888+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8889+
8890+
openFile(session, unitTest1);
8891+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8892+
8893+
const navBarResultUnitTest1 = navBarFull(session, unitTest1);
8894+
host.removeFile(unitTest1.path);
8895+
host.checkTimeoutQueueLengthAndRun(2);
8896+
checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
8897+
8898+
session.executeCommandSeq<protocol.CloseRequest>({
8899+
command: protocol.CommandTypes.Close,
8900+
arguments: { file: unitTest1.path }
8901+
});
8902+
checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
8903+
8904+
const unitTest1WithChangedContent: File = {
8905+
path: unitTest1.path,
8906+
content: `import assert = require('assert');
8907+
8908+
export function Test1() {
8909+
assert.ok(true, "This shouldn't fail");
8910+
};
8911+
8912+
export function Test2() {
8913+
assert.ok(1 === 1, "This shouldn't fail");
8914+
assert.ok(false, "This should fail");
8915+
};`
8916+
};
8917+
host.writeFile(unitTest1.path, unitTest1WithChangedContent.content);
8918+
host.runQueuedTimeoutCallbacks();
8919+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8920+
8921+
openFile(session, unitTest1WithChangedContent);
8922+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8923+
const sourceFile = project.getLanguageService().getNonBoundSourceFile(unitTest1WithChangedContent.path);
8924+
assert.strictEqual(sourceFile.text, unitTest1WithChangedContent.content);
8925+
8926+
const navBarResultUnitTest1WithChangedContent = navBarFull(session, unitTest1WithChangedContent);
8927+
assert.notStrictEqual(navBarResultUnitTest1WithChangedContent, navBarResultUnitTest1, "With changes in contents of unitTest file, we should see changed naviagation bar item result");
8928+
});
8929+
});
88338930
}

‎tests/baselines/reference/api/tsserverlibrary.d.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13319,18 +13319,21 @@ declare namespace ts.server.protocol {
1331913319
}
1332013320
}
1332113321
declare namespace ts.server {
13322+
interface ScriptInfoVersion {
13323+
svc: number;
13324+
text: number;
13325+
}
1332213326
class TextStorage {
1332313327
private readonly host;
1332413328
private readonly fileName;
13329+
version: ScriptInfoVersion;
1332513330
private svc;
13326-
private svcVersion;
1332713331
private text;
1332813332
private lineMap;
13329-
private textVersion;
1333013333
isOpen: boolean;
1333113334
private ownFileText;
1333213335
private pendingReloadFromDisk;
13333-
constructor(host: ServerHost, fileName: NormalizedPath);
13336+
constructor(host: ServerHost, fileName: NormalizedPath, initialVersion?: ScriptInfoVersion);
1333413337
getVersion(): string;
1333513338
hasScriptVersionCache_TestOnly(): boolean;
1333613339
useScriptVersionCache_TestOnly(): void;
@@ -13370,7 +13373,8 @@ declare namespace ts.server {
1337013373
readonly isDynamic: boolean;
1337113374
private realpath;
1337213375
cacheSourceFile: DocumentRegistrySourceFileCache;
13373-
constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path);
13376+
constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path, initialVersion?: ScriptInfoVersion);
13377+
getVersion(): ScriptInfoVersion;
1337413378
isDynamicOrHasMixedContent(): boolean;
1337513379
isScriptOpen(): boolean;
1337613380
open(newText: string): void;

0 commit comments

Comments
 (0)