docs(components): record why the eight small Shadcn divergences exist, and split --check into documented vs undocumented - #3054
Merged
Conversation
…, and split --check into documented vs undocumented Triaged the eight components diverging from upstream by 1–2 lines, on the theory that small divergences would be stale cruft worth dropping to widen the safe-to-sync set. That theory was wrong. **All eight are load-bearing; none can be synced away.** Six are the Tailwind v4 arbitrary-value migration (`popover`, `tooltip`, `hover-card`, `menubar`, `dropdown-menu`, `context-menu`). Upstream still ships v3's `origin-[--radix-…]`, which on Tailwind 4.x compiles to a bare `transform-origin: --radix-…` instead of `var(--radix-…)` — invalid CSS that browsers drop, leaving the open/close animation scaling from the default origin rather than the one Radix computes. Verified by compiling both forms against the installed tailwindcss 4.3.3: the paren form emits `var()`, the bracket form does not. Applied repo-wide in 925051d, with zero v3-form occurrences left. `form` adds `Record<\`data-${string}\`, string | undefined>` to FormItem so callers can attach a stable `data-testid` to the field wrapper — the ADR-0054 testability contract again, same family as `command`'s `contentProps`. `sonner` re-exports `toast` so consumers take it from @object-ui/components rather than depending on `sonner` directly; plugin-form and plugin-kanban import it that way. So the deliverable inverts. Nothing gets synced; instead each reason is recorded in a new `localEdits` field on the manifest entry, and the script surfaces it — printed in full when `--update` refuses, and reduced to a `[documented]` / `[UNDOCUMENTED]` marker in `--check`, which now splits the Modified bucket: documented (8): context-menu, dropdown-menu, form, hover-card, menubar, popover, sonner, tooltip UNDOCUMENTED (9): badge, calendar, chart, command, select, sheet, sidebar, slider, table That split is the point. Divergence with a recorded reason is a decision; divergence without one is an open question, and until someone writes it down there is no way to tell a deliberate fix from stale drift — which is exactly the ambiguity that let `resizable` rot for a major version. Manifest only; no component source changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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
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.
I proposed triaging the eight components diverging from upstream by 1–2 lines, on the theory they'd be stale cruft worth dropping to widen the safe-to-sync set.
The theory was wrong. All eight are load-bearing; none can be synced away. The safe-to-sync count cannot go up, so the deliverable inverts: instead of removing divergence, this records why each one exists.
What they actually are
Six are the Tailwind v4 arbitrary-value migration —
popover,tooltip,hover-card,menubar,dropdown-menu,context-menu.Upstream still ships v3's
origin-[--radix-…]. I compiled both forms against the installed tailwindcss 4.3.3:origin-(--x)— v4 shorthand, ourstransform-origin: var(--x)origin-[--x]— v3 bracket, upstreamtransform-origin: --xA bare custom-property name isn't a value, so syncing would leave every one of these popovers animating from the default origin instead of the one Radix computes. Applied repo-wide in
925051db6, with zero v3-form occurrences left.formaddsRecord<\data-${string}`, string | undefined>toFormItemso callers can attach a stabledata-testidto the field wrapper — the ADR-0054 testability contract, same family ascommand'scontentProps`.sonnerre-exportstoastso consumers take it from@object-ui/componentsrather than depending onsonnerdirectly.plugin-form,plugin-kanbanand others import it that way; syncing breaks those imports.What ships instead
A
localEditsfield on each manifest entry, surfaced by the script:--updaterefuses, so the reason arrives at the moment someone is about to override it[documented]/[UNDOCUMENTED]marker in--checkThat split is the point. Divergence with a recorded reason is a decision; divergence without one is an open question — and that ambiguity is exactly what let
resizablerot for a major version (#3029).Verification
greppattern treating(as a regex group, not a real result — re-run with-F, all five forms behave as tabled.sonner'stoastre-export confirmed to have real consumers acrossplugin-form/plugin-kanbanJSON.stringify(…, null, 2));--check,--diff,--update, and the fix(scripts): shadcn-sync rewrites the registry paths Shadcn actually serves, and refuses to write a file when it cannot #3033 / fix(scripts): shadcn-sync refuses to silently delete local edits, and compiles the package after it writes #3035 guards all regression-checkedeslintclean. No component source changes — manifest and script onlyNo changeset.
🤖 Generated with Claude Code