Skip to content

Commit e4c6550

Browse files
authored
(chore) minor snapshot refactoring (#917)
Moved actual snapshot update/creation into ts service and DocumentSnapshot functions to remove duplicated code
1 parent bd31f58 commit e4c6550

File tree

7 files changed

+86
-91
lines changed

7 files changed

+86
-91
lines changed

packages/language-server/src/plugins/typescript/DocumentSnapshot.ts

+33-5
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,45 @@ export namespace DocumentSnapshot {
112112
/**
113113
* Returns a svelte or ts/js snapshot from a file path, depending on the file contents.
114114
* @param filePath path to the js/ts/svelte file
115+
* @param createDocument function that is used to create a document in case it's a Svelte file
115116
* @param options options that apply in case it's a svelte file
116117
*/
117-
export function fromFilePath(filePath: string, options: SvelteSnapshotOptions) {
118-
const originalText = ts.sys.readFile(filePath) ?? '';
119-
118+
export function fromFilePath(
119+
filePath: string,
120+
createDocument: (filePath: string, text: string) => Document,
121+
options: SvelteSnapshotOptions
122+
) {
120123
if (isSvelteFilePath(filePath)) {
121-
return fromDocument(new Document(pathToUrl(filePath), originalText), options);
124+
return DocumentSnapshot.fromSvelteFilePath(filePath, createDocument, options);
122125
} else {
123-
return new JSOrTSDocumentSnapshot(INITIAL_VERSION, filePath, originalText);
126+
return DocumentSnapshot.fromNonSvelteFilePath(filePath);
124127
}
125128
}
129+
130+
/**
131+
* Returns a ts/js snapshot from a file path.
132+
* @param filePath path to the js/ts file
133+
* @param options options that apply in case it's a svelte file
134+
*/
135+
export function fromNonSvelteFilePath(filePath: string) {
136+
const originalText = ts.sys.readFile(filePath) ?? '';
137+
return new JSOrTSDocumentSnapshot(INITIAL_VERSION, filePath, originalText);
138+
}
139+
140+
/**
141+
* Returns a svelte snapshot from a file path.
142+
* @param filePath path to the svelte file
143+
* @param createDocument function that is used to create a document
144+
* @param options options that apply in case it's a svelte file
145+
*/
146+
export function fromSvelteFilePath(
147+
filePath: string,
148+
createDocument: (filePath: string, text: string) => Document,
149+
options: SvelteSnapshotOptions
150+
) {
151+
const originalText = ts.sys.readFile(filePath) ?? '';
152+
return fromDocument(createDocument(filePath, originalText), options);
153+
}
126154
}
127155

128156
/**

packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts

+6-20
Original file line numberDiff line numberDiff line change
@@ -75,32 +75,18 @@ export class LSAndTSDocResolver {
7575
this.workspaceUris,
7676
this.lsDocumentContext
7777
);
78-
const filePath = document.getFilePath()!;
79-
const tsDoc = this.getSnapshot(filePath, document);
78+
const tsDoc = this.getSnapshot(document);
8079
const userPreferences = this.getUserPreferences(tsDoc.scriptKind);
8180

8281
return { tsDoc, lang, userPreferences };
8382
}
8483

85-
getSnapshot(filePath: string, document: Document): SvelteDocumentSnapshot;
86-
getSnapshot(filePath: string, document?: Document): DocumentSnapshot;
87-
getSnapshot(filePath: string, document?: Document) {
84+
getSnapshot(document: Document): SvelteDocumentSnapshot;
85+
getSnapshot(pathOrDoc: string | Document): DocumentSnapshot;
86+
getSnapshot(pathOrDoc: string | Document) {
87+
const filePath = typeof pathOrDoc === 'string' ? pathOrDoc : pathOrDoc.getFilePath() || '';
8888
const tsService = this.getTSService(filePath);
89-
const { snapshotManager } = tsService;
90-
91-
let tsDoc = snapshotManager.get(filePath);
92-
if (!tsDoc) {
93-
const options = {
94-
strictMode: !!tsService.compilerOptions.strict,
95-
transformOnTemplateError: this.transformOnTemplateError
96-
};
97-
tsDoc = document
98-
? DocumentSnapshot.fromDocument(document, options)
99-
: DocumentSnapshot.fromFilePath(filePath, options);
100-
snapshotManager.set(filePath, tsDoc);
101-
}
102-
103-
return tsDoc;
89+
return tsService.updateDocument(pathOrDoc);
10490
}
10591

10692
updateSnapshotPath(oldPath: string, newPath: string): DocumentSnapshot {

packages/language-server/src/plugins/typescript/SnapshotManager.ts

+17-31
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
import ts from 'typescript';
2-
import {
3-
DocumentSnapshot,
4-
JSOrTSDocumentSnapshot,
5-
SvelteSnapshotOptions
6-
} from './DocumentSnapshot';
2+
import { DocumentSnapshot, JSOrTSDocumentSnapshot } from './DocumentSnapshot';
73
import { Logger } from '../../logger';
84
import { TextDocumentContentChangeEvent } from 'vscode-languageserver';
95

@@ -50,36 +46,26 @@ export class SnapshotManager {
5046
this.projectFiles = Array.from(new Set([...this.projectFiles, ...projectFiles]));
5147
}
5248

53-
updateByFileName(fileName: string, options: SvelteSnapshotOptions) {
54-
if (!this.has(fileName)) {
55-
return;
56-
}
57-
58-
const newSnapshot = DocumentSnapshot.fromFilePath(fileName, options);
49+
updateTsOrJsFile(fileName: string, changes?: TextDocumentContentChangeEvent[]): void {
5950
const previousSnapshot = this.get(fileName);
6051

61-
if (previousSnapshot) {
62-
newSnapshot.version = previousSnapshot.version + 1;
52+
if (changes) {
53+
if (!(previousSnapshot instanceof JSOrTSDocumentSnapshot)) {
54+
return;
55+
}
56+
previousSnapshot.update(changes);
6357
} else {
64-
// ensure it's greater than initial version
65-
// so that ts server picks up the change
66-
newSnapshot.version += 1;
58+
const newSnapshot = DocumentSnapshot.fromNonSvelteFilePath(fileName);
59+
60+
if (previousSnapshot) {
61+
newSnapshot.version = previousSnapshot.version + 1;
62+
} else {
63+
// ensure it's greater than initial version
64+
// so that ts server picks up the change
65+
newSnapshot.version += 1;
66+
}
67+
this.set(fileName, newSnapshot);
6768
}
68-
69-
this.set(fileName, newSnapshot);
70-
}
71-
72-
updateTsOrJsFile(fileName: string, changes: TextDocumentContentChangeEvent[]): void {
73-
if (!this.has(fileName)) {
74-
return;
75-
}
76-
77-
const previousSnapshot = this.get(fileName);
78-
if (!(previousSnapshot instanceof JSOrTSDocumentSnapshot)) {
79-
return;
80-
}
81-
82-
previousSnapshot.update(changes);
8369
}
8470

8571
has(fileName: string) {

packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts

+1-6
Original file line numberDiff line numberDiff line change
@@ -385,12 +385,7 @@ export class TypeScriptPlugin
385385
return;
386386
}
387387

388-
// Since the options parameter only applies to svelte snapshots, and this is not
389-
// a svelte file, we can just set it to false without having any effect.
390-
snapshotManager.updateByFileName(fileName, {
391-
strictMode: false,
392-
transformOnTemplateError: false
393-
});
388+
snapshotManager.updateTsOrJsFile(fileName);
394389
}
395390
}
396391

packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts

-4
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,4 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
398398
private getLSAndTSDoc(document: Document) {
399399
return this.lsAndTsDocResolver.getLSAndTSDoc(document);
400400
}
401-
402-
private getSnapshot(filePath: string, document?: Document) {
403-
return this.lsAndTsDocResolver.getSnapshot(filePath, document);
404-
}
405401
}

packages/language-server/src/plugins/typescript/features/RenameProvider.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -374,8 +374,8 @@ export class RenameProviderImpl implements RenameProvider {
374374
return this.lsAndTsDocResolver.getLSAndTSDoc(document);
375375
}
376376

377-
private getSnapshot(filePath: string, document?: Document) {
378-
return this.lsAndTsDocResolver.getSnapshot(filePath, document);
377+
private getSnapshot(filePath: string) {
378+
return this.lsAndTsDocResolver.getSnapshot(filePath);
379379
}
380380
}
381381

packages/language-server/src/plugins/typescript/service.ts

+27-23
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ import { getPackageInfo } from '../../importPackage';
66
import { DocumentSnapshot } from './DocumentSnapshot';
77
import { createSvelteModuleLoader } from './module-loader';
88
import { SnapshotManager } from './SnapshotManager';
9-
import { ensureRealSvelteFilePath, findTsConfigPath, isSvelteFilePath } from './utils';
9+
import { ensureRealSvelteFilePath, findTsConfigPath } from './utils';
1010

1111
export interface LanguageServiceContainer {
1212
readonly tsconfigPath: string;
1313
readonly compilerOptions: ts.CompilerOptions;
1414
readonly snapshotManager: SnapshotManager;
1515
getService(): ts.LanguageService;
16-
updateDocument(document: Document): ts.LanguageService;
16+
updateDocument(documentOrFilePath: Document | string): DocumentSnapshot;
1717
deleteDocument(filePath: string): void;
1818
}
1919

@@ -37,9 +37,7 @@ export function getLanguageServiceForDocument(
3737
workspaceUris: string[],
3838
docContext: LanguageServiceDocumentContext
3939
): ts.LanguageService {
40-
return getService(document.getFilePath() || '', workspaceUris, docContext).updateDocument(
41-
document
42-
);
40+
return getLanguageServiceForPath(document.getFilePath() || '', workspaceUris, docContext);
4341
}
4442

4543
export function getService(
@@ -130,23 +128,34 @@ export function createLanguageService(
130128
snapshotManager.delete(filePath);
131129
}
132130

133-
function updateDocument(document: Document): ts.LanguageService {
134-
const preSnapshot = snapshotManager.get(document.getFilePath()!);
131+
function updateDocument(documentOrFilePath: Document | string): DocumentSnapshot {
132+
const filePath =
133+
typeof documentOrFilePath === 'string'
134+
? documentOrFilePath
135+
: documentOrFilePath.getFilePath() || '';
136+
const document = typeof documentOrFilePath === 'string' ? undefined : documentOrFilePath;
137+
const prevSnapshot = snapshotManager.get(filePath);
135138

136139
// Don't reinitialize document if no update needed.
137-
if (preSnapshot?.version === document.version) {
138-
return languageService;
140+
if (document && prevSnapshot?.version === document.version) {
141+
return prevSnapshot;
139142
}
140143

141-
const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig);
142-
if (preSnapshot && preSnapshot.scriptKind !== newSnapshot.scriptKind) {
144+
const newSnapshot = document
145+
? DocumentSnapshot.fromDocument(document, transformationConfig)
146+
: DocumentSnapshot.fromFilePath(
147+
filePath,
148+
docContext.createDocument,
149+
transformationConfig
150+
);
151+
if (prevSnapshot && prevSnapshot.scriptKind !== newSnapshot.scriptKind) {
143152
// Restart language service as it doesn't handle script kind changes.
144153
languageService.dispose();
145154
languageService = ts.createLanguageService(host);
146155
}
147156

148-
snapshotManager.set(document.getFilePath()!, newSnapshot);
149-
return languageService;
157+
snapshotManager.set(filePath, newSnapshot);
158+
return newSnapshot;
150159
}
151160

152161
function getSnapshot(fileName: string): DocumentSnapshot {
@@ -157,16 +166,11 @@ export function createLanguageService(
157166
return doc;
158167
}
159168

160-
if (isSvelteFilePath(fileName)) {
161-
const file = ts.sys.readFile(fileName) || '';
162-
doc = DocumentSnapshot.fromDocument(
163-
docContext.createDocument(fileName, file),
164-
transformationConfig
165-
);
166-
} else {
167-
doc = DocumentSnapshot.fromFilePath(fileName, transformationConfig);
168-
}
169-
169+
doc = DocumentSnapshot.fromFilePath(
170+
fileName,
171+
docContext.createDocument,
172+
transformationConfig
173+
);
170174
snapshotManager.set(fileName, doc);
171175
return doc;
172176
}

0 commit comments

Comments
 (0)