fix: teach cn() the Tailwind v4 parenthesis hint spelling - #1357
Conversation
|
Design rationale: two rejected ways to avoid touching The The first is to give the paren border hint its own isolated The second is to have A related judgement: |
vivek7405
left a comment
There was a problem hiding this comment.
Read the whole diff. The three logic edits are right, and I checked the part I most wanted checked: the change is surgical, every pair it moves involves a paren token, and the two hand-synced copies still agree on the full ordered-pair sweep. The two-counter choice is the one call here that could have quietly gone wrong and it holds up against Tailwind's own matched-pair splitter.
What I did miss is doc sync, in the file an agent reads first. Two findings there, both on unchanged lines so they cannot be anchored inline:
packages/ui/AGENTS.md L481 to L484 still says a variant prefix is split on the last colon OUTSIDE square brackets, which is exactly what the bullet I rewrote further down now contradicts. Both source copies got that sentence updated and this one did not, so the file argues with itself about what variantPrefix does. That is worse than having left the whole thing alone.
packages/ui/AGENTS.md L485 opens "Once a bracketed value reaches the matcher, its TYPE HINT names the property", which scopes the hint rule to brackets, but hintedGroup() reads -[ and -( alike now.
Sweeping for that same stale sentence turned up a third the review did not name: packages/ui/test/cn-helper.test.js L123 and L141 carry it too. Folding all of them into this PR.
|
Resolution: the two unanchorable AGENTS.md findings, plus a third they led me to Both doc findings from the review are fixed in
Grepping every surface for that same stale sentence turned up Worth naming the pattern, since it is the more useful lesson than the individual lines. Rewriting one bullet in a file and leaving its neighbours asserting the behaviour the rewrite contradicts is a worse outcome than not touching the file at all, because a reader who hits the stale bullet first has no way to know it lost. The issue's docs section enumerated exactly one bullet, and I treated that enumeration as the surface list rather than grepping for the claim itself. The grep is what should have driven it. |
e1a55c5 to
d4ab339
Compare
Tailwind v4 added `shadow-(color:--x)` as shorthand for `shadow-[color:var(--x)]`. `variantPrefix` counted only bracket depth, so the colon inside the parentheses read as a variant separator and the matcher was handed the fragment `--x)`, which matches nothing. The utility ended up ungrouped: safe, in that it never evicts, but two utilities setting the identical property no longer collapse, so the winner falls to compiled stylesheet order. Three edits, and all three are load-bearing. `variantPrefix` gains a second counter for parens and splits only where both depths are zero. `hintedGroup()`'s regex accepts `-(` alongside `-[`. And `borderGroups()`'s width fragment reads the paren length hint. Two separate counters rather than one shared one, because a shared counter lets a stray `)` cancel a live `[` and reads `x-[y)-z:w` as having a top-level colon, reintroducing the same fragment bug on a different input. Tailwind's own splitter is a matched-pair stack, and two counters match it on every well-formed class. Teaching `variantPrefix` paren depth alone is worse than leaving the bug alone: it hands an intact `bg-(image:--g)` to a matcher that cannot read the hint, which falls through to the `^bg-` catch-all and evicts a real background colour. That is the #1065 defect class, so the halves land together.
The paren-gap bullet in packages/ui/AGENTS.md was rewritten, but two bullets above it still asserted the split is the last colon outside square brackets, so the file contradicted itself about what variantPrefix does. Two test comments carried the same stale sentence. Also qualify the segment() citation. It read as a repo-local path in a monorepo that has its own packages/ root, and the blog copy had dropped the path entirely, so the same rationale cited different evidence in the two hand-synced copies.
d4ab339 to
b0bdcde
Compare
Closes #1338
Tailwind v4 added a second spelling for a type-hinted arbitrary value that uses parentheses instead of brackets, and
cn()could not read it.variantPrefixcounted only bracket depth, so the colon insideshadow-(color:--x)read as a variant separator and the group matcher was handed the fragment--x), which matches nothing. The utility ended up ungrouped. That failed in the safe direction, since nothing was dropped, but it gave up the other half of the guarantee: two utilities setting the identical property did not collapse, so the winner fell to compiled stylesheet order.What changed
Three edits, in each of the two hand-synced copies (
packages/ui/packages/registry/lib/utils.tsandexamples/blog/lib/utils/cn.ts):variantPrefix()counts parens in a SECOND counter and splits only where both depths are zero.hintedGroup()'s regex accepts-(alongside-[, one character class.borderGroups()'swidthfragment gains one alternation branch for(length:...).No new
HINTED_GROUPSentries. Both spellings produce the identical<prefix>:<hint>key the map is already keyed on, which is why the fix is this small.Two counters, not one
A single shared counter lets a stray
)cancel a live[, sox-[y)-z:wreads as having a top-level colon and the matcher gets the fragmentw. That is precisely the bug this PR removes, wearing a different hat. Tailwind's own top-level splitter is a matched-pair stack, and two counters agree with it on every well-formed class and every unbalanced-delimiter case in the issue's corpus. Porting the full stack was considered and rejected: the only input it decides differently is a string Tailwind cannot compile, andcn()is deliberately small and auditable (package invariant 2).Why all three edits land together
A
variantPrefix-only fix is worse than the bug. It hands an intactbg-(image:--g)to a matcher that cannot read the hint, the token falls through to the^bg-catch-all asbg-color, and it evicts a real background colour. Same fortext-, and same forborder-if theborderGroups()fragment is left out. That is the #1065 defect class, so the halves are one change.Test plan
packages/ui/test/cn-helper.test.js: 21/21. The two fix: split the coarse bg, shadow and text-shadow cn() groups by property #1332 pinning assertions are inverted with their comment rewritten, and a new#1338test covers 30 assertions across the prefixes a partial fix damages. The other 201 pre-existing assertions pass unchanged.test/ui/cn-copies-in-sync.test.mjs: 2/2 on Node and under Bun. Thirteen paren tokens joined the sharedTOKENSbattery, so every ordered pair merges through both copies and is compared.variantPrefix's paren counter reverted: 2 test failures (the new#1338test and the box-shadow test carrying the two inverted pins).hintedGroup()'s regex reverted: 1 test failure. The shadow pins survive here, becauseshadow-(color:--x)still reaches the^shadow-catch-all through GROUPS oncevariantPrefixstops mangling it.borderGroups()'s width fragment reverted: 1 test failure, and it is a DROP rather than a non-collapse.cn('border-(length:--w)', 'border-primary')returnsborder-primary, losing the width. That is the defect the third edit exists to prevent.npm test: 4150 tests, 4142 pass, 7 fail, and all 7 are pre-existing. Five are the known linked-worktree baseline (the listener pair and three elision assertions, which pass in a primary checkout and in CI). The other two aretest/scaffolds/gallery-coverage.test.js, which I reproduced on an untouched primary checkout at79fc28fcwith none of this change present.npm test --workspace=@webjsdev/ui: 210/210.webjs checkfromexamples/blog: all checks pass.cn()is a pure string function with no DOM, no network and no app-boot behaviour, and the N by N sweep in the issue shows zero changed pairs where neither token contains a parenthesis.test/bun/<feature>.mjsN/A: the Bun parity hook gates on^packages/([^/]+/src|editors/[^/]+/src|cli/lib)/, which neither touched source path matches. Cross-runtime coverage is real regardless, through the drift file above.Docs
packages/ui/AGENTS.md: the paren-gap bullet described the gap as live behaviour and is rewritten rather than deleted, since thehintedGroup()centrality lesson still applies.variantPrefix,HINTED_GROUPSandborderGroups()header comments in both copies..agents/skills/webjs/references/styling.md: its coarseness caveat names two other gaps and never named this one, so nothing there became false.cn()'s hint handling.website/lib/utils/cn.tsis generated from the registry and inherits the fix.lib/utils/cn.tsis copied verbatim from the registry at create time.Merge dependency
#1320 edits the same
utils.tsin a different region (it memoises theGROUPStable at L53 to L145 and its read site at L286). This diff stays strictly insidevariantPrefix(),hintedGroup()and one string literal inborderGroups(), so the two land on separate hunks. #1320 merges first; this branch rebases onto main and re-runs before merging.