From 096fb7d116a38c337dbb507e7433abd25e176a93 Mon Sep 17 00:00:00 2001 From: os-dev Date: Mon, 17 Aug 2026 12:59:02 +0000 Subject: [PATCH 1/2] fix(scripts): re-target the slider delivery patch at the line upstream serves (#4976) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pnpm shadcn:check` has been reporting the declared local patch `slider-thumb-aria-delivery` as unappliable since the day it landed. The file on disk was never the problem: all four markers are present, and `verifyLocalPatches` reports nothing missing. The anchor was. It named the line that forwards the host's accessible name onto `SliderPrimitive.Thumb` — a line objectui added by hand in a014bc00c ("fix Slider accessibility", 2026-04-13), not a line the registry has ever served. Upstream renders the thumb with a class list and nothing else, which slider's own `localEdits` entry in `shadcn-components.json` had said all along. So the anchor matched the local file and could not match upstream: `found 0x`, on the first weekly run after the declaration merged. Re-targeted at upstream's single-line self-closing Thumb, expanding it into the multi-line form that carries the routed props. The payload is unchanged and `packages/components/src/ui/slider.tsx` is not touched, so no behaviour moves. With the anchor correct, the component's only divergence from upstream is the declared patch family, so it now reads as `Identical to upstream` instead of a permanent documented-modified row. The fixture that hid this is replaced with the registry's verbatim bytes, and the assertion that generalises is added: patching those bytes must reproduce the shipped primitive byte for byte. An anchor invented from the local file cannot satisfy that, so the whole class is caught offline on every PR — the module and its "Adding a patch" note now say plainly that `find` is written from registry bytes, never from `src/ui/**`. Fixes #4976 Co-authored-by: Claude --- .../shadcn-slider-delivery-anchor-4976.md | 23 +++ .../__tests__/shadcn-local-patches.test.ts | 162 +++++++++++++----- scripts/shadcn-local-patches.mjs | 60 ++++++- scripts/shadcn-sync.js | 18 +- 4 files changed, 205 insertions(+), 58 deletions(-) create mode 100644 .changeset/shadcn-slider-delivery-anchor-4976.md diff --git a/.changeset/shadcn-slider-delivery-anchor-4976.md b/.changeset/shadcn-slider-delivery-anchor-4976.md new file mode 100644 index 0000000000..c41c871772 --- /dev/null +++ b/.changeset/shadcn-slider-delivery-anchor-4976.md @@ -0,0 +1,23 @@ +--- +--- + +Tooling + test-only (objectui#4976). The declared shadcn local patch +`slider-thumb-aria-delivery` is re-targeted at the line upstream actually serves, so +`pnpm shadcn:check` stops reporting it as unappliable and a future `pnpm shadcn:update` +can re-apply it instead of refusing to write. + +The anchor as first declared named the line that forwarded the host's accessible name +onto the slider thumb — a line objectui added by hand in commit `a014bc00c`, never a line +the registry has served. Upstream renders the thumb with a class list and nothing else, +which slider's own `localEdits` entry in `shadcn-components.json` had recorded all along. +So the anchor matched the file on disk and could not match upstream: the first weekly +check after it landed reported `found 0x` while the shipped primitive was perfectly +correct. Only the anchor moved; the patch payload, and therefore every byte of +`packages/components/src/ui/slider.tsx` and its behaviour, is untouched — the file is not +edited by this change at all. + +The test fixture that hid it is replaced with the registry's verbatim bytes, and the new +assertion is the one that generalises: applying the declared patches to those bytes must +reproduce the shipped primitive byte for byte. An anchor invented from the local file +cannot satisfy that, so the class of mistake is now caught offline on every PR rather +than by the next weekly sync. No published package changes, hence "no release". diff --git a/scripts/__tests__/shadcn-local-patches.test.ts b/scripts/__tests__/shadcn-local-patches.test.ts index 3c368fe88f..6c79738f33 100644 --- a/scripts/__tests__/shadcn-local-patches.test.ts +++ b/scripts/__tests__/shadcn-local-patches.test.ts @@ -15,6 +15,11 @@ import { patchedComponents, describePatchFailure, } from '../shadcn-local-patches.mjs'; +// The licence header `updateComponent` prepends on write. Imported rather than +// re-typed so the round-trip assertion below strips exactly what the sync adds; +// importing the CLI module is safe (it only runs `main()` when it IS the +// process entry point) and `shadcn-sync-fetch-cache.test.ts` already does it. +import { OBJECTUI_HEADER } from '../shadcn-sync.js'; /** * objectstack#5505 — the Shadcn `Sheet`/`Dialog` primitives shipped a hardcoded @@ -82,6 +87,58 @@ const DialogContent = React.forwardRef((props, ref) => ( )) `; +/** + * What the registry serves for `slider`, after `rewriteRegistryImports` (so + * `@/lib/utils` already reads `../lib/utils`) — the exact input the patch engine + * sees during `--update`. + * + * Verbatim and complete, not an excerpt, and that is the whole point: this is + * the only offline statement of what upstream actually looks like, so the + * round-trip assertion below can hold it to the shipped primitive byte for + * byte. Transcribed from `https://ui.shadcn.com/r/styles/default/slider.json`, + * which shadcn-ui/ui checks in verbatim as + * `apps/v4/public/r/styles/default/slider.json` — read there at HEAD + * 8a7701ec27eb9cb8e0377db769fbe6d744113c52, where the sha256 of + * `files[0].content` is + * 48bd0ba32cc7f341ecca995374be73111da2f761694cfcf91dbf8d4d9e632c06. + * + * objectui#4976: the fixture this replaces was reverse-engineered from the + * LOCAL file instead, so it carried objectui's own hand-written accessible-name + * forwarding (commit a014bc00c) as though upstream had shipped it, and trimmed + * the rest to a sketch. The delivery patch was anchored on that invented line + * and could not match any real registry response — while this test stayed green, + * because the fixture agreed with the anchor rather than with upstream. + */ +const UPSTREAM_SLIDER = `"use client" + +import * as React from "react" +import * as SliderPrimitive from "@radix-ui/react-slider" + +import { cn } from "../lib/utils" + +const Slider = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({ className, ...props }, ref) => ( + + + + + + +)) +Slider.displayName = SliderPrimitive.Root.displayName + +export { Slider } +`; + describe('shadcn local patches — application to fresh upstream (objectstack#5505)', () => { it.each(closeLabelComponents)('%s declares the i18n close patch', (name: string) => { const ids = LOCAL_PATCHES[name].map((p: { id: string }) => p.id); @@ -114,36 +171,8 @@ describe('shadcn local patches — application to fresh upstream (objectstack#55 }); it('applies the slider family to fresh upstream, and is idempotent', () => { - // The upstream shape the registry serves, after `rewriteRegistryImports`. - // `aria-label` on the thumb is upstream's OWN hand-written bridge — the - // anchor the delivery patch replaces, and the standing evidence that the - // thumb cannot be addressed from outside the primitive. - const upstream = `"use client" - -import * as React from "react" -import * as SliderPrimitive from "@radix-ui/react-slider" - -import { cn } from "../lib/utils" + const once = applyLocalPatches('slider', UPSTREAM_SLIDER); -const Slider = React.forwardRef< - React.ElementRef, - React.ComponentPropsWithoutRef ->(({ className, ...props }, ref) => ( - - - - -)) -`; - - const once = applyLocalPatches('slider', upstream); expect(once.failed).toEqual([]); expect(once.applied).toHaveLength(4); expect(verifyLocalPatches('slider', once.content)).toEqual([]); @@ -151,32 +180,71 @@ const Slider = React.forwardRef< // `props` object any more — the two halves that must land together. expect(once.content).toContain('{...splitSliderThumbProps(props).thumb}'); expect(once.content).toContain('{...splitSliderThumbProps(props).root}'); - expect(once.content).not.toContain('aria-label={props["aria-label"]}'); const twice = applyLocalPatches('slider', once.content); expect(twice.applied).toEqual([]); expect(twice.content).toBe(once.content); }); - it('refuses when upstream restructures the thumb away', () => { - // A future registry drop of the hand-written `aria-label` bridge takes the - // delivery patch's anchor with it. That must be a hard failure the operator - // sees, not a sync that quietly ships an unreachable thumb again. - const withoutThumbAnchor = `import { cn } from "../lib/utils" + /** + * The assertion that would have caught objectui#4976 on the day it landed. + * + * Patching real registry bytes must reproduce the primitive we ship, byte for + * byte. Anything weaker is a claim nobody can check: an anchor invented from + * the local file matches the local file quite happily, and a hand-trimmed + * "faithful excerpt" of upstream can be wrong about the very line a patch + * targets without one assertion noticing. Equality can only hold if every + * anchor in the family was written from registry bytes — which is exactly the + * property the declaration needs and cannot otherwise state offline. + * + * When this goes red, the question is which side moved. Upstream drifting is + * the expected cause: re-target the patch against the new bytes, refresh this + * fixture from the registry, and if the incoming shape is an improvement take + * it into `src/ui/slider.tsx` too. What red must never mean is "trim the + * fixture until it agrees again". + */ + it('regenerates the shipped slider.tsx byte for byte from registry bytes', () => { + const patched = applyLocalPatches('slider', UPSTREAM_SLIDER); + const shipped = fs.readFileSync(path.join(uiDir, 'slider.tsx'), 'utf-8'); + + expect(patched.failed).toEqual([]); + // The header is added on write, downstream of the patch engine, so it is + // stripped here rather than baked into the fixture. + expect(patched.content).toBe(shipped.replace(OBJECTUI_HEADER, '')); + }); -const Slider = React.forwardRef< - React.ElementRef, - React.ComponentPropsWithoutRef ->(({ className, ...props }, ref) => ( - - - -)) -`; + it('refuses when upstream restyles the thumb', () => { + // The realistic churn: shadcn tweaks the thumb's class list. The anchor + // names that list verbatim, so a tweak takes the anchor with it. Loud is + // the correct outcome — the operator has to re-target, and pick up the new + // classes while doing so — but it must be loud about the DELIVERY patch + // only, not smear across the family. + const restyled = UPSTREAM_SLIDER.replace('block h-5 w-5', 'block size-4'); + expect(restyled).not.toBe(UPSTREAM_SLIDER); + + const result = applyLocalPatches('slider', restyled); + + expect(result.failed.map((p: { id: string }) => p.id)).toEqual([ + 'slider-thumb-aria-delivery', + ]); + expect(result.failed[0].found).toBe(0); + // The other three anchors are independent of the class list and still land. + expect(result.applied).toHaveLength(3); + // And the delivery payload is NOT in the content: a caller that ignored + // `failed` would ship an unreachable thumb again. + expect(result.content).not.toContain('splitSliderThumbProps(props).thumb'); + }); + + it('refuses when upstream drops the thumb element altogether', () => { + // The structural case: no thumb to deliver to. Radix would still render one + // internally, so this compiles and renders — the patch must refuse rather + // than let a sync quietly ship a slider nothing can address. + const withoutThumb = UPSTREAM_SLIDER.replace(/^ {4} p.id)).toEqual([ 'slider-thumb-aria-delivery', diff --git a/scripts/shadcn-local-patches.mjs b/scripts/shadcn-local-patches.mjs index dc6b2d82bd..e6d12d603b 100644 --- a/scripts/shadcn-local-patches.mjs +++ b/scripts/shadcn-local-patches.mjs @@ -43,6 +43,14 @@ * a small anchored reference survives upstream churn far better than an * inlined implementation, and the implementation itself stays reviewable and * unit-testable in a normal file. + * + * Write `find` from REGISTRY bytes, never from `src/ui/**`. The local file is + * the patched artefact, so any line an earlier local edit left there reads as a + * perfectly good anchor: it matches the file in front of you and matches + * nothing upstream, which makes the patch dead on arrival while looking + * correct. objectui#4976 is the worked example. The round-trip assertion in + * `scripts/__tests__/shadcn-local-patches.test.ts` is what holds a new anchor + * to this rule offline, without a network round-trip. */ /** @@ -175,17 +183,46 @@ const sidebarCookieReadPatches = [ * the same defect one step easier) a widget has NO handle on the element that * must carry those facts. Hence a declared `thumbProps`. * - * Four one-liners, each anchored on a line upstream has carried across every - * sync in this file's history. As with the families above, the payload stays - * OUT of `src/ui/`: the routing and its reasoning live in + * Four patches. Three are one-liners; the delivery half expands upstream's + * single-line self-closing Thumb into the multi-line form that can carry the + * routed props. As with the families above, the payload stays OUT of + * `src/ui/`: the routing and its reasoning live in * `packages/components/src/lib/slider-thumb.ts`. * * The root half is patched too, and that is not optional: leaving `thumbProps` * in Root's spread would stringify it onto the wrapper (`thumbprops="[object * Object]"`), the exact leak objectui#3291 sweeps for. * + * ## The delivery anchor was re-targeted once (objectui#4976) + * + * As first declared, the delivery patch anchored the line that forwarded the + * host's accessible name onto the thumb — a line objectui added by hand in + * commit a014bc00c ("fix Slider accessibility", 2026-04-13), NOT a line the + * registry has ever served. Upstream renders the thumb with a className and + * nothing else, and slider's own `localEdits` entry in `shadcn-components.json` + * said exactly that the whole time. So the anchor matched the file on disk and + * could never match upstream: the first `--check` after it landed reported the + * patch unappliable (`found 0x`) while the shipped file was perfectly correct. + * + * Re-targeting it moved the anchor only — the payload, and therefore every byte + * of the shipped primitive's behaviour, is untouched. + * * @type {LocalPatch[]} */ +/** + * Upstream's thumb class list, spelled exactly once. + * + * The delivery patch has to name it twice — the single-line element it matches + * and the expanded element it writes back — and a list this long is precisely + * the kind of string that drifts one character between two copies. One spelling + * means the anchor and its replacement cannot disagree, the same reason + * SIDEBAR_COOKIE_NAME is passed into the sidebar patch rather than retyped. + */ +const UPSTREAM_THUMB_CLASSNAME = + 'block h-5 w-5 rounded-full border-2 border-primary bg-background ring-offset-background ' + + 'transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring ' + + 'focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50'; + const sliderThumbPassThroughPatches = [ { id: 'slider-thumb-import', @@ -230,11 +267,18 @@ const sliderThumbPassThroughPatches = [ issue: 'objectui#3318', reason: 'Delivers the host\'s control-channel facts to the element that can carry ' + - 'them. Replaces (and preserves) the narrower hand-written `aria-label` ' + - 'bridge upstream already needed here for the very same reason — proof the ' + - 'thumb is unreachable from outside, not a new claim.', - find: ' aria-label={props["aria-label"]}\n', - replace: ' {...splitSliderThumbProps(props).thumb}\n', + 'them. Upstream renders the thumb with a class list and nothing else, so ' + + 'this is the only route to the focusable span — hence the anchor expands ' + + 'the self-closing element instead of replacing an attribute on it. The ' + + 'spread sits after `className`, the precedence the shipped primitive ' + + 'already gives the routed props. Re-targeted in objectui#4976: the first ' + + 'anchor named a local-only line, so it never matched a registry response.', + find: ` \n`, + replace: + ' \n', marker: 'splitSliderThumbProps(props).thumb', occurrences: 1, }, diff --git a/scripts/shadcn-sync.js b/scripts/shadcn-sync.js index 9007275d6c..edfea8c370 100755 --- a/scripts/shadcn-sync.js +++ b/scripts/shadcn-sync.js @@ -1064,6 +1064,18 @@ if (invokedAsCli()) { }); } -// Exported for scripts/__tests__/shadcn-sync-fetch-cache.test.ts. The CLI is -// the only production consumer; nothing else imports this module. -export { fetchUrl, fetchRegistry, isRegistryEntry, cacheFileFor, cacheStats, bodySnippet, CACHE_TTL_MS }; +// Exported for scripts/__tests__/shadcn-sync-fetch-cache.test.ts and +// scripts/__tests__/shadcn-local-patches.test.ts (OBJECTUI_HEADER, so the +// round-trip assertion strips the header this script prepends rather than +// keeping a second spelling of it). The CLI is the only production consumer; +// nothing else imports this module. +export { + fetchUrl, + fetchRegistry, + isRegistryEntry, + cacheFileFor, + cacheStats, + bodySnippet, + CACHE_TTL_MS, + OBJECTUI_HEADER, +}; From 87098b8fc4d36a8365d78b4379c45420a0abfcf5 Mon Sep 17 00:00:00 2001 From: os-dev Date: Mon, 17 Aug 2026 13:08:53 +0000 Subject: [PATCH 2/2] fix(scripts): keep the LocalPatch[] JSDoc attached to the slider family array The shared thumb class-list constant was inserted between the family docblock and the array it annotates, so `@type {LocalPatch[]}` documented the string instead. Moved above the docblock; no behaviour change. Co-authored-by: Claude --- scripts/shadcn-local-patches.mjs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/scripts/shadcn-local-patches.mjs b/scripts/shadcn-local-patches.mjs index e6d12d603b..f6cdbae86b 100644 --- a/scripts/shadcn-local-patches.mjs +++ b/scripts/shadcn-local-patches.mjs @@ -167,6 +167,20 @@ const sidebarCookieReadPatches = [ }, ]; +/** + * Upstream's thumb class list, spelled exactly once. + * + * The delivery patch below has to name it twice — the single-line element it + * matches and the expanded element it writes back — and a list this long is + * precisely the kind of string that drifts one character between two copies. One + * spelling means the anchor and its replacement cannot disagree, the same reason + * SIDEBAR_COOKIE_NAME is passed into the sidebar patch rather than retyped. + */ +const UPSTREAM_THUMB_CLASSNAME = + 'block h-5 w-5 rounded-full border-2 border-primary bg-background ring-offset-background ' + + 'transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring ' + + 'focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50'; + /** * The slider thumb pass-through patch (objectui#3318). * @@ -209,20 +223,6 @@ const sidebarCookieReadPatches = [ * * @type {LocalPatch[]} */ -/** - * Upstream's thumb class list, spelled exactly once. - * - * The delivery patch has to name it twice — the single-line element it matches - * and the expanded element it writes back — and a list this long is precisely - * the kind of string that drifts one character between two copies. One spelling - * means the anchor and its replacement cannot disagree, the same reason - * SIDEBAR_COOKIE_NAME is passed into the sidebar patch rather than retyped. - */ -const UPSTREAM_THUMB_CLASSNAME = - 'block h-5 w-5 rounded-full border-2 border-primary bg-background ring-offset-background ' + - 'transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring ' + - 'focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50'; - const sliderThumbPassThroughPatches = [ { id: 'slider-thumb-import',