Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-detail): resolve headerColor through literal Tailwind classes, not a template literal - #6294
Merged
Merged
Conversation
`DetailSection` built the header background as `bg-` concatenated with the authored `headerColor`. Tailwind v4 scans source TEXT for complete class tokens and this workspace ships no `bg-*` safelist, so that expression contributed nothing to the compiled stylesheet: compiling the console's CSS with it deleted produced a byte-identical sheet (same sha256). An authored value rendered only where another file happened to author the identical class literally. Resolve through a lookup of complete class literals instead, the shape `useRowColor` already uses in @object-ui/plugin-grid: tint-only design-system classes (`muted`, `muted/50`, `accent`, `primary/10`, `secondary/10`, `destructive/10`), verbatim pass-through for a value that is already a `bg-*` class, and no class at all for anything else — never a fabricated one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mn4BZ5AVDM81pvfij1WwM9
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-litant
marked this pull request as ready for review
August 25, 2026 10:27
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes#6178
The premise, re-measured
The card is correct, and the strongest evidence it predicted exists.
The repo's own reading.
apps/console/src/__tests__/sdui-preview-page-source-styling.test.ts, header:The compiled stylesheet, not an inference. I compiled
apps/console/src/index.cssthe way the console build does (@tailwindcss/postcss, v4.3.3), then again with thebg-template literal deleted from both call sites. The two sheets are byte-identical — 441302 bytes, sha256dffcd88d…e7e3b2. That call site contributed zero rules. Asking Tailwind's design system for the candidatebg-(all the extractor could take from a template literal) returns no rule either."Works only by collision" — measured, and it matters. Both values the
@object-ui/typesmirror documents are present in the built console CSS today:.bg-mutedand.bg-primary/10are 2 of 227 distinctbg-*selectors. They are there because other files author them literally —bg-muted691 times,bg-primary/1063 times elsewhere in the workspace. So the key was not inert in effect; it was live by accident, and the fix had to keep the documented examples working rather than simply stop lying.Two corrections to the card's wording, neither changing its conclusion:
apps/console/src/index.cssdoes use@source inline(...)— nine lines of it, for streamdown's runtime classes. There is nobg-*safelist, which is what the argument needs.headerColormention ispackages/plugin-detail/src/index.tsx:310(not:287), and it is thedetail-sectionregistry input row — a declaration, not a fourth read site. The two read sites areDetailSection.tsx:513/:542, exactly as filed.The issue body shows no sanitizer truncation: it contains no angle-bracket fragments at all, and the split spelling of the template literal in bullet 1 reads as the author working around the sanitizer, not as damage.
The shape, decided by measurement
Triage asked for a closed vocabulary mapped to static literal classes or inline style from the authored value — whichever matches how sibling colour keys here already solved it. Three sibling species exist:
getBadgeHexAppearance(@object-ui/fields)bg-[color:var(--os-badge-bg)]+ inline custom properties — the AGENTS.md carve-outcolumnColors(PivotTable)cn(), no prefixuseRowColor(@object-ui/plugin-grid)bg-pass-through +undefinedotherwiseuseRowColoris the same problem, and this PR is the same answer. The custom-property carve-out does not fit: it is for values that are CSS colours, andheaderColor's values (muted,primary/10) have no meaning as one. AndDetailSectionwas the only`bg-${…}`-shaped class in the entire workspace — every sibling had already avoided this shape.New module
packages/plugin-detail/src/headerColor.ts:muted,muted/50,accent,primary/10,secondary/10,destructive/10.CardHeadersets no foreground, so solidbg-primary/bg-destructivewould leave the title unreadable — those need a pairedtext-*-foregroundand are left to the pass-through, where the pairing is the author's explicit choice. Both documented examples are in the map, so nothing that rendered before renders differently;bg-*class. New behaviour:headerColor: 'bg-muted'previously produced the meaninglessbg-bg-muted;undefinedotherwise — never a fabricated class.hasOwnProperty, not a bare index, soheaderColor: 'constructor'cannot hand back anObject.prototypemember.The literals now live in a file every consuming app scans (
packages/plugin-detail/src/**appears inapps/console,examples/console-starter,examples/byo-backend-console), so the key works because this module declares it, not because a neighbour happens to.Evidence
Two instruments, because one of them is blind. Asserting a
classNamein the DOM proves nothing about CSS emission — the old code putbg-<value>in the DOM too. So:DetailSection.headerColor.test.tsx— renders the real component, asserts which class reaches the header. States in its header that it cannot show CSS generation.headerColor.test.ts— the generation half. (a) every vocabulary class appears verbatim in the module's source text, the property the v4 extractor needs; (b) every vocabulary class builds a rule against this workspace's real@theme(__unstable__loadDesignSystemfromtailwindcss, root devDependency), withbg-,bg-mutedd,bg-not-a-tokenas the negative controls that prove the instrument can say no. It states what it still cannot show: it does not run the oxide scanner, and it does not verify any app's@sourceglobs.Every negative pin has a positive probe beside it (the header element is located
getBy-style and the assertion fails if no header rendered, so nothing passes vacuously).Per-point ablation — direction predicted before each run, all four correct:
bg-muted→bg-mutedd(still a complete literal)'bg-muted'→['bg-', 'muted'].join('')(still a valid utility at runtime)P4 is #6178 reproduced in miniature: a class that is valid at runtime but unextractable from source, with every instrument except the source-text one reporting green. P1/P2 record the honest limit of the render tests — under P1 the two "a mapped token renders its literal class" cases still passed, because the old concatenation produced the same string for a mapped value.
Each ablation script carries a
trap … EXIT INT TERMrestore and refuses to run (exit 90) unless the mutation is confirmed on disk; the restore leg is printed each time.Gates
Run at
799d2385e, which is the HEAD of this branch and byte-identical to the tree every measurement above was taken on (git status --porcelainempty).pnpm exec vitest run packages/plugin-detail— 106 files / 1007 tests passed (the two new files contribute 21).pnpm --filter @object-ui/plugin-detail type-check— clean (tsc --noEmit && tsc -p tsconfig.test.json, both echoed).check:control-bytes,check:phantom-deps,check:entry-guard,check:self-import,check:vi-mock-specifiers,check:esm-specifiers,check-type-check-coverage,check-lint-coverage,check-changeset-presence,check-changeset-no-major,check-changeset-fixed— all exit 0. Changeset gate prints:4 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s).check:eager-closure("No eager-closure report atapps/console/dist/eager-closure.json… an absent report means the console was not built") andcheck:readme-exports("its type entry./dist/index.d.tsis not on disk — runpnpm buildfirst"). This worktree had 0 of 40 packages built and noapps/console/distwhen they ran; CI builds first. Neither names a file in this diff.Lint, run narrowed and the narrowing declared.
pnpm exec eslint packages/plugin-detail --format json— exit 0, 0 errors across the 159 files eslint's own config resolution judged (count read from the JSON, not from my guess at what counts). My four files:headerColor.ts0/0,headerColor.test.ts0/0,DetailSection.headerColor.test.tsx0/0,DetailSection.tsx0 errors and 12 pre-existing warnings at lines 70/102/105/111/122/147/221/223/261/261/263/428 — none on the three lines this PR touches (37, 514, 543). The narrowing is a measurement rather than a skip becauseeslint.config.jsconfigures no type-aware linting (noprojectService, noparserOptions.project) and no custom rule ineslint-rules/reads another file, so a file's verdict is a function of its own text and the shared config — neither of which this diff moves outsidepackages/plugin-detail. Repo-widepnpm lintis CI's run.Scope
packages/plugin-detail/src/**and its tests only, as dispatched. Deliberately not touched:packages/types/src/views.ts/zod/views.zod.ts— theheaderColordeclaration and its@example 'muted', 'primary/10'. Out of this PR's file surface, and objectui#6267 holds that package. The examples stay accurate: both still render. Naming the vocabulary in that JSDoc is a follow-up for whoever owns it next.detail-sectionregistry input row (index.tsx:310) — astringinput, still accurate.@objectstack/spec, which refusesheaderColoron the strictrecord:detailssection schema today, with a pinned test whose comment cites this exact defect ("declaring it would advertise a capability the renderer does not deliver"). That refusal is the thing this PR makes obsolete; flipping it is a spec decision, raised as an open question in the dev report rather than taken here. Back-link: record:details sections: five more renderer-honoured keys remain undeclared by the strict section schema (title / defaultCollapsed / icon / description / headerColor) objectstack#11661.The one thing a reviewer should rule on
The declared type is unchanged and no value is rejected, but the effective value set is reshaped: an undocumented bare suffix that happened to collide with another file's literal (say
headerColor: 'blue-100') no longer reaches the DOM asbg-blue-100. Ending that accidental liveness is the point of the fix, not a side effect of it — no fix satisfying the card can preserve it. Three things bound the risk: zero producers authorheaderColoranywhere in either repo (only declarations and the spec-side refusal); both documented examples are preserved; and every previously-colliding suffix has a one-word migration through the pass-through ('blue-100'→'bg-blue-100'). Whether the vocabulary should be exactly these six, and whether the spec should now declare the key over them, is the maintainer's call.Generated by Claude Code