Skip to content

Commit d5b1f7b

Browse files
authored
feat: pull diagnostics support (#2978)
This switches the diagnostics from push model (server to client) to the pull model when the client supports it. The advantage is that the client can prioritise sending the diagnostic request for the visible tab. With this, we can check the document split to the side immediately and don't need to check all open files after a file save. The pull mode has a way to tell the client that the diagnostics haven't changed. I implement this mostly because VSCode sends another diagnostic request for the same files when switching files in the editor, even if the file isn't changed. I also lowered the debounce delay in the push-mode diagnostics. I've been wondering if some of the performance complaints are actually just because the artificial delay is too long for some people. The pull mode also has a much lower artificial delay. I think we can give it a try. If people are complaining about it, we can switch to an approach similar to the "smart" mode.
1 parent 77578ed commit d5b1f7b

File tree

11 files changed

+434
-23
lines changed

11 files changed

+434
-23
lines changed

.changeset/purple-lions-fold.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte-language-server': patch
3+
---
4+
5+
feat: pull diagnostics support

packages/language-server/src/lib/DiagnosticsManager.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,14 @@ export type GetDiagnostics = (
1414
cancellationToken?: CancellationToken
1515
) => Thenable<Diagnostic[]>;
1616

17-
export class DiagnosticsManager {
17+
export interface DiagnosticsManager {
18+
scheduleUpdate(document: Document): void;
19+
scheduleUpdateAll(): void;
20+
removeDiagnostics(document: Document): void;
21+
cancelStarted(uri: string): void;
22+
}
23+
24+
export class PushDiagnosticsManager implements DiagnosticsManager {
1825
constructor(
1926
private sendDiagnostics: SendDiagnostics,
2027
private docManager: DocumentManager,
@@ -93,5 +100,39 @@ export class DiagnosticsManager {
93100
this.update(doc);
94101
});
95102
this.pendingUpdates.clear();
96-
}, 700);
103+
}, 500);
104+
}
105+
106+
export class PullDiagnosticsManager implements DiagnosticsManager {
107+
constructor(
108+
private sendDiagnostics: SendDiagnostics,
109+
private sendRefreshDiagnostics: () => void
110+
) {}
111+
112+
private refreshTimeout: NodeJS.Timeout | null = null;
113+
114+
scheduleUpdate(): void {
115+
// No-op, diagnostics are pulled by the client.
116+
}
117+
118+
scheduleUpdateAll(): void {
119+
if (this.refreshTimeout) {
120+
clearTimeout(this.refreshTimeout);
121+
}
122+
this.refreshTimeout = setTimeout(() => {
123+
this.sendRefreshDiagnostics();
124+
this.refreshTimeout = null;
125+
}, 500);
126+
}
127+
128+
removeDiagnostics(document: Document): void {
129+
this.sendDiagnostics({
130+
uri: document.getURL(),
131+
diagnostics: []
132+
});
133+
}
134+
135+
cancelStarted(): void {
136+
// No-op, diagnostics are pulled by the client.
137+
}
97138
}

packages/language-server/src/plugins/PluginHost.ts

Lines changed: 96 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ import {
3535
WorkspaceEdit,
3636
InlayHint,
3737
WorkspaceSymbol,
38-
DocumentSymbol
38+
DocumentSymbol,
39+
DocumentDiagnosticReport
3940
} from 'vscode-languageserver';
40-
import { DocumentManager, getNodeIfIsInHTMLStartTag } from '../lib/documents';
41+
import { Document, DocumentManager, getNodeIfIsInHTMLStartTag } from '../lib/documents';
4142
import { Logger } from '../logger';
4243
import { isNotNullOrUndefined, regexLastIndexOf } from '../utils';
4344
import {
@@ -85,15 +86,7 @@ export class PluginHost implements LSProvider, OnWatchFileChanges {
8586
): Promise<Diagnostic[]> {
8687
const document = this.getDocument(textDocument.uri);
8788

88-
if (
89-
(document.getFilePath()?.includes('/node_modules/') ||
90-
document.getFilePath()?.includes('\\node_modules\\')) &&
91-
// Sapper convention: Put stuff inside node_modules below src
92-
!(
93-
document.getFilePath()?.includes('/src/node_modules/') ||
94-
document.getFilePath()?.includes('\\src\\node_modules\\')
95-
)
96-
) {
89+
if (this.canSkipDiagnostics(document)) {
9790
// Don't return diagnostics for files inside node_modules. These are considered read-only (cannot be changed)
9891
// and in case of svelte-check they would pollute/skew the output
9992
return [];
@@ -109,6 +102,98 @@ export class PluginHost implements LSProvider, OnWatchFileChanges {
109102
).flat();
110103
}
111104

105+
private canSkipDiagnostics(document: Document) {
106+
return (
107+
(document.getFilePath()?.includes('/node_modules/') ||
108+
document.getFilePath()?.includes('\\node_modules\\')) &&
109+
// Sapper convention: Put stuff inside node_modules below src
110+
!(
111+
document.getFilePath()?.includes('/src/node_modules/') ||
112+
document.getFilePath()?.includes('\\src\\node_modules\\')
113+
)
114+
);
115+
}
116+
117+
async getDiagnosticsForPullMode(
118+
textDocument: TextDocumentIdentifier,
119+
previousResultId: string | undefined,
120+
cancellationToken?: CancellationToken
121+
): Promise<DocumentDiagnosticReport> {
122+
const document = this.getDocument(textDocument.uri);
123+
let previousResultIdData: Record<string, string> = {};
124+
if (previousResultId) {
125+
try {
126+
previousResultIdData = JSON.parse(previousResultId);
127+
} catch (error) {}
128+
}
129+
130+
if (this.canSkipDiagnostics(document)) {
131+
// Don't return diagnostics for files inside node_modules. These are considered read-only (cannot be changed)
132+
return {
133+
kind: 'full',
134+
items: []
135+
};
136+
}
137+
138+
const plugins = this.plugins.filter(
139+
(plugin) => typeof plugin.getDiagnosticsForPullMode === 'function'
140+
);
141+
142+
const results = await Promise.all(
143+
plugins.map(
144+
async (plugin): Promise<[string, DocumentDiagnosticReport]> => [
145+
plugin.__name,
146+
await this.tryExecutePlugin(
147+
plugin,
148+
'getDiagnosticsForPullMode',
149+
[document, previousResultIdData[plugin.__name], cancellationToken],
150+
{ kind: 'full', items: [] }
151+
)
152+
]
153+
)
154+
);
155+
156+
const newResultId = JSON.stringify(
157+
Object.fromEntries(results.map(([name, res]) => [name, res.resultId]))
158+
);
159+
const unchanged = results
160+
.filter(([, res]) => res.kind === 'unchanged')
161+
.map(([name]) => name);
162+
const fullResults = results.flatMap(([, res]) => (res.kind === 'full' ? res.items : []));
163+
164+
if (unchanged.length === plugins.length) {
165+
return {
166+
kind: 'unchanged',
167+
resultId: newResultId
168+
};
169+
}
170+
if (unchanged.length === 0) {
171+
return {
172+
kind: 'full',
173+
resultId: newResultId,
174+
items: fullResults
175+
};
176+
}
177+
const unchangedPlugins = plugins.filter((plugin) => unchanged.includes(plugin.__name));
178+
const unchangedResults = await Promise.all(
179+
unchangedPlugins.map(async (plugin) => {
180+
const result = await this.tryExecutePlugin(
181+
plugin,
182+
'getDiagnostics',
183+
[document, cancellationToken],
184+
[]
185+
);
186+
return result;
187+
})
188+
);
189+
190+
return {
191+
kind: 'full',
192+
resultId: newResultId,
193+
items: fullResults.concat(unchangedResults.flat())
194+
};
195+
}
196+
112197
async doHover(textDocument: TextDocumentIdentifier, position: Position): Promise<Hover | null> {
113198
const document = this.getDocument(textDocument.uri);
114199

packages/language-server/src/plugins/css/CSSPlugin.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import {
1515
CompletionItemKind,
1616
SelectionRange,
1717
DocumentHighlight,
18-
WorkspaceFolder
18+
WorkspaceFolder,
19+
FullDocumentDiagnosticReport,
20+
UnchangedDocumentDiagnosticReport
1921
} from 'vscode-languageserver';
2022
import {
2123
Document,
@@ -144,6 +146,32 @@ export class CSSPlugin
144146
.map((diagnostic) => mapObjWithRangeToOriginal(cssDocument, diagnostic));
145147
}
146148

149+
getDiagnosticsForPullMode(
150+
document: Document,
151+
previousResultId: string | undefined
152+
): FullDocumentDiagnosticReport | UnchangedDocumentDiagnosticReport {
153+
if (!this.featureEnabled('diagnostics')) {
154+
return {
155+
kind: 'full',
156+
items: []
157+
};
158+
}
159+
160+
const resultId = document.version.toString();
161+
if (previousResultId === resultId) {
162+
return {
163+
kind: 'unchanged',
164+
resultId
165+
};
166+
}
167+
const diagnostics = this.getDiagnostics(document);
168+
return {
169+
kind: 'full',
170+
items: diagnostics,
171+
resultId
172+
};
173+
}
174+
147175
doHover(document: Document, position: Position): Hover | null {
148176
if (!this.featureEnabled('hover')) {
149177
return null;

packages/language-server/src/plugins/interfaces.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
CancellationToken,
33
CompletionContext,
4+
DocumentDiagnosticReport,
45
FileChangeType,
56
LinkedEditingRanges,
67
SemanticTokens,
@@ -52,6 +53,11 @@ export interface AppCompletionList<T extends TextDocumentIdentifier = any> exten
5253

5354
export interface DiagnosticsProvider {
5455
getDiagnostics(document: Document): Resolvable<Diagnostic[]>;
56+
getDiagnosticsForPullMode(
57+
document: Document,
58+
previousResultId?: string,
59+
cancellationToken?: CancellationToken
60+
): Resolvable<DocumentDiagnosticReport>;
5561
}
5662

5763
export interface HoverProvider {

packages/language-server/src/plugins/svelte/SveltePlugin.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
CompletionContext,
88
CompletionList,
99
Diagnostic,
10+
DocumentDiagnosticReport,
1011
FormattingOptions,
1112
Hover,
1213
Position,
@@ -107,6 +108,33 @@ export class SveltePlugin
107108
);
108109
}
109110

111+
async getDiagnosticsForPullMode(
112+
document: Document,
113+
previousResultId: string | undefined,
114+
cancellationToken?: CancellationToken
115+
): Promise<DocumentDiagnosticReport> {
116+
if (!this.featureEnabled('diagnostics')) {
117+
return {
118+
kind: 'full',
119+
items: []
120+
};
121+
}
122+
123+
const resultId = document.version.toString();
124+
if (previousResultId === resultId) {
125+
return {
126+
kind: 'unchanged',
127+
resultId
128+
};
129+
}
130+
const diagnostics = await this.getDiagnostics(document, cancellationToken);
131+
return {
132+
kind: 'full',
133+
items: diagnostics,
134+
resultId
135+
};
136+
}
137+
110138
async getCompiledResult(document: Document): Promise<SvelteCompileResult | null> {
111139
try {
112140
const svelteDoc = await this.getSvelteDoc(document);

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
CompletionList,
1212
DefinitionLink,
1313
Diagnostic,
14+
DocumentDiagnosticReport,
1415
DocumentHighlight,
1516
FileChangeType,
1617
FoldingRange,
@@ -236,6 +237,25 @@ export class TypeScriptPlugin
236237
return this.diagnosticsProvider.getDiagnostics(document, cancellationToken);
237238
}
238239

240+
async getDiagnosticsForPullMode(
241+
document: Document,
242+
previousResultId?: string,
243+
cancellationToken?: CancellationToken
244+
): Promise<DocumentDiagnosticReport> {
245+
if (!this.featureEnabled('diagnostics')) {
246+
return {
247+
kind: 'full',
248+
items: []
249+
};
250+
}
251+
252+
return this.diagnosticsProvider.getDiagnosticsForPullMode(
253+
document,
254+
previousResultId,
255+
cancellationToken
256+
);
257+
}
258+
239259
async doHover(document: Document, position: Position): Promise<Hover | null> {
240260
if (!this.featureEnabled('hover')) {
241261
return null;

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import ts, { flattenDiagnosticMessageText } from 'typescript';
2-
import { CancellationToken, Diagnostic, DiagnosticSeverity, Range } from 'vscode-languageserver';
2+
import {
3+
CancellationToken,
4+
Diagnostic,
5+
DiagnosticSeverity,
6+
DocumentDiagnosticReport,
7+
Range
8+
} from 'vscode-languageserver';
39
import {
410
Document,
511
getNodeIfIsInStartTag,
@@ -105,6 +111,35 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
105111
return mapAndFilterDiagnostics(diagnostics, document, tsDoc, lang);
106112
}
107113

114+
async getDiagnosticsForPullMode(
115+
document: Document,
116+
previousResultId?: string,
117+
cancellationToken?: CancellationToken
118+
): Promise<DocumentDiagnosticReport> {
119+
const { tsDoc, lsContainer } = await this.getLSAndTSDoc(document);
120+
if (cancellationToken?.isCancellationRequested) {
121+
return {
122+
kind: 'full',
123+
items: []
124+
};
125+
}
126+
const resultId = tsDoc.version.toString() + '-' + lsContainer.getProjectVersion();
127+
128+
if (previousResultId === resultId) {
129+
return {
130+
kind: 'unchanged',
131+
resultId
132+
};
133+
}
134+
135+
const diagnostics = await this.getDiagnostics(document);
136+
return {
137+
kind: 'full',
138+
resultId,
139+
items: diagnostics
140+
};
141+
}
142+
108143
private async getLSAndTSDoc(document: Document) {
109144
return this.lsAndTsDocResolver.getLSAndTSDoc(document);
110145
}

0 commit comments

Comments
 (0)