feat(critical-css): accept requiredClassNames and retries at config level

[tomasmax]

The problem

The README documents requiredClassNames and retries as config parameters:

Additionally there are two optional config parameters:

• requiredClassNames: A list of required css class names...
• retries: Number of retries if the requiredClassNames aren't present...

They were never read from config. extractCSSFromApp only ever took them off the route:

const {requiredClassNames, retries} = pathOptions

Two consequences.

A route declared as a plain string is silently never validated. createUrlFrom explicitly supports that form (typeof pathOptions === 'string' ? pathOptions : pathOptions.url), and the README's own first example uses it. Destructuring a string yields undefined for both, and then if (!requiredClassNames) return css returns the first extraction with no check and no warning. So the shortest way to declare a route is also the one where the safety net quietly does not exist.

The same list has to be repeated on every route. requiredClassNames is usually a per-app assertion ("the extraction is only valid if the design system's base classes made it in"), not a per-route one. Ours is identical across all 5 routes of each of our 2 tenants, which is the same array written 10 times, each of them a place to forget to update.

The fix

One resolution point, route over config:

export const resolveRouteOptions = ({pathOptions, config = {}}) => {
  const routeOptions = typeof pathOptions === 'string' || !pathOptions ? {} : pathOptions

  return {
    requiredClassNames: routeOptions.requiredClassNames ?? config.requiredClassNames,
    retries: routeOptions.retries ?? config.retries ?? DEFAULT_RETRIES
  }
}

?? and not ||, so retries: 0 on a route means "do not retry this one" instead of falling through to the config, and requiredClassNames: [] is a deliberate opt-out of a config-level list rather than an empty value to be replaced.

Fully backwards compatible: with nothing set on config, every route resolves exactly what it resolved before.

The documented default was wrong

The README says the default is 2 retries. The code has always used 3 (retries = 3 in the signature of extractCriticalCSS). The doc now says 3 and the number is a named DEFAULT_RETRIES constant instead of a literal buried in a parameter default. Changing the behaviour to match the doc would have silently cut an attempt from every consumer that relies on the default, so the doc is what moved.

Tests

13 specs, 48 passing for the server suite, up from 35.

resolveRouteOptions: config applied to a string route, config applied to an object route, route overriding config, empty list as an opt-out, both undefined, the default of 3, config retries, route retries, retries: 0, and a missing pathOptions.

createUrlFrom had no coverage at all and gets 3 specs, since the string form of a route is exactly what this change hinges on.

Verified end to end, not just unit tested

The specs cover option resolution only; extractCSSFromApp itself opens a browser, so it has none. Run against a static fixture with a layered stylesheet, two routes ('/home': '/' as a plain string, '/other': {url: '/other/'}), and requiredClassNames set on config.

Output for the string route, which is the one that could not be validated before:

@layer base;@layer components;@layer utilities;@layer base{.used-base{color:#010101}}@media (max-width:500px){@layer components{.used-mobile{padding:3px}}}.used-unlayered{margin:5px}

The at-rule context is intact: statements split and hoisted, the @layer base wrapper kept, the @media kept with its nested @layer inside, uncovered siblings dropped, the unlayered rule left unlayered. The second route drops the @media block, since .used-mobile is not on that page. Manifest and per-route files written as expected.

Then the same run with config.requiredClassNames naming a class that is not on either page, before and after the change:

before  0 retry attempts, 267 bytes written   <- config silently ignored
after   2 attempts per route, 0 bytes written <- validation actually runs

And with a class that is present, the output is byte-identical to the previous code on both routes and in critical.json.

Upgrade note

That second run is also the one behaviour change to be aware of. Anyone who followed the README and set requiredClassNames on config has been getting a no-op. After this, the validation is real, and a list that does not match the page ends with the attempts exhausted and an empty critical CSS written for every route, which is the documented "it would be discarded" behaviour finally taking effect. Worth checking the extraction log for Attempt limit reached on the first build after upgrading.

Note on versions

1.33.0 was tagged and its CHANGELOG written, but it never reached npm, so npm view @s-ui/critical-css versions does not list it. Merging this releases 1.34.0, which carries the at-rule-context fix of #1995 as well.

🤖 Generated with Claude Code