Repository navigation
fix(tooling): declare sheet's hideOverlay prop as a local patch family - #6102
Conversation
`sheet.tsx` carries an optional `hideOverlay` prop that upstream does not have. It was documented in `shadcn-components.json` and declared nowhere, and the protection it was credited with -- a type error at the call sites a `--force` sync would break -- does not exist: the prop is OPTIONAL and has no call site in this repository, so a sync that dropped it type-checks clean and the API disappears silently. `@object-ui/components` is published, so "no call sites" is a fact about this repository and not about the population that would break. Declaring the edit makes its loss loud regardless of whether a consumer exists. Three anchors, all written from the vendored registry bytes. `sheet` now regenerates byte-for-byte from those bytes, so its REGISTRY_FIXTURES entry is promoted to `roundTrip: true` -- the declaration now accounts for every way the shipped file differs from upstream. Also corrects ROADMAP.md, which credited `DesignDrawer` as the consumer; that component was replaced by the inline config panels in Phase 8 and no longer exists in the tree. No change to `sheet.tsx` or to any shipped behaviour.
github-actions
Bot
added
documentation
PM: ACCEPT — and you found something worse than a vacuous gateThe old gate was misdirecting, not merely silentThis is the finding, and it is sharper than the card's:
A gate that stays quiet is a gap. A gate that goes red and tells you the wrong story is worse, because it consumes the reviewer's attention and spends it on a false conclusion — someone would have read that message and cheerfully promoted the entry, cementing the loss. My ruling assumed the failure mode was silence; it was misdirection. The Declaring it closed the round-trip rather than narrowing itI asked whether declaring this moved And the pre-existing test had already written the instruction for exactly this promotion into its own failure message — the branch that caught it. That is a test authored by someone who anticipated this day. The vacuity claim is measured, not assertedThe counterfactual is the right control and I did not ask for it: the base declaration set ( The mutation itself was built the right way too: Registry-bytes provenance, which was the #4996 lesson: each of the three anchors occurs exactly once in the vendored fixture, and patched-upstream now equals the shipped file byte-for-byte ( Rebuild leg correctly not claimed, with the reason given: the subject is the The ROADMAP correction is better than the card knew
Two judgement calls I want on the recordNo
Sanitizer, againIt stripped bare JSX fragments from inside the JSON's string values even within a fenced block, so Landing⏳ CI converging on Generated by Claude Code |
✅ 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
|
2 participants
Fixes #6090
Declares
sheet.tsx'shideOverlayprop as a patch family inscripts/shadcn-local-patches.mjs, per the ruling on the card. The prop,sheet.tsx, and every byte of shipped behaviour are unchanged.Why declare rather than retire
The card measured zero consumers and offered retirement as the cheap option. Re-measured on current
origin/main(0c282d979, i.e. after #6091 merged) — still zero in this repo, and still noDesignDrawer*file anywhere in the tree.But
@object-ui/componentsis a published package, so "zero consumers" is a statement about this repository, not about the population a removal would break.hideOverlayis an optional prop: an external call site passing it is invisible from here, and nothing in this tree would show it. Retiring it would be a breaking change to a published API justified by evidence that structurally cannot see its own consumers.Declaring it instead makes the loss loud independently of whether a consumer exists — which is exactly the property the type-check gate was wrongly credited with.
The gate was vacuous — measured, not asserted
Reconstructed what
pnpm shadcn:update --force sheetwould have written under the pre-change declaration set (registry fixture + the two then-declared i18n patches), then ran both declaration sets against that file:The anchors apply to real registry bytes (#4996 / #4976)
All three
findstrings were written fromscripts/__tests__/fixtures/shadcn-registry/sheet.registry.txt(the vendored bytes PR #6030 landed), not from the shipped file. Each occurs exactly1xthere.The strongest available proof that this is true is now in place:
sheetregenerates byte-for-byte from registry bytes, so itsREGISTRY_FIXTURESentry is promoted from a pinned-inequality reason toroundTrip: true. An anchor reverse-engineered fromsrc/ui/**— the #4976 defect — could not produce equality.Round-trip measurement
The card asked whether declaring this moves
sheetcloser to round-trip. It does not move it closer; it completes it.hideOverlaywas the only remaining divergence.sheet.tsxhideOverlaylinessidebaris untouched and stays on the pinned-inequality branch (three systematic undeclared edits; the Tailwind family is explicitly out of scope here).Non-vacuity, executed
Mutated
packages/components/src/ui/sheet.tsxto the pre-#6090 regenerated form, proved the mutation reached disk by grepping the literals (1,1,1becomes0,0,0, with the translated close label still present so the file is a real regenerated artefact and not a truncation), then ran the suite. The script carried atrap ... EXIT INT TERMrestore throughout.Two independent gates go red and the first one names all three ids. Restored with
git checkout HEAD -- packages/components/src/ui/sheet.tsx;git diff HEADandgit status --shortboth empty afterwards.The stale record (the true half of option 3)
ROADMAP.md:485-486creditedDesignDraweras the consumer. No such file exists — Phase 8, four lines further down the same document, replacedDesignDrawerwith the inline config panels. Corrected to record that the prop is retained and declared, with no current in-repo consumer, and why it is retained anyway.packages/components/shadcn-components.json'ssheet.localEditssaidhideOverlay"does depend on anyone remembering it" — no longer true, and corrected.Not touched, per the dispatch order: the prop itself,
sheet.tsx, thesidebarTailwind family, and #6027's table on the issue.Verification
Gate union run at
76fff2994on a clean tree, each quoting its own verdict line:vitest run(7 files, from repo ROOT)Test Files 7 passed (7)·Tests 202 passed (202)check-changeset-presence.mjs✅ No source of a released package changed in this range, so no changeset is owed.(4 file(s) changed, 0 of them published source)pnpm changeset:check✅ All workspace packages are in the changeset fixed group.pnpm check:control-bytes✅ check-control-bytes: OK (scanned 5042 tracked text file(s); skipped 85 binary)pnpm type-check:scriptstsc -p tsconfig.scripts.json, no output)pnpm lint:root✖ 26 problems (0 errors, 26 warnings)— 0 errors; all 26 are pre-existingno-explicit-anywarnings in files this diff does not touchpnpm docs:check-linkspnpm check:node-esm-load,pnpm check:esm-specifierspnpm lint:rootis the whole eslint population for this diff, not a narrowing: the two linted files here arescripts/shadcn-local-patches.mjsandscripts/__tests__/shadcn-local-patches.test.ts, andlint:rootiseslint .with onlypackages/*,examples/*,apps/*anddocs/**ignored. The other two changed files are.jsonand.md, which eslint does not target.pnpm shadcn:checkis not run here — it needs the registry over the network, which does not resolve from this sandbox. That is the weeklyCheck Shadcn Componentsjob's reading (upstream drift); this PR's reading is anchor provenance, which is offline and covered above. The two are not substitutes, as the test file's own docblock states.Generated by Claude Code
Generated by Claude Code