Repository navigation
fix(components): a bare-string trigger renders on all eight sibling overlay blocks - #9796
Conversation
… overlay blocks (objectui#9710) The defect objectui#9701 repaired on `collapsible`, in eight more places: the authored `trigger` slot handed straight to a Radix `*Trigger` with `asChild` written unconditionally. `asChild` resolves the primitive to its `Slot`, which merges onto its child through `React.Children.only` — a single React element and nothing else — while both published faces admit a bare string on the key. So `trigger: 'Open it'` validated twice and then threw into `SchemaRenderer`'s error boundary. `asChild` now asks Radix's own structural question through one shared seam, `asChildSlotProps`, rather than eight copies of the predicate: a multi-node array and an empty slot's `null` are the same structural mismatch wearing different values and take the same arm, and the element arm keeps the merge. `context-menu` declares the same key and is deliberately untouched — its `asChild` child is a real `div`, so the rendered slot goes inside it and `Children.only` never sees the string. The class pin enumerates the REGISTRY for blocks declaring a `trigger` slot rather than naming files, so a ninth key is covered the day it registers one; that property is itself asserted by registering a deliberately defective block inside the test and observing the pin's own assertion fail on it. No published face moves: `packages/types` is untouched. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
A per-block `expect` inside the loop stopped at the first failure and reported a single file, which for a card whose subject is a class of eight is the wrong diagnostic. The survey now runs the whole population first and asserts on the LIST of blocks that reached the error boundary, so one red run prints all of them. `REDDENS_FOR_A_NINTH` asserts that same list rather than a paraphrase of it: with a deliberately defective block registered, the list holds exactly that block's name and nothing else. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
|
changeset-claim-re-read
|
✅ 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
|
Contract reviewServed-tier: ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: FAIL Generated by Claude Code |
…wo arms are separated Contract review found the round-1 seam regressed a legal input: a document that omits `trigger` rendered one empty focusable button where the base rendered nothing, on every key whose zod face carries `.optional()`. An empty `*Trigger` still paints its own element, and an empty `button` is an unlabeled tab stop with nothing to read. The stated mechanism is what hid it, and it was false. Read from the lockfile-pinned `@radix-ui/react-slot`, the throwing branch is guarded by `if (children || children === 0)`, so a `null` child is returned as-is: the empty arm NEVER threw. It is not the same structural mismatch as a bare string or a multi-node array, and the changeset no longer says it is — that sentence publishes verbatim to the CHANGELOG. The seam is now `renderTriggerSlot`, which carries both rules in one place: render nothing when the slot is empty (`renderNodeSlot`'s own contract — chrome that disappears with its content), and ask Radix's structural question about `asChild` otherwise. A call site that reached for one rule and forgot the other is what this replaces. `OMITTED_TRIGGER_PAINTS_NOTHING` pins the restored arm. Its population is read from the published zod faces, NOT written down: the `.optional()` wrapper is the operative fact, because `shape.trigger.isOptional()` is a dead instrument on these keys — `SchemaNodeSchema` itself admits `undefined`, so it answers true for all ten and cannot separate `dialog` from `popover`. `ZOD_FACES_READ` asserts the derivation discriminates, so the arm cannot go vacuously green. `ASCHILD_STILL_ON` no longer uses a `button button` selector, which could not see over-reach on `HoverCardTrigger` (an anchor) or `ContextMenuTrigger` (a span). It now asks whether an ancestor wearing Radix's trigger wiring is one of the tags Radix draws, with `div` excluded and the reason named: `context-menu` authors its own `div` and the primitive correctly merges onto it. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
✅ 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
|
The instrument is now blind — the pin is the only live onePosted by the dispatching After this repair the eight call sites carry no ⛔ Why this is a comment and not an edit to the PR body — a correction against this seatThe dispatch asked the dev to put this note in the PR body. That instruction was wrong, and the dev was right to refuse it rather than comply silently. The standing ⭐ And the durable half was already handled by the dev without being asked: the same note is committed in the
Generated by Claude Code |
Contract reviewServed-tier: ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: FAIL Generated by Claude Code |
…d button, and its own failure is now observed Re-review measured the previous detector at 1-of-8 on this card's own renderers — weaker than the `button button` selector it replaced, which caught seven and missed only hover-card. The hole was never the `div` exclusion, which is correct and necessary: context-menu's authored div and collapsible's root div both wear `data-state` legitimately. It was the authored-element finder. It took the FIRST button whose `textContent` matched the label — and when a primitive WRAPS the authored button, the Radix-drawn wrapper carries the same `textContent` and comes first in document order. So the finder returned the wrapper, `closest` searched above it, and the over-reach read as clean. Seven of the eight draw a button wrapper; only hover-card, which draws an anchor, was still caught. The finder now takes the innermost match — the button containing no button of its own. `OVER_REACH_IS_VISIBLE` makes the failure standing rather than argued: a block that withholds `asChild` where Radix could have served is registered inside the test on a BUTTON-drawing primitive, and the list `ASCHILD_STILL_ON` asserts empty is observed holding exactly that block. With the first-match finder that list reads empty and this control reddens — so it guards the very regression this commit repairs, on every run, instead of resting on the reasoning above. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
✅ 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
|
Contract reviewServed-tier: ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Contract-review gate CLEARED — provenance
⛔ No self-review at any point: served tier measured ⭐ What the three rounds actually bought
⭐ The through-line worth keeping: twice, a green suite plus a plausible mechanism sentence covered a real defect — and both times what exposed it was ablating the thing the control exists to catch and reading what it actually named.
|
…ger seam moved CI went red on three of four shards: a lit control in `overlay-node-slot-doc-types-7082.test.ts` asserted the alert-dialog renderer contains `renderChildren(schema.trigger)`, and objectui#9710 moved that spelling onto a shared seam. The control licensed a zero-hit claim about `AlertDialogSchema.actions`, so it had to keep proving the scan can find things in that file — it now reads `renderChildren(schema.content)`, a sibling slot the trigger work does not touch, rather than re-creating the coupling that just broke. Running the whole `packages/types` suite rather than the named file found what a path grep could not: `overlay-trigger-union-7081.test.ts` builds its renderer path from a constant, and nine of its assertions rested on the same spelling. - The per-member read-site assertion accepts both spellings. Its claim is unchanged — the renderer READS `schema.trigger`, which is what licenses the widened declaration — and `context-menu`, never part of the repair, still calls `renderChildren` directly. - The chain is pinned end to end instead of one hop: the seam must reach `renderChildren`, or "the renderer calls the seam" would leave the array form's read site unpinned. - The docblock pin no longer requires a trailing `.tsx:`. It was demanding a cross-file LINE ADDRESS — the form AGENTS.md #11 bans outright, and the seven addresses it required were rotted by this very change. The file path carries the claim; the line number never did. The eight `trigger` docblocks in `overlay.ts` named a call that no longer exists, in JSDoc that ships in `.d.ts` and shows in an author's editor. They now name the current spelling, without line addresses. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
✅ 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
|
…r true, and the claim now lives in one place The sentence was true when it was written and this branch's own CI-fix round falsified it: `packages/types/src/overlay.ts` moved by 62 lines. A changeset that declares a patch release for a package while stating that package is untouched contradicts itself, and it publishes verbatim to the CHANGELOG — where the reader it misleads is the one diffing their `.d.ts` after the bump. The claim this change actually owns is narrower and survives: no type, no accept set, no export and no runtime behaviour moves in `@object-ui/types`, and every changed line in that file is a docblock line. What moves there is documentation that SHIPS. ⛔ The two paragraphs are folded into one rather than both narrowed. The defect here was one claim stated in two places, where the copy that rotted was not the copy being maintained; narrowing both would have kept that shape. Verified before publishing rather than asserted: every changed line in `overlay.ts` is a docblock line, and in the emitted `dist/overlay.d.ts` the corrected spelling is present on the eight while the old spelling is absent — same file, same instrument, so the zero is admissible. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
✅ 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 #9710
Clause-②: yespackages/types/src/zod/overlay.zod.tsis untouched and no published face moves, so the conservativeyeson the claim may be overturned tonoby review — that is the designed outcome of a conservative declaration, not a fault. The reasoning is in "Does any of them want a bare string at all?" below.What lands
All eight sibling renderers in one change, plus a class-level pin that reddens — the binding direction from triage
5719728719:overlay/alert-dialog.tsxAlertDialogTriggeroverlay/dialog.tsxDialogTriggeroverlay/drawer.tsxDrawerTriggeroverlay/dropdown-menu.tsxDropdownMenuTriggeroverlay/hover-card.tsxHoverCardTriggeroverlay/popover.tsxPopoverTriggeroverlay/sheet.tsxSheetTriggeroverlay/tooltip.tsxTooltipTriggerThe predicate from PR objectui#9708 is lifted into ONE seam,
renderTriggerSlotinpackages/components/src/lib/utils.tsx, rather than eight copies: it renders the slot once and asks Radix's own structural question of the result. WhereSlotcannot serve, the primitive renders its own element around the content; where it can, the merge is unchanged.⛔ RETRACTED — an earlier revision of this body claimed that a multi-node trigger array and an empty slot's
null「are the same structural mismatch wearing different values and take the same arm」. That is FALSE and it is the defect the clause-② review caught. Measured against the lockfile-pinned@radix-ui/react-slot@1.3.3,slot.tsx:45readsif (children || children === 0) throw …; return children⇒ a null child never threw, so the empty arm was never broken. Treating the two alike made a document with no trigger paint one empty focusable button on the five keys whosetriggeris.optional(). The seam now carries BOTH rules: render nothing when the slot is empty, and ask Radix's structural question otherwise.⛔
overlay/context-menu.tsxis deliberately untouched. It callsrenderChildren(schema.trigger …)exactly like the others, and itsasChildchild is a realdiv, so the rendered slot goes INSIDE it andChildren.onlynever sees the string. I checked it by reading the trigger element, and the pin below measures it green rather than taking my word for it. The discriminator is "is theasChildchild a single React element", ⛔ not "does this file call renderChildren(schema.trigger)".The census, re-run — and one published control that does not reproduce
The card orders the taker not to inherit its 8. Re-run on this branch's base (
57c07f002), corpus enumerated withgit ls-tree -r --name-only REVand read withgit show REV:PATH, never off a working tree — 195.tsxfiles underpackages/components/src/renderers/.asChildover the authored slot, at base2923cea165collapsible.tsx.ts/.tsxunderpackages+apps+examples)renderers/overlay/trigger:keys inoverlay.zod.tswith the identical uniontrigger:key in that filedisclosure.zod.ts5719013007states the conditional spelling reads 1 at headc90c68d63and 0 at base, that hit beingcollapsible.tsx. Taken literally —asChild={on the trigger line, with the next non-blank line within three matchingrenderChildren(\s*schema\.trigger— it reads 0 at today's base AND 0 atc90c68d63itself, which I fetched and read. PR objectui#9708 hoists the slot intoconst trigger = renderChildren(schema.trigger);three lines above the trigger, so a FORWARD adjacency regex structurally cannot see it. The published instrument was already blind to the repair at the revision the reading was stamped on.Repaired instrument: resolve a single-identifier slot back to its
const NAME = renderChildren(schema.trigger…)binding, and apply that to BOTH arms — because an unconditionalasChildover a hoisted const would be a defective site the published probe silently misses. Under it: target 8 / control 1 (collapsible.tsx) at base, and target 9 / control 0 at2923cea165. ⇒ the probe moves when the repair moves, which is what that control was for.⭐ The seat's refuted-count-kept-beside-the-right-one is the card's most copyable property, so this is recorded the same way: the published control's number is wrong, the target's is right, and the difference is an instrument that could not see a hoisted local.
Does any of them want a bare string at all?
Yes — all nine, and this is why nothing moves in
packages/types. Both published faces already say so:SchemaNodenamesstringexplicitly (packages/types/src/base.ts) and the zod mirror types every one of the nine keys against that same union. Narrowing them would retract a capability both faces shipped, and it is the direction objectui#7105 already ruled against for node slots — they RELAX the renderer rather than narrow the declaration, quoted inrenderChildren's own docblock. A trigger whose label is a string is the most natural thing an author writes. So this is the implementation catching up to a declaration that already said yes, ⛔ not a lenient fallback around off-spec metadata (AGENTS.md #0.1): the accept set is unchanged in both directions.The pin, and the ninth renderer
packages/components/src/__tests__/overlay-trigger-bare-string-9710.test.tsxenumerates the REGISTRY for blocks declaring atriggerslot, ⛔ never a list of files — a pin naming these eight is green forever on the ninth. It writes down no count; the population is whatever the registry holds.Controls:⚠️ Named residue, ⛔ not claimed away: it cannot see an over-reach whose wrapper is neither BUTTON, A nor SPAN — no Radix trigger draws a ⚠️ Its coverage is the primitive kind it registers on: an edit that blinds the detector to
ENUMERATION_CONNECTED(the population reachesui:collapsible, which lives inrenderers/disclosure/, andui:context-menu, which was never defective — so it is not a restatement of this diff);ASCHILD_STILL_ON(an element trigger is merged rather than nested — ⭐ repaired and now measured 8-of-8. It was briefly UNSOUND: its authored-element finder took the FIRST button whose text matched the label, so when a primitive WRAPS the authored button the finder searched above the wrapper and saw nothing — measured 1 of 8 under the exact over-reach it guards. The finder now takes the INNERMOST match (textContent equals the label AND it contains no button of its own), and under the same ablation it names all eight.divtoday — and it assumes the label is unique in the subtree, which holds because every overlay's content is closed at first paint) ·OVER_REACH_IS_VISIBLE(a STANDING control added in the same round: a deliberately over-reaching block is registered on a BUTTON-drawing primitive and the detector is observed catching it on every run, so the detector's own failure is witnessed in CI rather than in one ablation transcript.AorSPANwrappers passes CI while hover-card over-reach goes unnamed — a second registered over-reacher on an anchor-drawing primitive would close that);REDDENS_FOR_A_NINTH(a deliberately defective block is registered inside the test, and THE assertion — the errored-out list is empty — is observed holding exactly that block's name and nothing else).Reverse verification, from the committed state. The eight files were reverted to their base blobs, verified on disk by
git hash-objectagainstgit rev-parse BASE:PATHper file, with atraprestoring fromHEAD:All eight named in one run, and ⛔ neither
ui:collapsiblenorui:context-menu— the discriminator, measured rather than asserted. Restored and proved restored:git diff HEADandgit diff --cachedboth empty, and every file inrenderers/overlay/byte-identical to itsHEADblob.Gates
Exit codes redirected to files and captured before reading, never through a pipe.
pnpm exec vitest run packages/components/(281 files, 2742 tests)pnpm --filter '@object-ui/components^...' buildpnpm --filter @object-ui/components type-check(tsc --noEmit+tsconfig.test.json)pnpm --filter @object-ui/components lint(eslint .)node scripts/check-changeset-presence.mjspnpm check:control-bytes·check:new-line-citationspnpm check:component-surface-parity·check:registry-bare-names·check:element-data-source-declarationpnpm check:unreferenced-sources·check:test-path-roots·check:vi-mock-specifierspnpm check:doc-types·check:prompt-keys·check:changeset-claims·check:pending-changeset-literalspnpm check:sdui-registration-pins·check:readme-exports·check:dist-completeness·check:published-dist·check:side-effects-arrayturbo run buildpnpm check(aggregate schema check, 632 files)node scripts/check-governed-queue-guard.mjs --test PATHScheck:sdui-registration-pinsandcheck:readme-exportsfirst exited 2 and 1 with their own text saying the run had measured nothing ("No console build to weigh", "the population COLLAPSED"). Those are PREREQUISITE NOT MET, ⛔ not verdicts; both are green above only because the workspace was built first and they were re-run.The repo-wide
pnpm lint(turbo run lint) and the rest of the gate farm are CI's run, ⛔ not narrowed here.Notes
packages/components/src/index.tsis untouched, so no public API is added. The whole-tree sweep above is why — noplugin-*package carries this shape today. The pin's docblock records the matching limit: a block registering atriggerslot from another package is not in this file's population.overlay/menubar.tsxdeclares notriggerkey and carries noasChildtrigger; it is not part of the class.session_018HrVaotisyhgmot9o2MLRq.Generated by Claude Code
⛔ Seat correction to this body
Edited by the
domain:ui#2execution seat,session_018HrVaotisyhgmot9o2MLRq, ⛔ not by the dev. A dev writes a PR body once at create and never PATCHes it, so a body that goes false after a patch round is the seat's to repair — and this lane's own platform note is explicit that the double-footer condition is ⛔ not a reason to leave a false sentence standing in a body.Two statements were false against the current head and are corrected above: the seam's NAME (
asChildSlotPropsbecamerenderTriggerSlot, and its shape changed from a props spread to a render function, because an emptiness guard cannot live in a helper that only returns props), and the empty-slot MECHANISM (retracted in full). TheASCHILD_STILL_ONdescription was also stale and is now annotated with the defect the re-review measured. Contract-review records for this PR:5725825584(FAIL, head77849a9) and5726449675(FAIL, head5f775a19).⭐ Update — contract review PASSED at head
434d410c(record5726725216). The two earlier FAILs are5725825584(head77849a9, the empty-slot regression) and5726449675(head5f775a19, the blinded control). Both defects are closed and independently re-derived by the reviewer: the omitted-trigger arm reddens on exactly the five zod-optional keys, and the over-reach control reddens on all eight. Seat edit bysession_018HrVaotisyhgmot9o2MLRq; a dev writes its body once at create and never PATCHes it.Generated by Claude Code