From aecd791d43938e0ef32d58f1ee6403a418e7136b Mon Sep 17 00:00:00 2001 From: fedifazzy Date: Thu, 22 Oct 2026 13:10:29 +0200 Subject: [PATCH 1/3] test(server): await prettier in collect tests and run them in CI prettier 3 formats asynchronously, so every prettified snapshot was compared with a pending Promise and 16 of 22 snapshots failed. Nobody noticed because @linaria/server had no test script, so `turbo run test` skipped the package. Use the standalone build of prettier with the postcss plugin, which also works without --experimental-vm-modules, await it, and add the test script the other packages use. One snapshot is re-serialized without escaped quotes by the current jest. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../__snapshots__/collect.test.ts.snap | 4 +-- packages/server/__tests__/collect.test.ts | 28 ++++++++++++------- packages/server/package.json | 1 + 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/server/__tests__/__snapshots__/collect.test.ts.snap b/packages/server/__tests__/__snapshots__/collect.test.ts.snap index 18615477b..ff5f6c4fb 100644 --- a/packages/server/__tests__/__snapshots__/collect.test.ts.snap +++ b/packages/server/__tests__/__snapshots__/collect.test.ts.snap @@ -120,7 +120,7 @@ exports[`collects complex css critical 1`] = ` .linaria::before, .linaria::after { - content: \\"\\"; + content: ""; object-fit: contain; } } @@ -146,7 +146,7 @@ exports[`collects complex css critical 1`] = ` .lily { color: #fff; } -[data-theme=\\"dark\\"] .lily { +[data-theme="dark"] .lily { color: #000; } .linaria ~ div { diff --git a/packages/server/__tests__/collect.test.ts b/packages/server/__tests__/collect.test.ts index a25564ba6..64f3f533e 100644 --- a/packages/server/__tests__/collect.test.ts +++ b/packages/server/__tests__/collect.test.ts @@ -1,14 +1,19 @@ import dedent from 'dedent'; -import prettier from 'prettier'; +import * as postcssPlugin from 'prettier/plugins/postcss'; +import { format } from 'prettier/standalone'; import collect from '../src/collect'; -const prettyPrint = (src: string) => prettier.format(src, { parser: 'scss' }); +// prettier 3 formats asynchronously; the standalone build with an explicit plugin also runs without +// --experimental-vm-modules +const prettyPrint = (src: string) => + format(src, { parser: 'scss', plugins: [postcssPlugin] }); const testCollect = (html: string, css: string) => { const { critical, other } = collect(html, css); - test('critical', () => expect(prettyPrint(critical)).toMatchSnapshot()); - test('other', () => expect(prettyPrint(other)).toMatchSnapshot()); + test('critical', async () => + expect(await prettyPrint(critical)).toMatchSnapshot()); + test('other', async () => expect(await prettyPrint(other)).toMatchSnapshot()); }; const html = dedent` @@ -195,8 +200,9 @@ describe('works with global css', () => { const { critical, other } = collect(html, css); - test('critical', () => expect(prettyPrint(critical)).toMatchSnapshot()); - test('other', () => expect(prettyPrint(other)).toMatchSnapshot()); + test('critical', async () => + expect(await prettyPrint(critical)).toMatchSnapshot()); + test('other', async () => expect(await prettyPrint(other)).toMatchSnapshot()); }); describe('handles top-level @font-face', () => { @@ -210,8 +216,9 @@ describe('handles top-level @font-face', () => { `; const { critical, other } = collect(html, css); - test('critical', () => expect(prettyPrint(critical)).toMatchSnapshot()); - test('other', () => expect(prettyPrint(other)).toMatchSnapshot()); + test('critical', async () => + expect(await prettyPrint(critical)).toMatchSnapshot()); + test('other', async () => expect(await prettyPrint(other)).toMatchSnapshot()); }); // there was a bug when the whole atrule was included for each child rule @@ -231,8 +238,9 @@ describe('include atrule once', () => { `; const { critical, other } = collect(html, css); - test('critical', () => expect(prettyPrint(critical)).toMatchSnapshot()); - test('other', () => expect(prettyPrint(other)).toMatchSnapshot()); + test('critical', async () => + expect(await prettyPrint(critical)).toMatchSnapshot()); + test('other', async () => expect(await prettyPrint(other)).toMatchSnapshot()); }); describe('ignore empty class attribute', () => { diff --git a/packages/server/package.json b/packages/server/package.json index 7958f1fcd..a03608fc2 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -26,6 +26,7 @@ "build:declarations": "tsc --emitDeclarationOnly --outDir types", "build:esm": "babel src --out-dir esm --extensions '.js,.jsx,.ts,.tsx' --source-maps --delete-dir-on-start", "build:lib": "cross-env NODE_ENV=legacy babel src --out-dir lib --extensions '.js,.jsx,.ts,.tsx' --source-maps --delete-dir-on-start", + "test": "jest --config ../../jest.config.js --rootDir .", "typecheck": "tsc --noEmit --composite false", "watch": "pnpm build:lib --watch & pnpm build:esm --watch & pnpm build:declarations --watch" }, From e7ecd640a5fbe1ac9a72f22e7980ff99f357351e Mon Sep 17 00:00:00 2001 From: fedifazzy Date: Thu, 22 Oct 2026 13:16:48 +0200 Subject: [PATCH 2/3] fix(server): make collect linear in the number of rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit collect moved every top-level rule of the parsed stylesheet into `critical` or `other` with `append`. For a node with a parent, `append` removes it with `splice`, and the moved rule was always at index 0, so one call was O(n²) in the number of top-level rules: about 0.2 s for a 1.6 MB stylesheet on Node 20 and seconds while incremental marking is active. Rules are now collected first. The stylesheet is detached at once with `removeAll`, and only then are the nodes appended. Handled top-level at-rules are no longer spliced out one by one. Two bugs found along the way: - `other` dropped the non-critical rules of an at-rule and repeated its critical rules whenever a non-critical rule came first. The `index - removedNodesFromOther` lookup removed the wrong child. Both copies of the at-rule are now filtered by `isCritical` directly. - `blockedClasses` let every second matching rule into `critical`, because `test` on a regexp with the `g` flag keeps `lastIndex` between selectors. The class regexps no longer use `g`, and the classes from the HTML are deduplicated. Whitespace between rules in the output now follows the source stylesheet. Previously the rules were glued together by the removals. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/server-collect-linear.md | 12 +++ .../__snapshots__/collect.test.ts.snap | 16 ++++ packages/server/__tests__/collect.test.ts | 68 +++++++++++++++- packages/server/src/collect.ts | 77 +++++++++++++++---- 4 files changed, 154 insertions(+), 19 deletions(-) create mode 100644 .changeset/server-collect-linear.md diff --git a/.changeset/server-collect-linear.md b/.changeset/server-collect-linear.md new file mode 100644 index 000000000..69cbaf1a7 --- /dev/null +++ b/.changeset/server-collect-linear.md @@ -0,0 +1,12 @@ +--- +'@linaria/server': patch +--- + +`collect` no longer moves rules out of the parsed stylesheet one by one. That made it quadratic in the number of top-level rules and took seconds on large stylesheets. + +It also fixes two bugs: + +- `other` lost rules from an at-rule when a non-critical rule came before a critical one; +- `blockedClasses` let every second matching rule into `critical`. + +Whitespace between rules in the output now follows the source stylesheet. diff --git a/packages/server/__tests__/__snapshots__/collect.test.ts.snap b/packages/server/__tests__/__snapshots__/collect.test.ts.snap index ff5f6c4fb..6808f1335 100644 --- a/packages/server/__tests__/__snapshots__/collect.test.ts.snap +++ b/packages/server/__tests__/__snapshots__/collect.test.ts.snap @@ -108,11 +108,13 @@ exports[`collects complex css critical 1`] = ` ".lotus { vertical-align: top; } + @media (max-width: 1200px) { .lotus { vertical-align: bottom; } } + @supports (object-fit: contain) { .lotus { object-fit: contain; @@ -124,35 +126,45 @@ exports[`collects complex css critical 1`] = ` object-fit: contain; } } + .linaria { float: left; flex: 1; animation: custom-animation 0.2s; } + .linaria:hover { float: right; } + .linaria > span, .linaria + .linaria, .linaria ~ div { display: none; } + .linaria > span { display: none; } + .linaria::after { display: block; } + .lily { color: #fff; } + [data-theme="dark"] .lily { color: #000; } + .linaria ~ div { } + .linaria.linaria2 { } + @keyframes custom-animation { 0% { opacity: 0; @@ -266,14 +278,18 @@ exports[`works with global css critical 1`] = ` "body { font-size: 13.37px; } + html { -webkit-font-smoothing: antialiased; } + h1 { font-weight: bold; } + .linaria:active { } + .linaria::before { } " diff --git a/packages/server/__tests__/collect.test.ts b/packages/server/__tests__/collect.test.ts index 64f3f533e..0c0ce256f 100644 --- a/packages/server/__tests__/collect.test.ts +++ b/packages/server/__tests__/collect.test.ts @@ -1,4 +1,5 @@ import dedent from 'dedent'; +import postcss from 'postcss'; import * as postcssPlugin from 'prettier/plugins/postcss'; import { format } from 'prettier/standalone'; @@ -275,7 +276,10 @@ test('does not match selectors in ignoredClasses list', () => { ignoredClasses: ['ltr', 'dir'], }); expect(critical).toMatchInlineSnapshot(`".lily {}"`); - expect(other).toMatchInlineSnapshot(`".linaria.dir {}.linaria.ltr {}"`); + expect(other).toMatchInlineSnapshot(` + ".linaria.dir {} + .linaria.ltr {}" + `); }); test('does not match selectors in blockedClasses list', () => { @@ -290,7 +294,10 @@ test('does not match selectors in blockedClasses list', () => { `; const { critical, other } = collect(code, css, { blockedClasses: ['rtl'] }); - expect(critical).toMatchInlineSnapshot(`".linaria.ltr {}.lily {}"`); + expect(critical).toMatchInlineSnapshot(` + ".linaria.ltr {} + .lily {}" + `); expect(other).toMatchInlineSnapshot(`".linaria.rtl {}"`); }); @@ -331,3 +338,60 @@ test('does not match child selectors in blockedClasses list for media queries', }" `); }); + +const selectorsOf = (css: string) => { + const selectors: string[] = []; + postcss.parse(css).walkRules((rule) => { + selectors.push(rule.selector); + }); + return selectors; +}; + +test('splits rules of an at-rule between critical and other', () => { + const css = dedent` + @media (min-width: 600px) { + .unrelated { color: red; } + .linaria { color: blue; } + .unrelated2 { color: green; } + } + `; + + const { critical, other } = collect(html, css); + expect(selectorsOf(critical)).toEqual(['.linaria']); + expect(selectorsOf(other)).toEqual(['.unrelated', '.unrelated2']); +}); + +test('excludes every rule with a blocked class', () => { + const css = dedent` + .linaria.rtl { color: red; } + .linaria.rtl { color: blue; } + .linaria.rtl { color: green; } + .linaria { color: black; } + `; + + const { critical } = collect(html, css, { blockedClasses: ['rtl'] }); + expect(selectorsOf(critical)).toEqual(['.linaria']); +}); + +test('keeps the order of rules in a large stylesheet', () => { + const rules: string[] = []; + const expectedCritical: string[] = []; + const expectedOther: string[] = []; + for (let i = 0; i < 20000; i += 1) { + if (i % 1000 === 0) { + rules.push( + `@media (min-width: ${i}px) { .linaria-${i} {} .other-${i} {} }` + ); + expectedCritical.push(`.linaria-${i}`); + expectedOther.push(`.other-${i}`); + } else { + const selector = i % 2 ? `.linaria-${i}` : `.other-${i}`; + rules.push(`${selector} {}`); + (i % 2 ? expectedCritical : expectedOther).push(selector); + } + } + + const { critical, other } = collect(html, rules.join('\n')); + expect(selectorsOf(critical)).toEqual(expectedCritical); + expect(selectorsOf(other)).toEqual(expectedOther); +}); diff --git a/packages/server/src/collect.ts b/packages/server/src/collect.ts index aabcb4e9b..3f94fa06b 100644 --- a/packages/server/src/collect.ts +++ b/packages/server/src/collect.ts @@ -23,7 +23,7 @@ const extractClassesFromHtml = ( html: string, ignoredClasses: string[] ): RegExp => { - const htmlClasses: string[] = []; + const htmlClasses = new Set(); const regex = /\s+class="([^"]+)"/gm; let match = regex.exec(html); const ignoredClassesDeduped = new Set(ignoredClasses); @@ -33,13 +33,14 @@ const extractClassesFromHtml = ( // eslint-disable-next-line no-param-reassign className = escapeRegex(className); if (className !== '' && !ignoredClassesDeduped.has(className)) { - htmlClasses.push(className); + htmlClasses.add(className); } }); match = regex.exec(html); } - return new RegExp(htmlClasses.join('|'), 'gm'); + // No `g` flag: `test` must not carry `lastIndex` over from one selector to the next + return new RegExp(Array.from(htmlClasses).join('|'), 'm'); }; /** @@ -64,9 +65,10 @@ export default function collect( const htmlClassesRegExp = extractClassesFromHtml(html, ignoredClasses); const blockedClassesSanitized = blockedClasses.map(escapeRegex); + // No `g` flag: `test` must not carry `lastIndex` over from one selector to the next const blockedClassesRegExp = new RegExp( blockedClassesSanitized.join('|'), - 'gm' + 'm' ); const isCritical = (rule: ChildNode) => { @@ -76,12 +78,18 @@ export default function collect( blockedClasses.length > 0 && blockedClassesRegExp.test(rule.selector); if (isExcluded) return false; - return Boolean(rule.selector.match(htmlClassesRegExp)); + return htmlClassesRegExp.test(rule.selector); } return true; }; + // Nodes are collected first and moved to `critical` and `other` at the end. Moving a node with `append` removes + // it from its parent with `splice`, so moving top-level rules one by one made `collect` quadratic in their number. + const criticalNodes: ChildNode[] = []; + const otherNodes: ChildNode[] = []; + const handledTopLevelAtRules = new Set(); + const handleAtRule = (rule: AtRule) => { if (rule.name === 'keyframes') { return; @@ -90,23 +98,29 @@ export default function collect( const criticalRule = rule.clone(); const otherRule = rule.clone(); - let removedNodesFromOther = 0; - criticalRule.each((childRule: ChildNode, index: number) => { + criticalRule.each((childRule: ChildNode) => { + if (!isCritical(childRule)) { + childRule.remove(); + } + }); + otherRule.each((childRule: ChildNode) => { if (isCritical(childRule)) { - otherRule.nodes?.[index - removedNodesFromOther]?.remove(); - removedNodesFromOther += 1; - } else { childRule.remove(); } }); - rule.remove(); + if (rule.parent?.type === 'root') { + // Detached together with the rest of the stylesheet below + handledTopLevelAtRules.add(rule); + } else { + rule.remove(); + } if (criticalRule.nodes?.length) { - critical.append(criticalRule); + criticalNodes.push(criticalRule); } if (otherRule.nodes?.length) { - other.append(otherRule); + otherNodes.push(otherRule); } }; @@ -116,7 +130,7 @@ export default function collect( * we want only handle those from top-level, rest will be handled in stylesheet.walkRules */ if (rule.parent?.type === 'root') { - critical.append(rule); + criticalNodes.push(rule); } }); @@ -140,22 +154,51 @@ export default function collect( } if (isCritical(rule)) { - critical.append(rule); + criticalNodes.push(rule); } else { - other.append(rule); + otherNodes.push(rule); } }); + // Keyframes are looked up only where they used to stay in the stylesheet: in top-level at-rules that were not + // handled. Top-level rules and handled at-rules leave the stylesheet with their contents. + const keyframes: AtRule[] = []; + stylesheet.each((node) => { + if (node.type !== 'atrule' || handledTopLevelAtRules.has(node)) { + return; + } + if (node.name === 'keyframes') { + keyframes.push(node); + } else { + node.walkAtRules('keyframes', (rule) => { + keyframes.push(rule); + }); + } + }); + + // Detach all top-level nodes at once, so `append` does not remove them from the stylesheet one by one + stylesheet.removeAll(); + criticalNodes.forEach((node) => critical.append(node)); + otherNodes.forEach((node) => other.append(node)); + critical.walkDecls(/animation/, (decl) => { animations.add(decl.value.split(' ')[0]); }); - stylesheet.walkAtRules('keyframes', (rule) => { + keyframes.forEach((rule) => { if (animations.has(rule.params)) { critical.append(rule); } }); + // The first node still has the whitespace that preceded it in the stylesheet + if (critical.first) { + critical.first.raws.before = ''; + } + if (other.first) { + other.first.raws.before = ''; + } + return { critical: critical.toString(), other: other.toString(), From e59176523d682fce7ee15a95bab6a7dc2196ec4e Mon Sep 17 00:00:00 2001 From: Anton Evzhakov Date: Mon, 26 Oct 2026 10:31:10 +0200 Subject: [PATCH 3/3] fix(server): keep the source whitespace between rules on postcss 8.4 postcss 8.4 gives a node appended to a root of two or more nodes the whitespace of the last one, so from the third rule on the output lost the whitespace of the source stylesheet. Append each list of nodes in one call while the root is still empty, and test the raw output. The snapshots of two tests lose blank lines that came from the second rule rather than from the source. --- .../server/__tests__/__snapshots__/collect.test.ts.snap | 3 --- packages/server/__tests__/collect.test.ts | 7 +++++++ packages/server/src/collect.ts | 6 ++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/server/__tests__/__snapshots__/collect.test.ts.snap b/packages/server/__tests__/__snapshots__/collect.test.ts.snap index 6808f1335..125521ead 100644 --- a/packages/server/__tests__/__snapshots__/collect.test.ts.snap +++ b/packages/server/__tests__/__snapshots__/collect.test.ts.snap @@ -161,10 +161,8 @@ exports[`collects complex css critical 1`] = ` .linaria ~ div { } - .linaria.linaria2 { } - @keyframes custom-animation { 0% { opacity: 0; @@ -289,7 +287,6 @@ h1 { .linaria:active { } - .linaria::before { } " diff --git a/packages/server/__tests__/collect.test.ts b/packages/server/__tests__/collect.test.ts index 0c0ce256f..658f3a3bc 100644 --- a/packages/server/__tests__/collect.test.ts +++ b/packages/server/__tests__/collect.test.ts @@ -373,6 +373,13 @@ test('excludes every rule with a blocked class', () => { expect(selectorsOf(critical)).toEqual(['.linaria']); }); +test('keeps the whitespace between rules', () => { + const critical = '.linaria {}\n.lily {}\n\n.lotus {}\n\n\n.linaria .lily {}'; + const other = '.other {}\n.other2 {}\n\n.other3 {}'; + + expect(collect(html, `${critical}\n\n${other}`)).toEqual({ critical, other }); +}); + test('keeps the order of rules in a large stylesheet', () => { const rules: string[] = []; const expectedCritical: string[] = []; diff --git a/packages/server/src/collect.ts b/packages/server/src/collect.ts index 3f94fa06b..7c77791eb 100644 --- a/packages/server/src/collect.ts +++ b/packages/server/src/collect.ts @@ -178,8 +178,10 @@ export default function collect( // Detach all top-level nodes at once, so `append` does not remove them from the stylesheet one by one stylesheet.removeAll(); - criticalNodes.forEach((node) => critical.append(node)); - otherNodes.forEach((node) => other.append(node)); + // One call per root, while it is empty: postcss 8.4 gives a node appended to a root of two or more nodes the + // whitespace of the last one, so appending one by one lost the source whitespace from the third rule on + critical.append(criticalNodes); + other.append(otherNodes); critical.walkDecls(/animation/, (decl) => { animations.add(decl.value.split(' ')[0]);