[DS 2.0] Shrink the built-CSS checker and document the pitfalls - #104
Open
librowski wants to merge 2 commits into
Open
[DS 2.0] Shrink the built-CSS checker and document the pitfalls#104librowski wants to merge 2 commits into
librowski wants to merge 2 commits into
Conversation
librowski
requested review from
lukasz-jazwa,
piotrblaszczyk and
szymon-t-sc
as code owners
August 27, 2026 15:01
librowski
force-pushed
the
ds2-built-css-checks
branch
from
August 27, 2026 15:24
b4d8065 to
9ff579c
Compare
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 |
piotrblaszczyk
approved these changes
Aug 31, 2026
librowski
force-pushed
the
ds2-built-css-checks
branch
from
August 31, 2026 21:02
9ff579c to
8997dd3
Compare
librowski
force-pushed
the
ds2-built-css-checks
branch
from
August 31, 2026 21:12
8997dd3 to
3614263
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 thatwere 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 onparsed PostCSS nodes so every failure reports
file:linefor free. No hardcoded list ofvariable names remains.
var()takes a custom-property name--wb-ds-/--wb-sdk-/--wb-public-namespace@importurl()resolves inside dist@layer(statement-form declarations allowed):rootinsideui.baseThe 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 inCLAUDE.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:
default-srcfallbackTwo checks left the script:
exportspointing at real files is nowpublint, which covers that and more upstream@font-faceassertion became a unit test next tocombine-css-bundle.mts,where the invariant is produced
The public-default check accepts the root element written as either
:rootorhtml. Bothmatch the same element, and the repo writes theme scopes as
html[data-theme='...']in 21places, 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, thelookahead then sees whitespace instead of
--, and the match succeeds — sovar( --x )wasreported 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 thispackage's dist, which had no wrapped
var(calls.Not done on purpose
An earlier revision of this branch stripped statement-form
@layerdeclarations from the assetfiles 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
composesglobal type roles endsup with empty bodies, so its
@layer ui.component { }block serializes as@layer ui.component;;input-font-size.module.cssand the button font-size module each produce one.The leading-order check guarantees the canonical
@layer ui.base, ui.component;comes first inevery asset file, so a later declaration cannot reorder anything.
Verification
pnpm build:libgreen end to end, including the SDK's own dist passvar(), unlayered CSS,missing layer order, unknown layer name, namespace violation,
@import, missing URL target,mis-scoped public default
packages/uitests pass, including the newcombine-css-bundle.spec.mtsNote
Generalising the public-default check surfaced one pre-existing problem outside this PR's
scope:
--wb-public-node-gapis documented as an override on the NodePanel page but isredefined to
0by three built-in node templates, so a consumer's:rootoverride does notapply there. Filed separately rather than fixed here.