Check every @theme and :root block in the theme mirror guard, not just the first - #484
Conversation
The mirror guard added in #478 located its blocks with themeCss.match(), which stops at the first hit, and matched the literal '@theme {'. A second @theme or :root block was therefore read past in silence, and Tailwind v4's '@theme inline { ... }' never matched at all -- the form you reach for when a token's value is a var() chain, which is exactly what the guard checks. matchAll over global patterns closes both gaps. Verified: green on theme.css as it stands (34 document-level declarations, 27 body-level); appending '@theme inline { --color-new-thing: var(--vscode-badge-background); }' or a second ':root { --mt-extra: var(--vscode-font-size); }' passed before and fails now.
Deploying mouseterm with
|
| Latest commit: |
75add88
|
| Status: | 🚫 Build failed. |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The matchAll switch is right and I verified the "identical to before" claim — replaying both the old and new logic against theme.css gives 34 document-level and 27 body-level declarations either way, with no misses.
One gap survives, and it is the same silent-skip the PR exists to close. Tailwind v4's @theme takes four modifiers — inline, static, reference, default — and they combine, so @theme static inline { ... } is valid. The (?: inline| static)? alternation covers two of the four and none of the combinations, so @theme default { ... } and @theme static inline { ... } are still read past in silence. Replaying the new pattern against theme.css plus an appended block declaring --color-new-thing: var(--vscode-badge-background):
| block | new pattern | @theme\b[^{}\n]*\{ |
|---|---|---|
@theme inline |
caught | caught |
@theme static |
caught | caught |
@theme default |
skipped | caught |
@theme static inline |
skipped | caught |
@theme reference |
skipped | caught |
The inline suggestion swaps the enumeration for a bounded wildcard. [^{}\n]* rather than [^{]* matters twice: it stops the greedy run from swallowing a following @theme block when there are two (the exact case this PR adds matchAll for), and it keeps the two prose mentions of @theme in the file header comment from matching, since neither line contains a {. Baseline is unchanged under it — still 34/27, still no misses.
@theme reference is the one judgment call: it emits no custom properties, so a var()-bound token inside one arguably needn't be mirrored, and the wildcard would flag it. That trade seems right anyway — a false failure is loud and one edit away, while the current behavior is a skip nobody sees.
|
The one remaining red is Evidence
|
2 participants
The mirror guard that landed in #478 reads only the first
@themeblock and the first:rootblock, so a second one of either is skipped in silence — and its@theme \{literal never matches Tailwind v4's modifier forms like@theme inline { ... }, which is the shape you reach for precisely when a token's value is avar()chain, i.e. exactly the tokens the guard exists to check. SwitchingblockBodytomatchAllover global patterns, and matching the@themeheader with a bounded wildcard rather than a literal, closes both gaps; the check itself is unchanged.Verified locally:
vitest run src/lib/themes/consumed-keys.test.tsis green ontheme.cssas it stands (34 document-level declarations, 27 body-level, no misses — identical to before), and the mutations below all passed under the old version and fail under this one.Mutation checks
Each check appends a block to
lib/src/theme.cssdeclaring avar()-bound token that is never mirrored ontobody, then runs the test:The guard must fail with
AssertionError: expected [ '--color-new-thing' ] to deeply equal []. Results by block form:@theme inline@theme static@theme default@theme static inline@theme reference:root { --mt-extra: var(--vscode-font-size); }("skipped" = the test stays green because the declaration never enters the map.)
Tailwind v4's
@themetakes four modifiers —inline,static,reference,default— and they combine, so@theme static inline { ... }is valid. Matching the header with@theme\b[^{}\n]*\{covers all of them without enumerating. The[^{}\n]*bound matters twice: it stops the run from swallowing a following@themeblock when there are two (the casematchAllis here for), and it keeps the two prose mentions of@themeintheme.css's file-header comment from matching, since neither line contains a{.@theme referenceemits no custom properties, so avar()-bound token inside one arguably needn't be mirrored and this will flag it. That trade is deliberate: a false failure is loud, names the token, and is one line to fix, while the current behavior is a skip nobody sees.Every package in the workspace is on
tailwindcss ^4.3.0, where more than one@themeblock — and the modifier forms — are ordinary. No spec change:docs/specs/theme.mdalready states the invariant this enforces, and the invariant is unchanged.