fix(plugin-calendar): authored events never reaches CalendarView's events prop (#4433) - #4455
Merged
Merged
Conversation
…events prop (#4433) The calendar-view renderer computed a CalendarEvent[] from schema.data, passed it as `events={…}`, then spread the remaining props AFTER it. SchemaRenderer forwards a node's `events` key as a plain prop, so a node authoring `events` — ordinary SDUI action metadata, legal on any node — landed its `{ onClick: [...] }` object on the array prop and CalendarView threw `events is not iterable`. The authored key is destructured out before the spread (the #4357 / PR #4428 deny-list precedent), so the computed array always wins. This also closes the quiet half: an authored `events` ARRAY never threw, it silently replaced the calendar's contents with itself. Both halves of the #4425 sweep gate come out together: the CALENDAR_VIEW_OMITS withheld canary is gone (calendar-view is swept with the full canary set again) and the dedicated crash pin flips to asserting the calendar renders.
…endar-events-crash
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…endar-events-crash
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. Slot NOT refilled per maintainer's wind-down instruction. Generated by Claude Code Generated by Claude Code |
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
2 participants
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.
Fixes #4433
The
calendar-viewrenderer computed aCalendarEvent[]fromschema.data, passed it asevents={…}, then spread the remaining props after it.SchemaRendererforwards a node'seventskey as a plain prop — it is not on that renderer's strip list — so a node authoringevents, the ordinary SDUI action metadata of AGENTS.md section 4 that is legal on any node, landed its{ onClick: [...] }object on the array prop andCalendarViewthrewevents is not iterable.The authored key is destructured out before the spread, per the #4357 / PR #4428 deny-list precedent. The computed array always wins.
The semantics measurement first (ruling condition 1a)
Did authored
eventsever do anything on this node type? No — measured, and it never could.eventskey. Grepped acrosspackages/react/src,packages/core/src/{registry,actions}andpackages/components/src/renderers:SchemaRendererforwardseventsas a prop and no consumer reads it. Re-measured after this branch mergedorigin/main(feat(lint): require an explicit type on plain button elements; fix the 114-site population (#4045) #4450 touchesSchemaRenderer.tsx): the strip-list destructure atSchemaRenderer.tsxstill names onlydataSource,_hidden,_disabledandresponsiveStyles, and the stringeventsdoes not occur anywhere in that file. The one.eventshit in the renderer/core layer isobject-validation-engine.ts's validation-rule field, which is unrelated to a node key.properties.actionexecuted throughActionRunner— the wiringelement:buttonuses.packages/typesdoes declareEventableSchema.events, but asUIEventHandler[]— a different shape from the{ onClick: [...] }map, and nothing runtime reads either one.calendar-viewspecifically the key reached exactly one place:CalendarView'seventsprop, whose declared type isCalendarEvent[].CalendarViewtakes a fixed prop list and spreads nothing onto the DOM, so the key had no other destination.So this strip disables nothing. Pre-existing state, not a regression: the key was a landmine, never a feature. The component's real action channel is
onAction, and the last test case pins it still firing whileeventsis authored.The collision grading (ruling condition 1b)
Every prop the renderer declares before the spread, plus the neighbouring surface, measured node-by-node through the real SDUI host rather than reasoned about:
eventsprops: { events })events is not iterable. ARRAY → no throw, the authored array silently replaced the computed calendarclassNameSchemaRendereralso assignsclassNameafter the node'spropscontainer. Authoredprops.classNamenever reached the region; node-levelclassNamecomposes correctlyonEventClickonEventClick is not a functionas an uncaught window error — React does not route event-handler errors toSchemaErrorBoundaryonEventClickgraded crash-capable yet is not fixed here. Unlikeeventsit has a producer that works today: a React host renderingSchemaRenderer schema={…} onEventClick={fn}reaches the same channel through the renderer's trailing...props, and that is the component's genuine escape hatch. The key name cannot separate the working producer from the broken one — only the value's type can — so closing it is a choice between breaking a live path, adding value-type discrimination (AGENTS.md #0.1's lenient-coercion shape), or bounding the spread by declaration (#4425 phase 2). That is a contract decision this card's ruling did not make, so it is filed rather than guessed — but if the ruling intendsonEventClickclosed inside this card, this PR should be re-titledPart of #4433and #4453 folded in instead.The remaining canaries (
bind,props,ariaLabel,ariaDescribedBy,dataSource,schema, the authored open tail) reachCalendarViewand are dropped there — it names its props and never spreads onto an element. That is whyplugin-calendarmeasured zero leaks in the phase-1 sweep, and it stays true witheventsback in the set.Red-first evidence — re-derived on the final tree
The fix is already committed, so the red is re-established by mutation: revert only
packages/plugin-calendar/src/calendar-view-renderer.tsxto its pre-fix blob (git checkout c10c16c12^ -- path), keep the new tests and the gate flip, run, then restore withgit checkout HEAD -- pathand confirmgit status --porcelainis empty. Nevergit stash— that stack is shared across worktrees.Verbatim, on the post-merge tree:
with the card's own reported DOM, captured by the sweep's readiness failure:
Reverse verification — direction predicted first
expected null not to be nulleventsARRAY" caseComputed StandupTestingLibraryElementError: Unable to find role="button" and name "Computed Standup"expected null not to be nullplugin-calendar:calendar-viewreadiness selector `[role="region"][aria-label="Calendar"]` never matchedplugin-calendar:object-calendar,view:calendarObjectCalendarRenderer, the mutation cannot reach themSix red, each in its predicted direction. Correction to an earlier revision of this body: it reported the itemized six but summarised them as "5 tests failed, 7 files" — a miscount, with the file count carried over from a run that still included a since-deleted scratch probe. The re-derivation above is the measured truth:
2 failed (2)files,6 failed | 37 passed (43)tests. The per-case directions, which are the substance, were right both times; only the arithmetic under them was wrong, and it is corrected rather than quietly dropped.Post-fix the same four new cases are green, and the second one is the one that matters for "protected into emptiness": with
dataand authoredeventspresent,Computed StandupandComputed Reviewboth render. The third pins the quiet half — pre-fix the authored array renderedAuthored Overwriteand the computed events were gone; post-fix the computed events are back and the authored title is absent.The gate's two halves, together
CALENDAR_VIEW_OMITSand the target'somitCanariesentry are removed:plugin-calendar:calendar-viewis swept with the full canary set,eventsincluded, and passes — so no leak was hiding behind the crash.eventsthrows instead of rendering" to "authoringeventsrenders the calendar", assertingrole="region" aria-label="Calendar"with no error boundary.eventsalone and names the key.omitCanariesfacility itself stays, with no current user. It carries the discipline the docblock spells out — a withheld canary is a recorded defect with its own pin, never a quiet exemption — which finding(plugin-dashboard): the #3291toDomPropswhitelist stops atpackages/fields— SDUI widgets elsewhere still close their DOM leak by hand, if at all #4425 phase 2 will need the next time a target cannot take the full set. Flagged here as a deliberate non-removal, not an oversight.plugin-calendarwas clean "only because its one spreading target is swept with a canary WITHHELD". That sentence is now false, so it was corrected to the measured reason.Depends on #4441 (the gate file is its surface), which is in
mainat716c5ae91.Verification — all re-run on the final tree, after merging
origin/mainRepo-root vitest with paths relative to the repo root, per AGENTS.md 怎么跑测试.
origin/mainmoved again mid-flight (#4450, #4449, #4447, #4448 — including a change toSchemaRenderer.tsxand a newbutton-has-typelint rule); it was merged in with no conflicts (the four touched paths are disjoint from all of it), and every command above was run after that merge.Changeset:
patchon@object-ui/plugin-calendar— measured, not assumed.calendar-view-renderer.tsxhas no exports (it is a registration side-effect module, pulled in byimport './calendar-view-renderer'), and building the package pre-fix and post-fix emits a byte-identicaldist/index.d.ts(433 bytes both ways,diffempty). No exported type moves, so the ruling's grade-up condition does not fire, and nevermajor. Theapp-shellhalf is test-only and correctly carries no changeset of its own.Filed from the measurement
currentDateas the ISO string its own registry input documents crashes calendar-view #4452 —currentDateauthored as the ISO string its own registry input documents crashes the component (selectedDate.toLocaleDateString is not a function). Same renderer, opposite shape: a declared input that is never converted, not a collision. Measured again after this fix — unchanged, so the two are independent.onEventClickreaches CalendarView through the spread and throws an UNCAUGHT error on click — needs a contract decision, not a strip #4453 — theonEventClickexposure above, with the four options and why it needs a ruling.calendar-view'sallowCreateinput is declared but inert — the handler it would drive is built and never passed #4454 —finding:allowCreateis declared and inert, andhandleAddClickis built and never passed, so the "New event" button cannot exist on the SDUI path.Generated by Claude Code