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..f6cdbae86b 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. */ /** @@ -159,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). * @@ -175,15 +197,30 @@ 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[]} */ const sliderThumbPassThroughPatches = [ @@ -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, +};