Skip to content

Commit 862453c

Browse files
committed
perf: rework snapshot hot paths
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. This was originally using `split` to "parse" the cache key into a file/specifier pair. We don't actually need to do this as we know there's always one occurrence of `CACHE_KEY_SEPARATOR`. So we can instead just `slice` on the index of the separator. Additionally, we only compute any of this the first time we encounter an unresolved cache entry. I then noticed that `fileNameWithoutEnding` could be `a` (from `a.svelte`), for example. This then meant we would delete all cache entries with specifiers containing `a` (since we did `specifier.includes(fileNameWithoutEnding)`). I have changed this to explicitly test for these two cases instead: - Exact match (i.e. the specifier is exactly `fileNameWithoutEnding`) - Path-like string includes specifier (`/{fileNameWithoutEnding}.`) For example, `a.svelte` would become `a`. We then want to check for `/a.` to be able to match `/a.svelte`, `/a.svelte.js`, etc. Nobody can import it without a `/` preceding it since then it'd be a bare specifier. We currently key by file name and specifier. It shouldn't be possible for two sibling files to resolve the same specifier to two different modules. So it seems like we should be able to key by `dirname(path)` instead. This means we would cache resolution for entire directories rather than re-computing it for every file in that directory. This now uses `slice` on the last separator rather than splitting only to retrieve the last part.
1 parent dec37ea commit 862453c

File tree

3 files changed

+35
-16
lines changed

3 files changed

+35
-16
lines changed

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ 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+
81+
7682
export namespace DocumentSnapshot {
7783
/**
7884
* Returns a svelte snapshot from a svelte document.
@@ -121,30 +127,25 @@ export namespace DocumentSnapshot {
121127
* @param options options that apply in case it's a svelte file
122128
*/
123129
export function fromNonSvelteFilePath(filePath: string, tsSystem: ts.System) {
124-
let originalText = '';
125-
126130
// The following (very hacky) code makes sure that the ambient module definitions
127131
// that tell TS "every import ending with .svelte is a valid module" are removed.
128132
// They exist in svelte2tsx and svelte to make sure that people don't
129133
// get errors in their TS files when importing Svelte files and not using our TS plugin.
130134
// If someone wants to get back the behavior they can add an ambient module definition
131135
// on their own.
132-
const normalizedPath = filePath.replace(/\\/g, '/');
133-
if (!normalizedPath.endsWith('node_modules/svelte/types/runtime/ambient.d.ts')) {
136+
let originalText = '';
137+
if (!ambientPathPattern.test(filePath)) {
134138
originalText = tsSystem.readFile(filePath) || '';
135139
}
136140

137-
if (normalizedPath.endsWith('node_modules/svelte/types/index.d.ts')) {
141+
if (svelteTypesPattern.test(filePath)) {
138142
const startIdx = originalText.indexOf(`declare module '*.svelte' {`);
139143
const endIdx = originalText.indexOf(`\n}`, startIdx + 1) + 2;
140144
originalText =
141145
originalText.substring(0, startIdx) +
142146
' '.repeat(endIdx - startIdx) +
143147
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-
) {
148+
} else if (shimsPattern.test(filePath)) {
148149
// If not present, the LS uses an older version of svelte2tsx
149150
if (originalText.includes('// -- start svelte-ls-remove --')) {
150151
originalText =
@@ -157,7 +158,7 @@ export namespace DocumentSnapshot {
157158
}
158159

159160
const declarationExtensions = [ts.Extension.Dcts, ts.Extension.Dts, ts.Extension.Dmts];
160-
if (declarationExtensions.some((ext) => normalizedPath.endsWith(ext))) {
161+
if (declarationExtensions.some((ext) => filePath.endsWith(ext))) {
161162
return new DtsDocumentSnapshot(INITIAL_VERSION, filePath, originalText, tsSystem);
162163
}
163164

packages/language-server/src/plugins/typescript/module-loader.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import ts from 'typescript';
2+
import path from 'node:path';
23
import { FileMap, FileSet } from '../../lib/documents/fileCollection';
34
import { createGetCanonicalFileName, getLastPartOfPath, toFileNameLowerCase } from '../../utils';
45
import { DocumentSnapshot } from './DocumentSnapshot';
@@ -60,22 +61,36 @@ class ModuleResolutionCache {
6061
* and which might match the path.
6162
*/
6263
deleteUnresolvedResolutionsFromCache(path: string): void {
63-
const fileNameWithoutEnding =
64-
getLastPartOfPath(this.getCanonicalFileName(path)).split('.').shift() || '';
64+
let fileNameWithoutEnding: string | undefined;
6565
this.cache.forEach((val, key) => {
6666
if (val) {
6767
return;
6868
}
69-
const [containingFile, moduleName = ''] = key.split(CACHE_KEY_SEPARATOR);
70-
if (moduleName.includes(fileNameWithoutEnding)) {
69+
if (fileNameWithoutEnding === undefined) {
70+
const canonicalPath = this.getCanonicalFileName(path);
71+
const lastPart = getLastPartOfPath(canonicalPath);
72+
const dotIndex = lastPart.indexOf('.');
73+
fileNameWithoutEnding = dotIndex === -1 ? lastPart : lastPart.slice(0, dotIndex);
74+
}
75+
const separatorIndex = key.indexOf(CACHE_KEY_SEPARATOR);
76+
const filePart = key.slice(separatorIndex + CACHE_KEY_SEPARATOR.length);
77+
if (
78+
filePart === fileNameWithoutEnding ||
79+
filePart.includes(`/${fileNameWithoutEnding}.`)
80+
) {
81+
const containingFile = key.slice(0, separatorIndex);
7182
this.cache.delete(key);
7283
this.pendingInvalidations.add(containingFile);
7384
}
7485
});
7586
}
7687

7788
private getKey(moduleName: string, containingFile: string) {
78-
return containingFile + CACHE_KEY_SEPARATOR + ensureRealSvelteFilePath(moduleName);
89+
return (
90+
path.dirname(containingFile) +
91+
CACHE_KEY_SEPARATOR +
92+
ensureRealSvelteFilePath(moduleName)
93+
);
7994
}
8095

8196
clearPendingInvalidations() {

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)