Skip to content

Commit ddd51bf

Browse files
committed
perf: speed up ValidScopedCSSClass in the language server
Two changes that together drop the LSP save latency introduced by ValidScopedCSSClass from ~4.5s to <500ms on large themes after the first save of a session. 1. Session-scoped CSS class cache in runChecks Before, every save tore down the getCSSClassesForURI memoization and re-parsed every .liquid file with a {% stylesheet %} tag to rebuild the theme-wide class set. Now the Map<uri, Promise<Set<string>>> lives at makeRunChecks scope. When a save fires, the saved URI is evicted from the cache; every other entry stays warm. First save pays the full scan; subsequent saves re-parse one file. 2. Substring short-circuit in extractCSSClassesFromLiquidUri Skip toLiquidHtmlAST entirely when source contains no '{% stylesheet'. On Horizon (240 liquid files, 128 with stylesheet tags) this skips ~47% of parses on the first scan. Also helps the CLI: ~19.2s → ~17.4s on Horizon. Also adds [theme-check] timing logs in runChecks for profiling.
1 parent 54e1418 commit ddd51bf

3 files changed

Lines changed: 49 additions & 5 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@shopify/theme-check-common': patch
3+
'@shopify/theme-language-server-common': patch
4+
---
5+
6+
Reduce `ValidScopedCSSClass` save latency on large themes. Parsing of theme-wide stylesheets is now reused across saves within a language-server session and only invalidated for the file the user just edited. Also skips full syntax-tree parsing of Liquid files that contain no stylesheet tag at all. Fixes the save-lag reported in [#1179](https://github.com/Shopify/theme-tools/issues/1179).

packages/theme-check-common/src/utils/styles.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ export async function extractCSSClassesFromLiquidUri(
4141
const classes = new Set<string>();
4242
try {
4343
const source = await fs.readFile(uri);
44+
// Most liquid files have no {% stylesheet %} tag — skip the AST parse
45+
// entirely when the tag isn't present. Saves ~20ms/file on large themes.
46+
if (!/\{%-?\s+stylesheet/.test(source)) {
47+
return classes;
48+
}
4449
const ast = toLiquidHtmlAST(source);
4550
if (ast instanceof Error) return classes;
4651
const cssStrings = visit<SourceCodeType.LiquidHtml, string>(ast, {

packages/theme-language-server-common/src/diagnostics/runChecks.ts

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ export function makeRunChecks(
3636
'fs' | 'loadConfig' | 'themeDocset' | 'jsonValidationSet' | 'getMetafieldDefinitions'
3737
> & { cssLanguageService?: CSSLanguageService; themeGraphManager?: ThemeGraphManager },
3838
) {
39+
// Session-scoped cache of CSS classes per file URI. Living at the
40+
// makeRunChecks scope means each save only re-parses files that actually
41+
// changed — ValidScopedCSSClass's per-save full-theme scan drops from
42+
// O(files-with-stylesheet) parses to O(1) re-parse of the saved file.
43+
const cssClassCache = new Map<string, Promise<Set<string>>>();
44+
3945
return async function runChecks(triggerURIs: string[]): Promise<void> {
4046
// This function takes an array of triggerURIs so that we can correctly
4147
// recheck on file renames that came from out of bounds in a
@@ -46,23 +52,41 @@ export function makeRunChecks(
4652
// theme1/snippets/b.liquid
4753
//
4854
// then we recheck theme1
55+
const runStart = Date.now();
56+
console.error(
57+
`[theme-check] runChecks start — ${triggerURIs.length} trigger URI(s): ${triggerURIs
58+
.map((u) => u.split('/').pop())
59+
.join(', ')}`,
60+
);
61+
// Evict changed files from the session cache before running.
62+
for (const uri of triggerURIs) cssClassCache.delete(uri);
4963
const fileExists = makeFileExists(fs);
5064
const rootURIs = await Promise.all(triggerURIs.map((uri) => findRoot(uri, fileExists)));
5165
const deduplicatedRootURIs = new Set<string>(rootURIs.filter((x): x is string => !!x));
5266
await Promise.all([...deduplicatedRootURIs].map(runChecksForRoot));
67+
console.error(`[theme-check] runChecks done in ${Date.now() - runStart}ms`);
5368

5469
return;
5570

5671
async function runChecksForRoot(configFileRootUri: string) {
72+
const rootStart = Date.now();
5773
const config = await loadConfig(configFileRootUri, fs);
5874
const theme = documentManager.theme(config.rootUri);
75+
console.error(
76+
`[theme-check] root ${config.rootUri.split('/').slice(-2).join('/')}${
77+
theme.length
78+
} file(s)`,
79+
);
5980

81+
const cssStart = Date.now();
6082
const cssOffenses = cssLanguageService
6183
? await Promise.all(
6284
theme.map((sourceCode) => getCSSDiagnostics(cssLanguageService, sourceCode)),
6385
).then((offenses) => offenses.flat())
6486
: [];
87+
console.error(`[theme-check] cssDiagnostics: ${Date.now() - cssStart}ms`);
6588

89+
const checkStart = Date.now();
6690
const themeOffenses = await check(theme, config, {
6791
fs,
6892
themeDocset,
@@ -79,11 +103,14 @@ export function makeRunChecks(
79103
return themeGraphManager.getDependencies(uri);
80104
},
81105

82-
async getCSSClassesForURI(uri: string): Promise<Set<string>> {
83-
if (uri.endsWith('.css')) {
84-
return extractCSSClassesFromAssetUri(uri, fs);
85-
}
86-
return extractCSSClassesFromLiquidUri(uri, fs);
106+
getCSSClassesForURI(uri: string): Promise<Set<string>> {
107+
const cached = cssClassCache.get(uri);
108+
if (cached) return cached;
109+
const promise = uri.endsWith('.css')
110+
? extractCSSClassesFromAssetUri(uri, fs)
111+
: extractCSSClassesFromLiquidUri(uri, fs);
112+
cssClassCache.set(uri, promise);
113+
return promise;
87114
},
88115

89116
// TODO should do something for app blocks?
@@ -112,6 +139,11 @@ export function makeRunChecks(
112139
return doc.getLiquidDoc();
113140
},
114141
});
142+
console.error(
143+
`[theme-check] check() (all rules): ${Date.now() - checkStart}ms — ${
144+
themeOffenses.length
145+
} offense(s)`,
146+
);
115147
const offenses = [...themeOffenses, ...cssOffenses];
116148

117149
// We iterate over the theme files (as opposed to offenses) because if
@@ -121,6 +153,7 @@ export function makeRunChecks(
121153
const sourceCodeOffenses = offenses.filter((offense) => offense.uri === sourceCode.uri);
122154
diagnosticsManager.set(sourceCode.uri, sourceCode.version, sourceCodeOffenses);
123155
}
156+
console.error(`[theme-check] root done in ${Date.now() - rootStart}ms`);
124157
}
125158
};
126159
}

0 commit comments

Comments
 (0)