feat(studio): fail the suite on a class that resolves to nothing - #3623
Draft
miguel-heygen wants to merge 2 commits into
Draft
feat(studio): fail the suite on a class that resolves to nothing#3623miguel-heygen wants to merge 2 commits into
miguel-heygen wants to merge 2 commits into
Conversation
Tailwind silently drops a class it cannot compile, so `rounded-button` renders as no radius at all and nothing goes red. Two tests close that: - the token gate compiles Studio's entry stylesheet with every class the source claims and reports `file: class` for anything that produces no selector. Tailwind is the judge, so there is no allowlist. - the hex ratchet counts colour literals per file against a committed baseline. It fails on a rise, and on a fall it prints the command that banks the lower number. The baseline is only written under a named flag, never as a side effect of a run. The gate's first run found seven names the markup has always asked for and no config has ever defined: rounded-button, shadow-btn-primary, bg-surface-hover, bg-accent-red, ease-standard, text-2xs and two panel backgrounds. Each is added to theme.css, the semantic ones by role and the rest as deprecated aliases for the sweep to remove. It also found a dead state class on the timeline clip, styled nowhere, which is deleted.
The extractor treated a string as a class candidate only when its binding matched Styles/Classes/ClassName(s), so the same twelve classes were gated in `sizeStyles` and invisible in `buttonSizes`. A name is not a contract. Candidate-ness now follows one file's data flow: an identifier used inside a className attribute or a class-building call marks whatever is bound to it as classes, under any name. A subscript is excluded, so the key of `variantStyles[variant]` is not mistaken for a class list. The name-based anchor stays for the map whose only consumer is another module. The sweep this opened found one class no config has ever defined: the automation menu's hover row asked for a background that compiled to nothing, and now uses the token the rest of that row already uses.
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.
Lands unit U2 (token gate and hex ratchet) of the Studio design-system foundation. Stacked on the theme PR (#3621). Ratchet baseline: 475 color literals across 104 files, committed; this PR adds none.
What
Two tests in
packages/studio/src/stylesthat fail the Studio suite when a design value stops resolving.tokenGate.test.ts). Compiles Studio's real entry stylesheet through Tailwind v4'scompile()with every class the source claims as the candidate list, then reportsfile: classfor any candidate that produces no selector.hexRatchet.test.ts). Counts colour literals (#hex,rgb(),rgba(),hsl(),hsla()) per file against a committed baseline. A rise fails; a fall passes and prints the command that banks the lower number.classCandidates.ts, the pure extractor both the gate and its own tests use, plusstyleSources.ts, the small shared reader the three style tests now share.Stacked on the Tailwind v4 and theme-token branches.
Why
Tailwind has no strict mode for the classes it finds in markup. A class it cannot compile is silently dropped, so a button asking for
rounded-buttonrenders with no radius, review reads it as real, and nothing anywhere goes red. Colour literals have the same shape of problem in reverse: they always work, so they accumulate.Both checks are vitest tests inside
packages/studioso they run in the required Test job. A lint-only rule would not block.How
Tailwind is the judge, so there is no allowlist. Static utilities, arbitrary values, theme tokens and Studio's own hand-written CSS rules all appear in the emitted sheet; a name nothing defines does not. Two structural exceptions, both namespaces rather than lists: Tailwind's
groupandpeervariant markers, which by design emit no rule, and thehf-prefix Studio reserves for its own semantic hooks, which are not utilities and carry no design value.The extractor is deliberately narrow. It reads
className/classattributes, arguments to class-building calls,*class/*classNameproperties, and bindings named*Styles/*Classes/*ClassName(s)(the lookup-table shape the primitives use, which never reaches aclassNameliteral). Anchors are matched against a masked copy of the file with comment and string bodies blanked, so prose in a doc comment and generated markup inside a template literal are both invisible to it. A template chunk that touches an interpolation is dropped at that edge.The ratchet's baseline is per file, not per repository. Two sweep PRs touching the same file conflict on that file's line, which is exactly when a recount is wanted. A file absent from the baseline has a baseline of zero, so a rename cannot smuggle colours past it. The baseline is only written under
HEX_BASELINE_WRITE=1, never as a side effect of a normal run, and a missing baseline fails with that flag named.First-run false-positive list. The gate was run over the tree before it was allowed to fail the suite. Every entry is accounted for; none is allowlisted.
activeTool === "razor"), lookup keys, inline CSS texthf-fx-*,hf-automation-*,hf-volume-rowrounded-button,shadow-btn-primary,bg-surface-hover,bg-accent-red,ease-standard,text-2xs,bg-panel-bg-soft,bg-panel-bg-2theme.cssis-microon the timeline clipstudio.css, this one has none anywhere. Dead class removedThe new tokens:
--radius-buttonand--ease-standardare named by role and alias--radius-mdand--ease-out-quint;--shadow-btn-primaryis a new value, a contact shadow plus an ambient one plus an inner top highlight for the one light surface in a dark UI;--color-surface-hover,--color-accent-red,--text-2xs,--color-panel-bg-softand--color-panel-bg-2go in the deprecated block as aliases for the sweep to remove, so there is still one source per decision.Adding them changes what renders: those classes previously compiled to nothing, so the affected surfaces had no background, no radius and no shadow at all. The alias targets are a judgement call and worth a look in review.
Baseline total: 475 colour literals across 104 files. Higher than earlier hand counts because this regex also counts
rgb(),rgba(),hsl()andhsla()alongside hex, as the plan requires. Whatever this rule sees is the number.Test plan
bunx vitest run --poolOptions.forks.maxForks=4inpackages/studio: 431 files, 4778 tests, all passing.rounded-nonesuchand a hex toui/Button.tsxfails both, each naming the file and the offending string.cn()arguments, variant stripping, arbitrary-value counting, interpolation edges, unterminated regions and non-class strings.bun run typecheck,bun run build,bunx oxlint,bunx oxfmt --checkall clean.bunx fallow audit --base origin/main --fail-on-issuespasses, at the same counts as the branch point.Not covered
hf-hook classes are exempt, not fixed. 81 names are referenced in markup and styled nowhere in the repo. They are either intentional semantic hooks or leftovers from removed CSS; either way, deciding which needs a look at each panel and belongs with the sweep, not here.hover:resolves asbg-x. A misspelled variant is not caught.hf-color-grading-*andtimeline-cliprules instudio.cssare untouched.Follow-up commit
fix(studio): gate a class by where it is used, not what it is called. The first version decided candidate-ness by the name of the binding: onlyStyles/Classes/ClassName(s)counted. A primitive that keeps its size classes in a record namedbuttonSizeswas therefore invisible to the gate, sorounded-holograminside that record stayed green while the identical string in aclassNameattribute went red. A name is not a contract.Candidate-ness now follows one file's data flow. Every identifier used inside a
className/classattribute or a class-building call marks whatever that identifier is bound to elsewhere in the file as a class list, under any name. The four masking rules that kept the first run's false positives out (comments, generated code inside template strings, comparison operands, non-class call arguments) are unchanged, and one more joins them: a subscript is excluded, so the key ofvariantStyles[variant]is not read as a class and the parameter defaultvariant = "ghost"is not read as a class list. The name-based anchor is kept as a second source rather than the only one, because it is the only thing that can see a class map whose only consumer lives in another module, which single-file data flow cannot follow.Running the widened gate over the whole tree produced two new hits, one bug in the extractor and one real:
ui/Button.tsx(md,ghost,secondary) were lookup-key defaults, not classes. That is the subscript rule above, and a unit test pins it.bg-panel-bg-3in the automation selection menu is a real one: no config has ever defined that token, so the menu row's hover background compiled to nothing. It now uses the hover token the rest of that row already uses.Both the extractor test and the gate test are proven non-vacuous: with the old extractor restored, the
buttonSizesfixture and the subscript fixture fail and the gate reports nothing.Test plan for this commit:
bunx vitest run src/styles --poolOptions.forks.maxForks=4(38 passing, 4 files), the full Studio suite once (431 files, 4781 tests, 1 skipped, all passing),bun run typecheck,bunx oxlintandbunx oxfmt --checkon the changed files, andbunx fallow audit --base origin/main --fail-on-issuesclean.classCandidates.tsis 337 lines.Still not covered: the data flow is one file deep and one hop long. A class list assembled through two intermediate variables, or imported from another module under a name that does not end in
Styles, is still only reached by the name-based anchor.