Skip to content

Commit 69e258e

Browse files
43081jdummdidumm
43081j
and
dummdidumm
authored
perf: rework snapshot hot paths (#2852)
* perf: rework snapshot hot paths **`fromNonSveltePath`** This function was originally using `replace` to unix-ify paths before then testing against them with `endsWith`. We can instead use static patterns on the unaltered path regardless of separator. **`getLastPartOfPath`** This now uses `slice` on the last separator rather than splitting only to retrieve the last part. * perf: avoid garbage collection on diagnostics Reverts the `getKey` change and updates diagnostics iteration to avoid allocating an array we later discard. * Tweak snapshot hot paths for performance * chore: make linter happy --------- Co-authored-by: Simon H <[email protected]>
1 parent ec7be4b commit 69e258e

File tree

4 files changed

+76
-66
lines changed

4 files changed

+76
-66
lines changed

‎.changeset/cool-chefs-compete.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'svelte-language-server': patch
3+
'svelte-check': patch
4+
---
5+
6+
perf: tweak some snapshot hot paths

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ export interface SvelteSnapshotOptions {
7373
typingsNamespace: string;
7474
}
7575

76+
const ambientPathPattern = /node_modules[\/\\]svelte[\/\\]types[\/\\]ambient\.d\.ts$/;
77+
const svelteTypesPattern = /node_modules[\/\\]svelte[\/\\]types[\/\\]index\.d\.ts$/;
78+
const shimsPattern =
79+
/svelte2tsx[\/\\]svelte-shims\.d\.ts$|svelte-check[\/\\]dist[\/\\]src[\/\\]svelte-shims\.d\.ts$/;
80+
7681
export namespace DocumentSnapshot {
7782
/**
7883
* Returns a svelte snapshot from a svelte document.
@@ -121,30 +126,25 @@ export namespace DocumentSnapshot {
121126
* @param options options that apply in case it's a svelte file
122127
*/
123128
export function fromNonSvelteFilePath(filePath: string, tsSystem: ts.System) {
124-
let originalText = '';
125-
126129
// The following (very hacky) code makes sure that the ambient module definitions
127130
// that tell TS "every import ending with .svelte is a valid module" are removed.
128131
// They exist in svelte2tsx and svelte to make sure that people don't
129132
// get errors in their TS files when importing Svelte files and not using our TS plugin.
130133
// If someone wants to get back the behavior they can add an ambient module definition
131134
// on their own.
132-
const normalizedPath = filePath.replace(/\\/g, '/');
133-
if (!normalizedPath.endsWith('node_modules/svelte/types/runtime/ambient.d.ts')) {
135+
let originalText = '';
136+
if (!ambientPathPattern.test(filePath)) {
134137
originalText = tsSystem.readFile(filePath) || '';
135138
}
136139

137-
if (normalizedPath.endsWith('node_modules/svelte/types/index.d.ts')) {
140+
if (svelteTypesPattern.test(filePath)) {
138141
const startIdx = originalText.indexOf(`declare module '*.svelte' {`);
139142
const endIdx = originalText.indexOf(`\n}`, startIdx + 1) + 2;
140143
originalText =
141144
originalText.substring(0, startIdx) +
142145
' '.repeat(endIdx - startIdx) +
143146
originalText.substring(endIdx);
144-
} else if (
145-
normalizedPath.endsWith('svelte2tsx/svelte-shims.d.ts') ||
146-
normalizedPath.endsWith('svelte-check/dist/src/svelte-shims.d.ts')
147-
) {
147+
} else if (shimsPattern.test(filePath)) {
148148
// If not present, the LS uses an older version of svelte2tsx
149149
if (originalText.includes('// -- start svelte-ls-remove --')) {
150150
originalText =
@@ -157,7 +157,7 @@ export namespace DocumentSnapshot {
157157
}
158158

159159
const declarationExtensions = [ts.Extension.Dcts, ts.Extension.Dts, ts.Extension.Dmts];
160-
if (declarationExtensions.some((ext) => normalizedPath.endsWith(ext))) {
160+
if (declarationExtensions.some((ext) => filePath.endsWith(ext))) {
161161
return new DtsDocumentSnapshot(INITIAL_VERSION, filePath, originalText, tsSystem);
162162
}
163163

‎packages/language-server/src/svelte-check.ts‎

Lines changed: 56 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -252,66 +252,67 @@ export class SvelteCheck {
252252
const isKitFile = snapshot?.kitFile ?? false;
253253
const diagnostics: Diagnostic[] = [];
254254
if (!skipDiagnosticsForFile) {
255-
const originalDiagnostics = [
256-
...lang.getSyntacticDiagnostics(file.fileName),
257-
...lang.getSuggestionDiagnostics(file.fileName),
258-
...lang.getSemanticDiagnostics(file.fileName)
259-
];
260-
261-
for (let diagnostic of originalDiagnostics) {
262-
if (!diagnostic.start || !diagnostic.length || !isKitFile) {
263-
diagnostics.push(map(diagnostic));
264-
continue;
265-
}
255+
const diagnosticSources = [
256+
'getSyntacticDiagnostics',
257+
'getSuggestionDiagnostics',
258+
'getSemanticDiagnostics'
259+
] as const;
260+
for (const diagnosticSource of diagnosticSources) {
261+
for (let diagnostic of lang[diagnosticSource](file.fileName)) {
262+
if (!diagnostic.start || !diagnostic.length || !isKitFile) {
263+
diagnostics.push(map(diagnostic));
264+
continue;
265+
}
266266

267-
let range: Range | undefined = undefined;
268-
const inGenerated = isInGeneratedCode(
269-
file.text,
270-
diagnostic.start,
271-
diagnostic.start + diagnostic.length
272-
);
273-
if (inGenerated && snapshot) {
274-
const pos = snapshot.getOriginalPosition(
275-
snapshot.positionAt(diagnostic.start)
267+
let range: Range | undefined = undefined;
268+
const inGenerated = isInGeneratedCode(
269+
file.text,
270+
diagnostic.start,
271+
diagnostic.start + diagnostic.length
276272
);
277-
range = {
278-
start: pos,
279-
end: {
280-
line: pos.line,
281-
// adjust length so it doesn't spill over to the next line
282-
character: pos.character + 1
283-
}
284-
};
285-
// If not one of the specific error messages then filter out
286-
if (diagnostic.code === 2307) {
287-
diagnostic = {
288-
...diagnostic,
289-
messageText:
290-
typeof diagnostic.messageText === 'string' &&
291-
diagnostic.messageText.includes('./$types')
292-
? diagnostic.messageText +
293-
` (this likely means that SvelteKit's type generation didn't run yet - try running it by executing 'npm run dev' or 'npm run build')`
294-
: diagnostic.messageText
273+
if (inGenerated && snapshot) {
274+
const pos = snapshot.getOriginalPosition(
275+
snapshot.positionAt(diagnostic.start)
276+
);
277+
range = {
278+
start: pos,
279+
end: {
280+
line: pos.line,
281+
// adjust length so it doesn't spill over to the next line
282+
character: pos.character + 1
283+
}
295284
};
296-
} else if (diagnostic.code === 2694) {
297-
diagnostic = {
298-
...diagnostic,
299-
messageText:
300-
typeof diagnostic.messageText === 'string' &&
301-
diagnostic.messageText.includes('/$types')
302-
? diagnostic.messageText +
303-
` (this likely means that SvelteKit's generated types are out of date - try rerunning it by executing 'npm run dev' or 'npm run build')`
304-
: diagnostic.messageText
305-
};
306-
} else if (
307-
diagnostic.code !==
308-
2355 /* A function whose declared type is neither 'void' nor 'any' must return a value */
309-
) {
310-
continue;
285+
// If not one of the specific error messages then filter out
286+
if (diagnostic.code === 2307) {
287+
diagnostic = {
288+
...diagnostic,
289+
messageText:
290+
typeof diagnostic.messageText === 'string' &&
291+
diagnostic.messageText.includes('./$types')
292+
? diagnostic.messageText +
293+
` (this likely means that SvelteKit's type generation didn't run yet - try running it by executing 'npm run dev' or 'npm run build')`
294+
: diagnostic.messageText
295+
};
296+
} else if (diagnostic.code === 2694) {
297+
diagnostic = {
298+
...diagnostic,
299+
messageText:
300+
typeof diagnostic.messageText === 'string' &&
301+
diagnostic.messageText.includes('/$types')
302+
? diagnostic.messageText +
303+
` (this likely means that SvelteKit's generated types are out of date - try rerunning it by executing 'npm run dev' or 'npm run build')`
304+
: diagnostic.messageText
305+
};
306+
} else if (
307+
diagnostic.code !==
308+
2355 /* A function whose declared type is neither 'void' nor 'any' must return a value */
309+
) {
310+
continue;
311+
}
311312
}
312-
}
313313

314-
diagnostics.push(map(diagnostic, range));
314+
diagnostics.push(map(diagnostic, range));
315+
}
315316
}
316317
}
317318

‎packages/language-server/src/utils.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ export function normalizeUri(uri: string): string {
6060
* (bar or bar.svelte in this example).
6161
*/
6262
export function getLastPartOfPath(path: string): string {
63-
return path.replace(/\\/g, '/').split('/').pop() || '';
63+
const lastSlash = path.lastIndexOf('/');
64+
const lastBackslash = path.lastIndexOf('\\');
65+
const lastSeparator = Math.max(lastSlash, lastBackslash);
66+
return lastSeparator === -1 ? path : path.slice(lastSeparator + 1);
6467
}
6568

6669
export function flatten<T>(arr: Array<T | T[]>): T[] {

0 commit comments

Comments
 (0)