diff --git a/packages/sui-critical-css/README.md b/packages/sui-critical-css/README.md index cf143b5e8..2fab3a989 100644 --- a/packages/sui-critical-css/README.md +++ b/packages/sui-critical-css/README.md @@ -6,6 +6,11 @@ 1. Read the config options and routes provided. 2. For each route, it opens a browser, navigate and extract the Critical CSS. + Chrome reports coverage for style rules only, so the extracted CSS is rebuilt with postcss + instead of concatenating the raw byte ranges. That keeps the `@media` / `@layer` / `@supports` + context of every rule it keeps, the `@layer` statements that fix the layer order, and the + at-rules that define rather than style: `@property`, `@font-face`, and the `@keyframes` named by + a declaration that survived. 3. Create a css file in the `critical-css` folder. 4. After doing this for each route, then creates a `critical.json` file that could be read for every path to extract the critical-css. 5. Use then `@s-ui/critical-css-middleware` to extract to use in your Express app the CSS. diff --git a/packages/sui-critical-css/src/covered-css.js b/packages/sui-critical-css/src/covered-css.js index da85a073f..ed27a1ede 100644 --- a/packages/sui-critical-css/src/covered-css.js +++ b/packages/sui-critical-css/src/covered-css.js @@ -14,8 +14,38 @@ import postcss from 'postcss' // So instead of slicing bytes, parse the stylesheet and rebuild it: keep every // declaration-holding node whose source range intersects a covered range, together // with its whole chain of ancestor at-rules. +// +// The same "ranges cover style rules only" limitation also means an at-rule that +// DEFINES something rather than styling something is never attributable to a range, +// so it can only be carried over by an explicit exception. See `isDefinitionAtRule` +// and `isKeyframes` below. const KEEP_AT_RULE = 'layer' +// `@property` registers a custom property and `@font-face` a font: neither matches an +// element, so coverage never reports a range for them and both used to be dropped from +// every rebuild. Measured on a Tailwind v4 app: 0 `@property` and 0 `@font-face` in the +// rebuilt CSS against 29 and 5 in the source sheet. +// +// The `@property` gap changes what the browser paints. `border-style:var(--tw-border-style)` +// with the registration missing resolves to the empty token, which is invalid at computed-value +// time, so `border-style` computes to its initial `none` — and with `border-style:none` the +// computed `border-width` is `0px` even though the utility's `1px` applied and won the cascade. +// A button styled that way renders with no border at all until the full stylesheet arrives. +// +// Both are cheap to keep and safe to keep when unused: a registration only sets an initial +// value, and a `@font-face` never triggers a download unless a matched rule asks for the family. +const DEFINITION_AT_RULES = ['property', 'font-face'] + +// `@keyframes` holds rules, not declarations, and those inner rules +// (`from{}`, `50%{}`) never match an element either. Unlike the two above it is not +// bounded in size, so it is only carried over when a declaration that survived the +// rebuild actually names it. +const KEYFRAMES_AT_RULE = /^(-\w+-)?keyframes$/ +const ANIMATION_DECLARATIONS = ['animation', 'animation-name'] +const ANIMATION_VALUE_SEPARATOR = /[\s,]+/ + +const atRuleName = node => node.name.toLowerCase() + const holdsDeclarations = node => node.type === 'rule' || (node.type === 'atrule' && node.nodes?.some(child => child.type === 'decl')) @@ -24,6 +54,26 @@ const holdsDeclarations = node => const isLayerStatement = node => node.type === 'atrule' && node.nodes === undefined && node.name.toLowerCase() === KEEP_AT_RULE +const isDefinitionAtRule = node => node.type === 'atrule' && DEFINITION_AT_RULES.includes(atRuleName(node)) + +const isKeyframes = node => node.type === 'atrule' && KEYFRAMES_AT_RULE.test(atRuleName(node)) + +// Only the direct declarations of a node, so scanning a kept `@media` does not reach the +// declarations of the children that were discarded from it. +const animationNamesOf = node => { + const names = [] + + node.each?.(child => { + if (child.type !== 'decl' || !ANIMATION_DECLARATIONS.includes(child.prop.toLowerCase())) return + + // Every token of the shorthand, since the name can sit anywhere in it. A duration or a + // timing function that happens to match a `@keyframes` name only keeps one extra rule. + names.push(...child.value.split(ANIMATION_VALUE_SEPARATOR)) + }) + + return names +} + // One statement per layer name, because clean-css 5.3.3 mis-parses a bodyless `@layer` // that names more than one layer: it drops the statement AND every declaration of the // rule that follows it (`@layer a,b;.x{color:red}` minifies to nothing at all). A @@ -54,11 +104,19 @@ export const rebuildCoveredCSS = ({text, ranges}) => { const keep = new Set() const layerStatements = [] const nestedLayerStatements = [] + const keyframes = [] const keepWithAncestors = node => { for (let current = node; current && current.type !== 'root'; current = current.parent) keep.add(current) } + // An at-rule kept as a whole needs its children kept too, or the pass that removes + // everything uncovered would empty it out. + const keepWithSubtree = node => { + keepWithAncestors(node) + node.walk?.(child => keep.add(child)) + } + root.walk(node => { if (isLayerStatement(node)) { if (node.parent.type === 'root') return layerStatements.push(...splitLayerStatement(node)) @@ -68,9 +126,17 @@ export const rebuildCoveredCSS = ({text, ranges}) => { // cannot be hoisted out of its parent the way a top-level one can. Keep it in place. return nestedLayerStatements.push(node) } + if (isDefinitionAtRule(node)) return keepWithSubtree(node) + if (isKeyframes(node)) return keyframes.push(node) if (holdsDeclarations(node) && intersectsCoveredRange(node, ranges)) keepWithAncestors(node) }) + // After the walk, so it sees every node the rebuild is going to keep. + const animationNames = new Set([...keep].flatMap(animationNamesOf)) + keyframes.forEach(node => { + if (animationNames.has(node.params.trim())) keepWithSubtree(node) + }) + // Replacing them during the walk above would make postcss visit the statements the split // produces, so they are only rewritten once the walk is over. nestedLayerStatements.forEach(node => { diff --git a/packages/sui-critical-css/test/server/covered-cssSpec.js b/packages/sui-critical-css/test/server/covered-cssSpec.js index 56a58943e..162f6841b 100644 --- a/packages/sui-critical-css/test/server/covered-cssSpec.js +++ b/packages/sui-critical-css/test/server/covered-cssSpec.js @@ -108,6 +108,56 @@ describe('@s-ui/critical-css covered-css', () => { ) }) + describe('definition at-rules', () => { + it('keeps a @property registration even though coverage never marks it as used', () => { + const text = '@property --tw-border-style{syntax:"*";inherits:false;initial-value:solid}.used{color:red}' + + expect(rebuild(text, '.used{color:red}')).to.equal(text) + }) + + it('keeps the at-rules a @property registration is nested in', () => { + const text = '@layer properties{@property --x{syntax:"*";inherits:false}}.used{color:red}' + + expect(rebuild(text, '.used{color:red}')).to.equal(text) + }) + + it('keeps a @font-face nobody covered, so the first paint does not swap fonts', () => { + const text = '@font-face{font-family:F;src:url(f.woff2)}.used{color:red}' + + expect(rebuild(text, '.used{color:red}')).to.equal(text) + }) + + it('keeps a @keyframes named by a declaration that survived the rebuild', () => { + const text = '@keyframes spin{to{transform:rotate(1turn)}}.used{animation:spin 1s linear infinite}' + + expect(rebuild(text, '.used{animation:spin 1s linear infinite}')).to.equal(text) + }) + + it('keeps a @keyframes named by animation-name', () => { + const text = '@keyframes spin{to{opacity:0}}.used{animation-name:spin}' + + expect(rebuild(text, '.used{animation-name:spin}')).to.equal(text) + }) + + it('keeps a vendor-prefixed @keyframes', () => { + const text = '@-webkit-keyframes spin{to{opacity:0}}.used{animation:spin 1s}' + + expect(rebuild(text, '.used{animation:spin 1s}')).to.equal(text) + }) + + it('drops a @keyframes no kept declaration names', () => { + const text = '@keyframes spin{to{opacity:0}}.used{color:red}' + + expect(rebuild(text, '.used{color:red}')).to.equal('.used{color:red}') + }) + + it('does not keep a @keyframes named only by a rule that was discarded', () => { + const text = '@keyframes spin{to{opacity:0}}.used{color:red}.unused{animation:spin 1s}' + + expect(rebuild(text, '.used{color:red}')).to.equal('.used{color:red}') + }) + }) + it('returns an empty string when nothing is covered', () => { expect(rebuildCoveredCSS({text: '@media print{.unused{color:blue}}', ranges: []})).to.equal('') })