fix(sui-critical-css): carry @property, @font-face and referenced @keyframes into the rebuilt CSS

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.

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 => {

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('')
   })