From c10c16c129e0534562e41540a578053f418b2f6f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 08:53:02 +0000 Subject: [PATCH] fix(plugin-calendar): authored `events` never reaches CalendarView's events prop (#4433) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .changeset/calm-schools-tease.md | 22 ++ .../__tests__/widget-dom-leak-sweep.test.tsx | 85 ++++---- ...dar-view-renderer.eventsCollision.test.tsx | 188 ++++++++++++++++++ .../src/calendar-view-renderer.tsx | 33 ++- 4 files changed, 287 insertions(+), 41 deletions(-) create mode 100644 .changeset/calm-schools-tease.md create mode 100644 packages/plugin-calendar/src/calendar-view-renderer.eventsCollision.test.tsx diff --git a/.changeset/calm-schools-tease.md b/.changeset/calm-schools-tease.md new file mode 100644 index 0000000000..99ee16d88f --- /dev/null +++ b/.changeset/calm-schools-tease.md @@ -0,0 +1,22 @@ +--- +'@object-ui/plugin-calendar': patch +--- + +fix(plugin-calendar): authoring `events` on a `calendar-view` node no longer takes the calendar down + +`calendar-view`'s 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` — the +ordinary SDUI action metadata, legal on any node — landed its `{ onClick: [...] }` +object on the `events` array prop: `CalendarView` iterated it and threw +`events is not iterable`, and a spec-legal node rendered an error card instead of its +calendar. + +The authored key is now destructured out before the spread, so the computed array +always wins. This also closes the quiet half of the same collision: an authored +`events` **array** never threw — it silently replaced the calendar's contents with +itself. + +No capability is removed. Nothing in the renderer layer consumes a node's `events` +key (the action path is `properties.action` through `ActionRunner`), and the +component's own `onAction` channel is unaffected. diff --git a/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx b/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx index 80bbba9dc0..a9fc7a1f4f 100644 --- a/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx +++ b/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx @@ -35,10 +35,15 @@ * * 5 of 23 targets leak. The two clean packages are clean for opposite reasons * worth keeping straight: `plugin-charts` never spreads the node onto its - * container at all, while `plugin-calendar` is clean only because its one - * spreading target is swept with a canary WITHHELD — authoring `events` crashes - * it outright (objectui#4433), which is a worse failure than the leak this gate - * was looking for, and is pinned by its own case below. + * container at all, while `plugin-calendar`'s components take a declared prop + * list and drop what they do not name, so the node's keys never reach an + * element. + * + * `calendar-view` was originally swept with the `events` canary WITHHELD, because + * authoring it crashed the component outright (objectui#4433) — a worse failure + * than the leak this gate was looking for, and one that would have read as a + * clean pass. That is fixed, so the omission is gone and the target is swept + * with the full canary set; section 5 below carries what is left of it. * * ## The divergence this repo is living with, recorded deliberately * @@ -396,23 +401,6 @@ const CALENDAR_OBJECT_EXTRAS = { titleField: 'name', }; -/** - * `plugin-calendar:calendar-view` is rendered WITHOUT the `events` canary. - * - * Not a convenience: authoring `events` — the ordinary SDUI action metadata of - * AGENTS.md section 4, legal on any node — CRASHES this component outright. Its - * renderer computes a `CalendarEvent[]` from `schema.data`, passes it as - * `events={…}`, and then spreads `{...props}` AFTER it, so the SDUI `events` - * object overwrites the array and `CalendarView` throws `events is not - * iterable`. `SchemaErrorBoundary` then renders its own tidy alert, which has no - * leaked attributes — so including the canary here would replace a leak - * measurement with a crash measurement AND read as clean (trap 3). - * - * The crash is pinned instead by its own case below, so it cannot regress - * unnoticed while the fix is pending. - */ -const CALENDAR_VIEW_OMITS = ['events'] as const; - const TARGETS: Readonly> = { 'plugin-charts': [ { type: 'plugin-charts:bar-chart', schemaExtras: { data: CHART_DATA }, ready: '.recharts-responsive-container' }, @@ -426,7 +414,7 @@ const TARGETS: Readonly> = { { type: 'view:chart', schemaExtras: OBJECT_CHART_EXTRAS, ready: '[data-slot="chart"]' }, ], 'plugin-calendar': [ - { type: 'plugin-calendar:calendar-view', ready: '[role="region"][aria-label="Calendar"]', omitCanaries: CALENDAR_VIEW_OMITS }, + { type: 'plugin-calendar:calendar-view', ready: '[role="region"][aria-label="Calendar"]' }, { type: 'plugin-calendar:object-calendar', schemaExtras: CALENDAR_OBJECT_EXTRAS, ready: '[role="region"][aria-label="Calendar"]' }, { type: 'view:calendar', schemaExtras: CALENDAR_OBJECT_EXTRAS, ready: '[role="region"][aria-label="Calendar"]' }, ], @@ -936,21 +924,37 @@ describe.each(Object.keys(TARGETS))( ); /* ════════════════════════════════════════════════════════════════════════════ - * 5. The withheld canary is a recorded defect, not an exemption + * 5. The canary that was withheld, and is not any more * ══════════════════════════════════════════════════════════════════════════ */ /** - * `plugin-calendar:calendar-view` is swept without the `events` canary - * ({@link CALENDAR_VIEW_OMITS}). This case is the price of that omission: it - * pins the crash that forced it, so the defect cannot regress unnoticed and the - * omission cannot quietly outlive it. - * - * When the crash is fixed, this case goes red and BOTH halves are removed in the - * same change — the `omitCanaries` entry and this pin — putting `events` back - * into the sweep for that target. + * `plugin-calendar:calendar-view` used to be swept WITHOUT the `events` canary, + * because authoring `events` — the ordinary SDUI action metadata of AGENTS.md + * section 4, legal on any node — crashed the component outright: its renderer + * computed a `CalendarEvent[]`, passed it as `events={…}`, then spread + * `{...props}` AFTER it, so the authored object overwrote the array and + * `CalendarView` threw `events is not iterable`. A crashing render produces + * tidy, attribute-clean error-boundary DOM, so the canary had to be withheld or + * the target would have read as a clean pass (trap 3). + * + * objectui#4433 fixed that — the renderer destructures the authored key out + * before the spread — so BOTH halves came out in the same change: the + * `omitCanaries` entry is gone (this target is swept with the full canary set + * above, `events` included) and this case flipped from pinning the crash to + * pinning the render. + * + * It is kept rather than deleted because it is the DIAGNOSIS the sweep case + * cannot be: the sweep plants the whole canary set at once, so a regression + * there says only "calendar-view broke". This one plants `events` alone, and + * names the key. + * + * The `omitCanaries` facility itself stays. Nothing withholds a canary today, + * but it carries the discipline — a withheld canary is a recorded defect with + * its own pin, never a quiet exemption — that objectui#4425 phase 2 will need + * the next time a target cannot take the full set. */ -describe('the canary withheld from calendar-view records a real crash (objectui#4425)', () => { - it('authoring `events` on a calendar-view node throws instead of rendering', async () => { +describe('the canary once withheld from calendar-view is swept again (objectui#4433)', () => { + it('authoring `events` on a calendar-view node renders the calendar', async () => { const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); try { render( @@ -966,13 +970,18 @@ describe('the canary withheld from calendar-view records a real crash (objectui# /> , ); + // The real component, not the error boundary `SchemaErrorBoundary` + // rendered here before the fix. await waitFor(() => { - expect(document.body.textContent ?? '').toContain(ERROR_BOUNDARY_MARKER); + expect( + document.body.querySelector('[role="region"][aria-label="Calendar"]'), + ).not.toBeNull(); }); - // The mechanism, not just the symptom: the SDUI `events` OBJECT lands on - // `CalendarView`'s `events` ARRAY prop because the renderer's `{...props}` - // is spread after it. - expect(document.body.textContent ?? '').toContain('events is not iterable'); + expect(document.body.textContent ?? '').not.toContain(ERROR_BOUNDARY_MARKER); + // The mechanism, not just the symptom: the authored `events` OBJECT no + // longer reaches `CalendarView`'s `events` ARRAY prop, so nothing tries to + // iterate it. + expect(document.body.textContent ?? '').not.toContain('events is not iterable'); } finally { errors.mockRestore(); } diff --git a/packages/plugin-calendar/src/calendar-view-renderer.eventsCollision.test.tsx b/packages/plugin-calendar/src/calendar-view-renderer.eventsCollision.test.tsx new file mode 100644 index 0000000000..c73cf924a8 --- /dev/null +++ b/packages/plugin-calendar/src/calendar-view-renderer.eventsCollision.test.tsx @@ -0,0 +1,188 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `plugin-calendar:calendar-view` — the authored `events` collision + * (objectui#4433, measured by the objectui#4425 phase-1 sweep). + * + * The renderer computes a `CalendarEvent[]` from `schema.data` and passes it as + * `events={…}`, then spreads the remaining props AFTER it. `SchemaRenderer` + * forwards a node's `events` key as a prop — it is not on the renderer's strip + * list — so a node authoring `events`, the ordinary SDUI action metadata of + * AGENTS.md section 4 that is legal on any node, landed its `{ onClick: [...] }` + * OBJECT on the `events` ARRAY prop. `CalendarView` iterates it and throws + * `events is not iterable`: a spec-legal node lost its calendar to an error + * card. + * + * Two shapes, one collision, and the second is the quiet one: + * + * - an authored `events` OBJECT crashed the component (the reported defect); + * - an authored `events` ARRAY did NOT crash — it is iterable, so it silently + * REPLACED the computed calendar with itself. Same overwrite, no error + * card, so nothing would have reported it. + * + * Both are pinned below, because a fix that only stopped the throw would leave + * the second half live. + * + * The authored key is destructured out before the spread (the objectui#4357 / + * PR #4428 deny-list precedent), so the computed array always wins. That is not + * a disabled feature: no code in the renderer layer consumes a node's `events` + * key — `SchemaRenderer` forwards it as a prop and nothing reads it, and the + * repo's action path is `properties.action` through `ActionRunner`. On this node + * type the key has never done anything but crash. The last case here pins the + * action channel this component DOES have, so the strip cannot be mistaken for + * removing one. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { SchemaRenderer } from '@object-ui/react'; +// Module scope: the registration side effect this file renders through +// (AGENTS.md 测试纪律 — never inside a hook). +import './index'; + +/** The text `SchemaErrorBoundary` renders when a widget throws. */ +const ERROR_BOUNDARY_MARKER = 'failed to render'; + +/** The SDUI action metadata of AGENTS.md section 4 — legal on any node. */ +const AUTHORED_EVENTS = { onClick: [{ action: 'navigate', params: { url: '/x' } }] }; + +/** Two records in the CURRENT month, so the default month view shows them. */ +function currentMonthRecords() { + const now = new Date(); + const day = (n: number) => new Date(now.getFullYear(), now.getMonth(), n, 10, 0, 0, 0).toISOString(); + return [ + { id: 'r1', title: 'Computed Standup', start: day(10) }, + { id: 'r2', title: 'Computed Review', start: day(12) }, + ]; +} + +function calendarRegion(): Element | null { + return document.body.querySelector('[role="region"][aria-label="Calendar"]'); +} + +describe('calendar-view: authored `events` never reaches CalendarView (objectui#4433)', () => { + it('renders the calendar for a node whose only authored key is `events`', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + // The card's own minimal repro. Before the fix this rendered + // `SchemaErrorBoundary` with `events is not iterable`. + await waitFor(() => expect(calendarRegion()).not.toBeNull()); + expect(document.body.textContent ?? '').not.toContain(ERROR_BOUNDARY_MARKER); + expect(document.body.textContent ?? '').not.toContain('events is not iterable'); + } finally { + errors.mockRestore(); + } + }); + + it('still displays the events computed from `schema.data` when `events` is authored', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + // Protecting the prop into emptiness would satisfy "does not crash" and + // still lose the calendar's contents, so the computed events are asserted + // present, not merely non-throwing. + await waitFor(() => expect(calendarRegion()).not.toBeNull()); + expect(await screen.findByRole('button', { name: 'Computed Standup' })).toBeTruthy(); + expect(screen.getByRole('button', { name: 'Computed Review' })).toBeTruthy(); + } finally { + errors.mockRestore(); + } + }); + + it('does not let an authored `events` ARRAY replace the computed events', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + await waitFor(() => expect(calendarRegion()).not.toBeNull()); + expect(await screen.findByRole('button', { name: 'Computed Standup' })).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Authored Overwrite' })).toBeNull(); + } finally { + errors.mockRestore(); + } + }); + + it('leaves the component\'s own action channel working while `events` is authored', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + const onAction = vi.fn(); + try { + render( + , + ); + + await waitFor(() => expect(calendarRegion()).not.toBeNull()); + fireEvent.click(await screen.findByRole('button', { name: 'Computed Standup' })); + + // `onAction` is this component's real action channel (the host's prop), + // and it is unaffected by stripping the authored key. + expect(onAction).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'event-click', + payload: expect.objectContaining({ id: 'r1', title: 'Computed Standup' }), + }), + ); + } finally { + errors.mockRestore(); + } + }); +}); diff --git a/packages/plugin-calendar/src/calendar-view-renderer.tsx b/packages/plugin-calendar/src/calendar-view-renderer.tsx index b23ddd734f..1311c3799a 100644 --- a/packages/plugin-calendar/src/calendar-view-renderer.tsx +++ b/packages/plugin-calendar/src/calendar-view-renderer.tsx @@ -12,8 +12,33 @@ import { CalendarView, type CalendarEvent } from './CalendarView'; import React from 'react'; // Calendar View Renderer - Airtable-style calendar for displaying records as events -ComponentRegistry.register('calendar-view', - ({ schema, className, onAction, ...props }: { schema: CalendarViewSchema; className?: string; onAction?: (action: any) => void; [key: string]: any }) => { +ComponentRegistry.register('calendar-view', + ({ + schema, + className, + onAction, + // The authored SDUI `events` key, destructured out so the `{...props}` + // spread below cannot overwrite the `CalendarEvent[]` computed from + // `schema.data` (objectui#4433; the deny-list precedent is objectui#4357 / + // PR #4428, where `SchemaRenderer`'s injected schema-shaped props are + // stripped at the component's own signature). + // + // `events` is the ordinary action metadata of AGENTS.md section 4, legal on + // ANY node, and `SchemaRenderer` forwards it as a plain prop — it is not on + // that renderer's strip list. Both channels land here: the node's own + // `events` key and a `props: { events }` container, since the renderer + // spreads the container's contents too. + // + // Nothing is disabled by dropping it. No code in the renderer layer reads a + // node's `events` key — `SchemaRenderer` forwards it and nothing consumes + // it; this repo's action path is `properties.action` through `ActionRunner`. + // On this node type the key has never done anything but overwrite the + // calendar: an OBJECT threw `events is not iterable` (the reported crash), + // and an ARRAY silently replaced the computed calendar with itself. This + // component's real action channel is `onAction` below, which is untouched. + events: _authoredEvents, + ...props + }: { schema: CalendarViewSchema; className?: string; onAction?: (action: any) => void; [key: string]: any }) => { // Transform schema data to CalendarEvent format const events = React.useMemo(() => { if (!schema.data || !Array.isArray(schema.data)) return []; @@ -58,8 +83,10 @@ ComponentRegistry.register('calendar-view', }; return ( -