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 18615477b..125521ead 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; @@ -120,35 +122,43 @@ exports[`collects complex css critical 1`] = ` .linaria::before, .linaria::after { - content: \\"\\"; + content: ""; 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 { + +[data-theme="dark"] .lily { color: #000; } + .linaria ~ div { } .linaria.linaria2 { @@ -266,12 +276,15 @@ 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 a25564ba6..658f3a3bc 100644 --- a/packages/server/__tests__/collect.test.ts +++ b/packages/server/__tests__/collect.test.ts @@ -1,14 +1,20 @@ import dedent from 'dedent'; -import prettier from 'prettier'; +import postcss from 'postcss'; +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 +201,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 +217,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 +239,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', () => { @@ -267,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', () => { @@ -282,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 {}"`); }); @@ -323,3 +338,67 @@ 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 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[] = []; + 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/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" }, diff --git a/packages/server/src/collect.ts b/packages/server/src/collect.ts index aabcb4e9b..7c77791eb 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,53 @@ 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(); + // 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]); }); - 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(),