Skip to content

[DS 2.0] Shrink the built-CSS checker and document the pitfalls - #104

Open
librowski wants to merge 2 commits into
ds2-namespace-splitfrom
ds2-built-css-checks
Open

[DS 2.0] Shrink the built-CSS checker and document the pitfalls#104
librowski wants to merge 2 commits into
ds2-namespace-splitfrom
ds2-built-css-checks

Conversation

@librowski

@librowski librowski commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #102.

The built-CSS checker had grown per review finding rather than per invariant: 299 lines on
21.08, 433 after the font work, 480 after the public-variable rename, 523 after the namespace
split. Eight bug classes in a 40-line docstring, eleven report() calls, and two checks that
were snapshots of the current shape rather than rules.

This splits it into a small deterministic checker and a catalogue that carries the reasoning.

What changed

packages/ui/scripts/check-built-css.ts: 523 → 117 lines, eight checks, all operating on
parsed PostCSS nodes so every failure reports file:line for free. No hardcoded list of
variable names remains.

Check Scope
var() takes a custom-property name every dist
Sanctioned --wb-ds- / --wb-sdk- / --wb-public- namespace every dist
No @import every dist
Every non-data url() resolves inside dist every dist
Every rule inside @layer (statement-form declarations allowed) this package
File leads with the layer-order statement this package
Only declared layer names this package
Public defaults on :root inside ui.base this package

The four layer and public-default checks are scoped to this package's own output, because this
package owns those contracts. A consumer bundle ships its own unlayered component CSS and may
set a public variable on its own subtree as a scoped override.

packages/ui/built-css-pitfalls.md (new, registered in CLAUDE.md): fifteen failure classes,
each answering what breaks, why it is silent, how to spot it, and whether it is automated.
That includes five classes the checker deliberately does not decide, which is where a reviewer
still has to look:

  • font subsetting and flash of unstyled text, including which characters are inline
  • Content Security Policy font blocking, including the default-src fallback
  • a documented public variable absent from dist
  • a primitive token used where a role belongs, so the theme stops switching
  • typography metrics copied instead of composed

Two checks left the script:

  • exports pointing at real files is now publint, which covers that and more upstream
  • the entry-chunk @font-face assertion became a unit test next to combine-css-bundle.mts,
    where the invariant is produced

The public-default check accepts the root element written as either :root or html. Both
match the same element, and the repo writes theme scopes as html[data-theme='...'] in 21
places, so rejecting that spelling would have been a false positive.

Fixed along the way

The malformed-var() pattern never worked. /var\(\s*(?!--)/ backtracks \s* to empty, the
lookahead then sees whitespace instead of --, and the match succeeds — so var( --x ) was
reported as malformed while genuinely broken values could slip through. It is now
/var\((?!\s*--)/. The bug was invisible because the old script ran this check only over this
package's dist, which had no wrapped var( calls.

Not done on purpose

An earlier revision of this branch stripped statement-form @layer declarations from the asset
files at build time, so the coverage check had a single allowed shape. That has been removed:
rewriting build output to satisfy a lint is not worth the moving part. The check now accepts a
statement-form declaration instead.

Those declarations are harmless. A stylesheet whose rules only composes global type roles ends
up with empty bodies, so its @layer ui.component { } block serializes as @layer ui.component;; input-font-size.module.css and the button font-size module each produce one.
The leading-order check guarantees the canonical @layer ui.base, ui.component; comes first in
every asset file, so a later declaration cannot reorder anything.

Verification

  • pnpm build:lib green end to end, including the SDK's own dist pass
  • every retained check proved by temporarily breaking it: malformed var(), unlayered CSS,
    missing layer order, unknown layer name, namespace violation, @import, missing URL target,
    mis-scoped public default
  • packages/ui tests pass, including the new combine-css-bundle.spec.mts
  • ESLint and Prettier clean on touched files

Note

Generalising the public-default check surfaced one pre-existing problem outside this PR's
scope: --wb-public-node-gap is documented as an override on the NodePanel page but is
redefined to 0 by three built-in node templates, so a consumer's :root override does not
apply there. Filed separately rather than fixed here.

@piotrblaszczyk

Copy link
Copy Markdown
Contributor

Verified locally, including break-testing each check. Two narrowings vs #102 worth a follow-up: the namespace check now inspects only declared props, so a bad name inside a var() reference passes (stylelint covers it at source), and the url() check lost case-exact matching, so wrong-case paths pass on macOS and fail only in CI. Restore or document both in the pitfalls doc. Not blocking. CI never ran on this head, needs a nudge before merge.

@librowski
librowski force-pushed the ds2-built-css-checks branch from 9ff579c to 8997dd3 Compare August 31, 2026 21:02
@librowski
librowski force-pushed the ds2-built-css-checks branch from 8997dd3 to 3614263 Compare August 31, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants