Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .changeset/6124-view-handler-keys-are-event-names.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
---

The three view handler keys are declared as EVENT NAMES, not callbacks.

`ViewSwitcherSchema.onViewChange`, `FilterUISchema.onChange` and
`SortUISchema.onChange` were described as "change callback" on both the zod
mirror and the TS interface. They are not callbacks: the string an author
writes is the NAME of a `CustomEvent` the renderer dispatches on `window` —
`new CustomEvent(schema.onViewChange, { detail: { view } })` and its two
siblings.

**What an author feels.** Nothing they write breaks — the type is still
`string`, so no accept set moves and no existing document changes verdict.
What changes is the two places this contract is published — the zod mirror's
`describe()` text and the TS JSDoc — which now tell them what the string is
FOR, and what to listen for:

```json
{ "type": "sort-ui", "fields": [{ "field": "name" }], "onChange": "myapp:sort-changed" }
```

```js
window.addEventListener('myapp:sort-changed', (e) => e.detail.sort);
```

Previously "Sort change callback" invited the two readings the runtime does not
support — a function (unwritable in JSON) or a handler expression (dropped at
runtime) — with no hint that the working form is an event name.

The correction also protects the capability. A handler-key census that buckets
by declared TYPE cannot tell an event name from the unsupported
handler-expression dialect, and on that reading these three had been swept in
for retirement, which would have deleted working behaviour. A new pin
(`plugin-view/src/__tests__/handlerEventNameLiveness.6124.test.tsx`) now holds
both halves — that each key is DECLARED on the authorable surface, and that the
authored string reaches `new CustomEvent(...)`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
/**
* 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.
*/

/**
* objectui#6124 — the three `on*` mirrors in `views.zod.ts` that a handler-key
* census reads as dead callbacks and are in fact LIVE, JSON-authorable
* capabilities.
*
* ## What was measured, and why a pin exists at all
*
* #6182's census bucketed handler keys BY ZOD TYPE, which cannot tell a
* "handler expression" (a dialect this repo does not support — measurably
* dropped at runtime, #4453) from an EVENT NAME (a string the renderer
* dispatches). Three rows are the second kind, and the scope extension that
* swept them in for retirement would have DELETED WORKING BEHAVIOUR:
*
* - `ViewSwitcherSchema.onViewChange` — `ViewSwitcher.tsx:249-255`
* - `FilterUISchema.onChange` — `FilterUI.tsx:99-105`
* - `SortUISchema.onChange` — `SortUI.tsx:93-99`
*
* each `window.dispatchEvent(new CustomEvent(schema.<key>, { detail }))`, i.e.
* the AUTHORED STRING IS THE EVENT NAME. Note the dual channel that produced
* the mislabel: `onViewChange?.(next)` right above it is the REACT PROP (a
* function a host passes), while `schema.onViewChange` is the authored string.
* Same name, two channels — which is exactly what a type-shaped census cannot
* see, and why the `.describe()` text on those three rows now says "event name"
* rather than "callback".
*
* ## Why BOTH halves, and why the declared half is not a `safeParse`
*
* A retirement of one of these keys has two independent failure surfaces, so
* the pin has two halves:
*
* 1. DECLARED — the key is in the mirror's `shape`. ⚠️ This deliberately does
* NOT assert via `safeParse`: `BaseSchema` is `.passthrough()`, so a
* retired key still PARSES GREEN and the parsed output still CARRIES the
* value (measured on #6124). A `safeParse`-based pin would therefore stay
* green through the very deletion it exists to catch — absence is not
* refusal here, so the pin has to read the DECLARATION.
* 2. LIVE — the authored string reaches `new CustomEvent(...)` on `window`.
*
* Either half alone is passable by a change that breaks the capability: half 1
* alone allows the renderer's dispatch to be deleted; half 2 alone allows the
* key to leave the authorable surface while the runtime keeps working for React
* hosts only.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent } from '@testing-library/react';
import {
ViewSwitcherSchema as ViewSwitcherMirror,
FilterUISchema as FilterUIMirror,
SortUISchema as SortUIMirror,
} from '@object-ui/types/zod';
import type {
ViewSwitcherSchema,
FilterUISchema,
SortUISchema,
} from '@object-ui/types';
import { ViewSwitcher } from '../ViewSwitcher';
import { FilterUI } from '../FilterUI';
import { SortUI } from '../SortUI';

vi.mock('@object-ui/react', async (importOriginal) => {
const React = await import('react');
return {
...(await importOriginal<Record<string, unknown>>()),
SchemaRenderer: ({ schema }: { schema?: { type?: string } }) => (
<div data-schema-type={schema?.type} />
),
SchemaRendererContext: React.createContext(null),
subscribeDataChanges: () => () => {},
notifyDataChanged: () => {},
};
});

afterEach(() => cleanup());

/** Record every CustomEvent of `name` dispatched on `window` while `fn` runs. */
function captureWindowEvents<D>(name: string, fn: () => void): D[] {
const seen: D[] = [];
const listener = (e: Event) => seen.push((e as CustomEvent<D>).detail);
window.addEventListener(name, listener);
try { fn(); } finally { window.removeEventListener(name, listener); }
return seen;
}

describe('#6124 half 1 — the three event-name keys are DECLARED on the authorable surface', () => {
// Reading `.shape`, not `.safeParse`: under `BaseSchema.passthrough()` a
// retired key still parses green, so only the declaration can report a
// deletion. Retiring any of these three turns exactly this half red.
it('ViewSwitcherSchema declares onViewChange', () => {
expect(Object.keys(ViewSwitcherMirror.shape)).toContain('onViewChange');
});
it('FilterUISchema declares onChange', () => {
expect(Object.keys(FilterUIMirror.shape)).toContain('onChange');
});
it('SortUISchema declares onChange', () => {
expect(Object.keys(SortUIMirror.shape)).toContain('onChange');
});

it('each is a STRING mirror — an authored event name parses, a function does not', () => {
const base = { type: 'sort-ui' as const, fields: [{ field: 'name' }] };
expect(SortUIMirror.safeParse({ ...base, onChange: 'sort:changed' }).success).toBe(true);
// control: the instrument can say no — the string arm is a real constraint
expect(SortUIMirror.safeParse({ ...base, onChange: () => {} }).success).toBe(false);
});

it('the describe() text names the EVENT-NAME channel, not "callback"', () => {
// The mislabel is the measured root cause of the wrong bucketing, so the
// corrected wording is pinned rather than left to survive by luck.
for (const d of [
ViewSwitcherMirror.shape.onViewChange.description,
FilterUIMirror.shape.onChange.description,
SortUIMirror.shape.onChange.description,
]) {
// Names the real channel …
expect(d).toMatch(/event name/i);
// … and carries the DISCLAIMER, because "callback" in the old wording is
// what a type-shaped census read to mean "dead handler". The wording has
// to refuse that reading explicitly, not merely omit the word.
expect(d).toMatch(/not a callback or a handler expression/i);
}
});
});

describe('#6124 half 2 — the authored string is LIVE: it is the CustomEvent name', () => {
it('ViewSwitcher dispatches the authored name on view change', () => {
const schema: ViewSwitcherSchema = {
type: 'view-switcher', variant: 'buttons',
views: [{ type: 'list' }, { type: 'grid' }],
onViewChange: 'zz6124:view',
};
render(<ViewSwitcher schema={schema} />);
const details = captureWindowEvents<{ view: string }>('zz6124:view', () => {
fireEvent.click(screen.getByRole('button', { name: /grid/i }));
});
expect(details).toHaveLength(1);
expect(details[0].view).toBe('grid');
});

it('FilterUI dispatches the authored name on filter change', () => {
const schema: FilterUISchema = {
type: 'filter-ui', layout: 'inline',
filters: [{ field: 'qty', label: 'Qty', type: 'number' }],
onChange: 'zz6124:filter',
};
const { container } = render(<FilterUI schema={schema} />);
const input = container.querySelector('input[type="number"]') as HTMLInputElement;
expect(input).toBeTruthy();
const details = captureWindowEvents<{ values: Record<string, unknown> }>('zz6124:filter', () => {
fireEvent.change(input, { target: { value: '7' } });
});
expect(details).toHaveLength(1);
expect(details[0].values.qty).toBe(7);
});

it('SortUI dispatches the authored name on sort change', () => {
const schema: SortUISchema = {
type: 'sort-ui', variant: 'buttons',
fields: [{ field: 'name', label: 'Name' }],
onChange: 'zz6124:sort',
};
render(<SortUI schema={schema} />);
const details = captureWindowEvents<{ sort: Array<{ field: string }> }>('zz6124:sort', () => {
fireEvent.click(screen.getByRole('button', { name: /name/i }));
});
expect(details).toHaveLength(1);
expect(details[0].sort[0].field).toBe('name');
});
});
15 changes: 12 additions & 3 deletions packages/types/src/views.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -756,7 +756,10 @@ export interface ViewSwitcherSchema extends BaseSchema {
*/
position?: 'top' | 'bottom' | 'left' | 'right';
/**
* View change callback
* Event name dispatched on `window` when the view changes
* (`detail: { view }`) — an event NAME, not a callback or a handler
* expression. Read at `plugin-view/src/ViewSwitcher.tsx` as
* `new CustomEvent(schema.onViewChange, …)` (objectui#6124).
*/
onViewChange?: string;
/**
Expand DownExpand Up@@ -820,7 +823,10 @@ export interface FilterUISchema extends BaseSchema {
*/
values?: Record<string, any>;
/**
* Filter change callback
* Event name dispatched on `window` when the filters change
* (`detail: { values }`) — an event NAME, not a callback or a handler
* expression. Read at `plugin-view/src/FilterUI.tsx` as
* `new CustomEvent(schema.onChange, …)` (objectui#6124).
*/
onChange?: string;
/**
Expand DownExpand Up@@ -870,7 +876,10 @@ export interface SortUISchema extends BaseSchema {
direction: 'asc' | 'desc';
}>;
/**
* Sort change callback
* Event name dispatched on `window` when the sort changes
* (`detail: { sort }`) — an event NAME, not a callback or a handler
* expression. Read at `plugin-view/src/SortUI.tsx` as
* `new CustomEvent(schema.onChange, …)` (objectui#6124).
*/
onChange?: string;
/**
Expand Down
9 changes: 6 additions & 3 deletions packages/types/src/zod/views.zod.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -136,7 +136,8 @@ export const ViewSwitcherSchema = BaseSchema.extend({
activeView: ViewTypeSchema.optional().describe('Current active view'),
variant: z.enum(['tabs', 'buttons', 'dropdown']).optional().describe('Switcher variant'),
position: z.enum(['top', 'bottom', 'left', 'right']).optional().describe('Switcher position'),
onViewChange: z.string().optional().describe('View change callback'),
onViewChange: z.string().optional()
.describe('Event name dispatched on window when the view changes (detail: { view }) — an event NAME, not a callback or a handler expression'),
persistPreference: z.boolean().optional().describe('Persist view preference'),
storageKey: z.string().optional().describe('Storage key for persisting view'),
allowCreateView: z.boolean().optional().describe('Show "+" button to add/create a new view'),
Expand All@@ -160,7 +161,8 @@ export const FilterUISchema = BaseSchema.extend({
placeholder: z.string().optional().describe('Placeholder'),
})).describe('Available filters'),
values: z.record(z.string(), z.any()).optional().describe('Current filter values'),
onChange: z.string().optional().describe('Filter change callback'),
onChange: z.string().optional()
.describe('Event name dispatched on window when the filters change (detail: { values }) — an event NAME, not a callback or a handler expression'),
showClear: z.boolean().optional().describe('Show clear button'),
showApply: z.boolean().optional().describe('Show apply button'),
layout: z.enum(['inline', 'popover', 'drawer']).optional().describe('Filter layout'),
Expand All@@ -179,7 +181,8 @@ export const SortUISchema = BaseSchema.extend({
field: z.string().describe('Field to sort by'),
direction: z.enum(['asc', 'desc']).describe('Sort direction'),
})).optional().describe('Current sort configuration'),
onChange: z.string().optional().describe('Sort change callback'),
onChange: z.string().optional()
.describe('Event name dispatched on window when the sort changes (detail: { sort }) — an event NAME, not a callback or a handler expression'),
multiple: z.boolean().optional().describe('Allow multiple sort fields'),
variant: z.enum(['dropdown', 'buttons']).optional().describe('UI variant'),
});
Expand Down
Loading