From 7b438ab335c0416591edbacebc630556841cf16c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 08:22:52 +0000 Subject: [PATCH 1/4] fix(components): filter-builder renderer hands onChange the declared FilterGroup The renderer typed its onChange as (event: any) => void and called it with a form-style { target: { name, value } } event, while FilterBuilderSchema.onChange declares (filter: FilterGroup) => void. It now takes the schema's own type and passes the group. Claude-Session: https://claude.ai/code/session_01ADHkmNhEw4jVVQ4UTxrxMZ Co-authored-by: Claude --- ...r-builder-renderer-onchange-26382.test.tsx | 97 +++++++++++++++++++ .../src/renderers/complex/filter-builder.tsx | 47 ++++++--- 2 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx diff --git a/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx b/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx new file mode 100644 index 00000000000..3e0dc875d1f --- /dev/null +++ b/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx @@ -0,0 +1,97 @@ +/** + * 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. + */ + +/** + * The `filter-builder` renderer calls `onChange` with the shape + * `FilterBuilderSchema.onChange` declares: the `FilterGroup` itself (#26382). + * + * It used to call it with a form-style `{ target: { name, value } }` event + * while the schema declared `(filter: FilterGroup) => void`, and an `any` on + * the renderer's own `onChange` kept the compiler from seeing the gap. A + * handler written from the declaration read `.logic` / `.conditions` off the + * event and got `undefined`. + * + * The renderer is invoked DIRECTLY from the registry, as the sibling + * `inline-locale-label-read-sites.test.tsx` does, so the assertion reads what + * the renderer itself hands over and not what `SchemaRenderer` adds around it. + */ + +import * as React from 'react'; +import { describe, it, expect, expectTypeOf, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { I18nProvider } from '@object-ui/i18n'; +import { ComponentRegistry } from '@object-ui/core'; +import type { FilterBuilderSchema, FilterGroup } from '@object-ui/types'; +// Registers the renderers at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout`. See +// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). +import '../renderers'; + +const FIELDS = [ + { value: 'name', label: 'Name', type: 'text' }, + { value: 'age', label: 'Age', type: 'number' }, +]; + +const VALUE: FilterGroup = { + id: 'root', + logic: 'or', + conditions: [{ id: 'c1', field: 'name', operator: 'equals', value: 'Ada' }], +}; + +function renderDirect(props: Record) { + const C = ComponentRegistry.get('filter-builder') as React.ComponentType; + return render( + + + , + ); +} + +describe('filter-builder renderer hands onChange the declared FilterGroup (#26382)', () => { + it('calls onChange with the group, not with a { target } event', () => { + const onChange = vi.fn<(filter: FilterGroup) => void>(); + renderDirect({ + schema: { type: 'filter-builder', name: 'filters', fields: FIELDS, value: VALUE }, + onChange, + }); + + fireEvent.click(screen.getByRole('button', { name: /add filter/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + const [group] = onChange.mock.calls[0]; + expect(group).not.toHaveProperty('target'); + expect(group.logic).toBe('or'); + expect(group.conditions.map((c) => c.id)[0]).toBe('c1'); + expect(group.conditions).toHaveLength(2); + expect(group.conditions[1]).toMatchObject({ field: 'name', operator: 'equals' }); + }); + + it('calls a handler authored on the node itself the same way', () => { + const onChange = vi.fn<(filter: FilterGroup) => void>(); + renderDirect({ + schema: { type: 'filter-builder', name: 'filters', fields: FIELDS, value: VALUE }, + // `SchemaRenderer` spreads the node's own keys as props, so a typed node + // that authors `onChange` reaches the renderer through the same prop. + ...({ onChange } satisfies Pick), + }); + + fireEvent.click(screen.getByRole('button', { name: /add filter/i })); + + expect(onChange.mock.calls[0][0].conditions).toHaveLength(2); + }); + + it('pins the declared signature at the type level', () => { + expectTypeOf>().parameter(0).toEqualTypeOf(); + + // @ts-expect-error — the form-style event the renderer used to send is not the declared argument. + const eventHandler: NonNullable = (event: { target: { name?: string; value: unknown } }) => { + void event.target.value; + }; + expect(typeof eventHandler).toBe('function'); + }); +}); diff --git a/ui/packages/components/src/renderers/complex/filter-builder.tsx b/ui/packages/components/src/renderers/complex/filter-builder.tsx index 81896bd22b1..96162878fc0 100644 --- a/ui/packages/components/src/renderers/complex/filter-builder.tsx +++ b/ui/packages/components/src/renderers/complex/filter-builder.tsx @@ -13,28 +13,53 @@ import { useDisplayLocale } from '@object-ui/i18n'; // use of it): this repo has its OWN `resolveKeyedI18nLabel` over a DIFFERENT // vocabulary, and neither resolver accepts the other's shape. `schema.label` is // the spec's INLINE locale map — see `BaseSchema.label` (objectui#4580). -import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui'; -import { FilterBuilder } from '../../custom/filter-builder'; +import { resolveI18nLabel as resolveInlineI18nLabel, VIEW_FILTER_OPERATORS } from '@objectstack/spec/ui'; +import { FilterBuilder, type FilterBuilderRowGroup } from '../../custom/filter-builder'; import type { RendererHostProps } from '../../lib/renderer-host-props'; +const PROTOCOL_OPERATOR_IDS: ReadonlySet = new Set(VIEW_FILTER_OPERATORS); + +/** + * Is every row of the builder's group on the protocol's operator vocabulary, + * i.e. is it the `FilterGroup` that `FilterBuilderSchema.onChange` declares? + * + * The builder's own row also admits the opt-in `exists` / `notExists`, which a + * `FilterGroup` cannot carry. This node grants neither (`FilterBuilderSchema` + * declares no `extraOperators`), and its `value` is a `FilterGroup`, whose zod + * mirror refuses an operator outside the protocol. A group that fails here + * therefore reached the builder off contract. + */ +function isAuthoredFilterGroup(group: FilterBuilderRowGroup): group is FilterBuilderRowGroup & FilterGroup { + return group.conditions.every((condition) => PROTOCOL_OPERATOR_IDS.has(condition.operator)); +} + ComponentRegistry.register('filter-builder', // `value` is declared because it is read by name: a host that binds the node // (a form field) forwards the group it holds, the same authored `FilterGroup` // the node's own `value` is. - ({ schema, className, onChange, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: (event: any) => void; value?: FilterGroup; [key: string]: unknown } & RendererHostProps) => { + // + // `onChange` is typed by the schema's own declaration and is called with the + // group (#26382). It was typed `(event: any) => void` and called with a + // form-style `{ target: { name, value } }` event, so a handler written from + // `FilterBuilderSchema.onChange` read `.logic` / `.conditions` off the event + // and got `undefined`, and the `any` kept the compiler from saying so. + ({ schema, className, onChange, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: FilterBuilderSchema['onChange']; value?: FilterGroup; [key: string]: unknown } & RendererHostProps) => { // Read-time resolution against the display locale (objectui#4580 revised // Q1-A). `BaseSchema.label` accepts `string | I18nLabel`; rendering the map // straight into a text node THREW "Objects are not valid as a React child". const locale = useDisplayLocale(); - const handleChange = (value: any) => { - if (onChange) { - onChange({ - target: { - name: schema.name, - value: value, - }, - }); + const handleChange = (group: FilterBuilderRowGroup) => { + if (!onChange) return; + // Loud rather than lenient: handing the row on would make the declared + // `FilterGroup` false, and dropping it would lose the edit in silence. + if (!isAuthoredFilterGroup(group)) { + const offending = group.conditions.find((condition) => !PROTOCOL_OPERATOR_IDS.has(condition.operator)); + throw new Error( + `filter-builder: a row holds the operator "${offending?.operator}", which is outside the protocol's ` + + 'view-filter operators, so the FilterGroup that onChange hands over cannot carry it.', + ); } + onChange(group); }; return ( From e285c684c0819d074fedd0441edb5cb96ae003a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 08:29:57 +0000 Subject: [PATCH 2/4] test(components): pin the off-contract operator refusal in the filter-builder renderer Claude-Session: https://claude.ai/code/session_01ADHkmNhEw4jVVQ4UTxrxMZ Co-authored-by: Claude --- ...r-builder-renderer-onchange-26382.test.tsx | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx b/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx index 3e0dc875d1f..f53b837fdf4 100644 --- a/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx +++ b/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx @@ -85,6 +85,39 @@ describe('filter-builder renderer hands onChange the declared FilterGroup (#2638 expect(onChange.mock.calls[0][0].conditions).toHaveLength(2); }); + it('refuses loudly, rather than handing on, a row the FilterGroup cannot carry', () => { + const onChange = vi.fn<(filter: FilterGroup) => void>(); + // Off contract on purpose: `exists` is the builder's opt-in operator, not + // a protocol one, and the zod mirror refuses it in an authored `value`. + const offContract = { + ...VALUE, + conditions: [{ id: 'c1', field: 'name', operator: 'exists' }], + }; + renderDirect({ + schema: { type: 'filter-builder', name: 'filters', fields: FIELDS, value: offContract }, + onChange, + }); + + // A throw inside a React event handler does not reach `fireEvent`'s caller: + // the DOM reports it as an uncaught error on `window`. Read it there, and + // `preventDefault()` so jsdom does not also print it as unhandled. + const reported: unknown[] = []; + const onError = (event: ErrorEvent) => { + reported.push(event.error); + event.preventDefault(); + }; + window.addEventListener('error', onError); + try { + fireEvent.click(screen.getByRole('button', { name: /add filter/i })); + } finally { + window.removeEventListener('error', onError); + } + + expect(reported).toHaveLength(1); + expect(String(reported[0])).toMatch(/operator "exists", which is outside the protocol/); + expect(onChange).not.toHaveBeenCalled(); + }); + it('pins the declared signature at the type level', () => { expectTypeOf>().parameter(0).toEqualTypeOf(); From eb17b2a653ebad863697dab73048c5e56a8c55a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 08:31:04 +0000 Subject: [PATCH 3/4] chore(changeset): filter-builder renderer onChange hands over the FilterGroup Claude-Session: https://claude.ai/code/session_01ADHkmNhEw4jVVQ4UTxrxMZ Co-authored-by: Claude --- .changeset/26382-filter-builder-onchange-group.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/26382-filter-builder-onchange-group.md diff --git a/.changeset/26382-filter-builder-onchange-group.md b/.changeset/26382-filter-builder-onchange-group.md new file mode 100644 index 00000000000..e66684c47e7 --- /dev/null +++ b/.changeset/26382-filter-builder-onchange-group.md @@ -0,0 +1,12 @@ +--- +'@object-ui/components': patch +--- + +fix(components): the `filter-builder` renderer calls `onChange` with the `FilterGroup` that `FilterBuilderSchema.onChange` declares + +Clause-②: no +Metadata-protocol: none + +`FilterBuilderSchema.onChange` declares `(filter: FilterGroup) => void`, but the `filter-builder` renderer typed its own `onChange` as `(event: any) => void` and called it with a form-style `{ target: { name, value } }` event. A handler written from the declaration read `.logic` and `.conditions` off that event and got `undefined`, and the `any` kept the compiler from reporting the mismatch. The renderer now takes the schema's own `onChange` type and passes the group it holds, so the declaration is the contract on both sides. No in-repo host bound the event shape. A host that read `event.target.value` reads the argument itself instead. + +The builder's own rows also admit the opt-in `exists` / `notExists` operators, which a `FilterGroup` cannot carry. This node grants neither, and the `FilterGroup` zod mirror refuses them in an authored `value`. If one still reaches the renderer, it throws an error naming the operator instead of handing on a group the declared type does not describe. From 39ae55ecc7c57fef7e3981e7614be62e9ffb9a4c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 09:25:55 +0000 Subject: [PATCH 4/4] fix(components): filter-builder renderer keeps a host's extraOperators out of the builder The node declares no extraOperators, and what they grant (exists / notExists) is outside the FilterGroup onChange declares. The off-contract branch, now reachable only through a stored value that skipped validation, skips the emit and logs instead of throwing in a click handler. Claude-Session: https://claude.ai/code/session_01ADHkmNhEw4jVVQ4UTxrxMZ Co-authored-by: Claude --- .../26382-filter-builder-onchange-group.md | 2 +- ...r-builder-renderer-onchange-26382.test.tsx | 58 ++++++++++++------- .../src/renderers/complex/filter-builder.tsx | 31 +++++++--- 3 files changed, 60 insertions(+), 31 deletions(-) diff --git a/.changeset/26382-filter-builder-onchange-group.md b/.changeset/26382-filter-builder-onchange-group.md index e66684c47e7..00da71ef1d5 100644 --- a/.changeset/26382-filter-builder-onchange-group.md +++ b/.changeset/26382-filter-builder-onchange-group.md @@ -9,4 +9,4 @@ Metadata-protocol: none `FilterBuilderSchema.onChange` declares `(filter: FilterGroup) => void`, but the `filter-builder` renderer typed its own `onChange` as `(event: any) => void` and called it with a form-style `{ target: { name, value } }` event. A handler written from the declaration read `.logic` and `.conditions` off that event and got `undefined`, and the `any` kept the compiler from reporting the mismatch. The renderer now takes the schema's own `onChange` type and passes the group it holds, so the declaration is the contract on both sides. No in-repo host bound the event shape. A host that read `event.target.value` reads the argument itself instead. -The builder's own rows also admit the opt-in `exists` / `notExists` operators, which a `FilterGroup` cannot carry. This node grants neither, and the `FilterGroup` zod mirror refuses them in an authored `value`. If one still reaches the renderer, it throws an error naming the operator instead of handing on a group the declared type does not describe. +The builder's own rows also admit the opt-in `exists` / `notExists` operators, which a `FilterGroup` cannot carry. The renderer no longer forwards a host's `extraOperators` to the builder, since `FilterBuilderSchema` declares no such key, so the node never offers them. The `FilterGroup` zod mirror refuses them in an authored `value`. If a stored value that skipped that validation still carries one, the renderer does not call `onChange` and logs a console error naming the operator, instead of handing on a group the declared type does not describe. diff --git a/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx b/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx index f53b837fdf4..1d9b4b4ca01 100644 --- a/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx +++ b/ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx @@ -23,7 +23,7 @@ import * as React from 'react'; import { describe, it, expect, expectTypeOf, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { I18nProvider } from '@object-ui/i18n'; import { ComponentRegistry } from '@object-ui/core'; import type { FilterBuilderSchema, FilterGroup } from '@object-ui/types'; @@ -85,37 +85,53 @@ describe('filter-builder renderer hands onChange the declared FilterGroup (#2638 expect(onChange.mock.calls[0][0].conditions).toHaveLength(2); }); - it('refuses loudly, rather than handing on, a row the FilterGroup cannot carry', () => { + it('keeps a host-forwarded extraOperators out of the builder, so exists / notExists are never offered', async () => { + // `extraOperators` is the builder's only operator-granting prop, and what it + // grants is outside the FilterGroup that onChange declares. A host that + // forwards it through the node must not widen the dropdown. + renderDirect({ + schema: { type: 'filter-builder', name: 'filters', fields: FIELDS, value: VALUE }, + extraOperators: ['exists', 'notExists'], + onChange: vi.fn<(filter: FilterGroup) => void>(), + }); + + // The row's controls are Radix comboboxes in field / operator / value + // order, so index 1 is the operator. + fireEvent.keyDown(screen.getAllByRole('combobox')[1], { key: 'ArrowDown' }); + const labels = await waitFor(() => { + const options = screen.getAllByRole('option').map((o) => o.textContent); + expect(options, 'the operator dropdown did not open').toContain('Is empty'); + return options; + }); + expect(labels).not.toContain('Is set'); + expect(labels).not.toContain('Is not set'); + }); + + it('skips the emit, and names the operator on the console, for a stored row the FilterGroup cannot carry', () => { const onChange = vi.fn<(filter: FilterGroup) => void>(); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); // Off contract on purpose: `exists` is the builder's opt-in operator, not // a protocol one, and the zod mirror refuses it in an authored `value`. + // A stored value that skipped that validation is the one way in left. const offContract = { ...VALUE, conditions: [{ id: 'c1', field: 'name', operator: 'exists' }], }; - renderDirect({ - schema: { type: 'filter-builder', name: 'filters', fields: FIELDS, value: offContract }, - onChange, - }); - - // A throw inside a React event handler does not reach `fireEvent`'s caller: - // the DOM reports it as an uncaught error on `window`. Read it there, and - // `preventDefault()` so jsdom does not also print it as unhandled. - const reported: unknown[] = []; - const onError = (event: ErrorEvent) => { - reported.push(event.error); - event.preventDefault(); - }; - window.addEventListener('error', onError); try { + renderDirect({ + schema: { type: 'filter-builder', name: 'filters', fields: FIELDS, value: offContract }, + onChange, + }); + fireEvent.click(screen.getByRole('button', { name: /add filter/i })); + + expect(onChange).not.toHaveBeenCalled(); + const messages = consoleError.mock.calls.map((args) => String(args[0])); + expect(messages.filter((m) => m.startsWith('filter-builder: onChange not called'))).toHaveLength(1); + expect(messages.join('\n')).toMatch(/operator "exists", which is outside the protocol/); } finally { - window.removeEventListener('error', onError); + consoleError.mockRestore(); } - - expect(reported).toHaveLength(1); - expect(String(reported[0])).toMatch(/operator "exists", which is outside the protocol/); - expect(onChange).not.toHaveBeenCalled(); }); it('pins the declared signature at the type level', () => { diff --git a/ui/packages/components/src/renderers/complex/filter-builder.tsx b/ui/packages/components/src/renderers/complex/filter-builder.tsx index 96162878fc0..5645f25cbc2 100644 --- a/ui/packages/components/src/renderers/complex/filter-builder.tsx +++ b/ui/packages/components/src/renderers/complex/filter-builder.tsx @@ -24,10 +24,11 @@ const PROTOCOL_OPERATOR_IDS: ReadonlySet = new Set(VIEW_FILTER_OPERATORS * i.e. is it the `FilterGroup` that `FilterBuilderSchema.onChange` declares? * * The builder's own row also admits the opt-in `exists` / `notExists`, which a - * `FilterGroup` cannot carry. This node grants neither (`FilterBuilderSchema` - * declares no `extraOperators`), and its `value` is a `FilterGroup`, whose zod + * `FilterGroup` cannot carry. This node never grants them: `FilterBuilderSchema` + * declares no `extraOperators`, and the renderer below keeps a host's + * `extraOperators` out of the builder. Its `value` is a `FilterGroup`, whose zod * mirror refuses an operator outside the protocol. A group that fails here - * therefore reached the builder off contract. + * therefore came from a stored `value` that skipped that validation. */ function isAuthoredFilterGroup(group: FilterBuilderRowGroup): group is FilterBuilderRowGroup & FilterGroup { return group.conditions.every((condition) => PROTOCOL_OPERATOR_IDS.has(condition.operator)); @@ -43,21 +44,33 @@ ComponentRegistry.register('filter-builder', // form-style `{ target: { name, value } }` event, so a handler written from // `FilterBuilderSchema.onChange` read `.logic` / `.conditions` off the event // and got `undefined`, and the `any` kept the compiler from saying so. - ({ schema, className, onChange, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: FilterBuilderSchema['onChange']; value?: FilterGroup; [key: string]: unknown } & RendererHostProps) => { + // + // `extraOperators` is taken out of the host spread and never reaches + // `FilterBuilder`. It is the builder's only operator-granting prop, and what + // it grants (`exists` / `notExists`) is outside the `FilterGroup` that + // `onChange` declares. `FilterBuilderSchema` declares no such key, so a host + // that forwards one is off contract, and the builder offers only protocol + // operators. + ({ schema, className, onChange, extraOperators: _refusedExtraOperators, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: FilterBuilderSchema['onChange']; value?: FilterGroup; [key: string]: unknown } & RendererHostProps) => { // Read-time resolution against the display locale (objectui#4580 revised // Q1-A). `BaseSchema.label` accepts `string | I18nLabel`; rendering the map // straight into a text node THREW "Objects are not valid as a React child". const locale = useDisplayLocale(); const handleChange = (group: FilterBuilderRowGroup) => { if (!onChange) return; - // Loud rather than lenient: handing the row on would make the declared - // `FilterGroup` false, and dropping it would lose the edit in silence. + // No dropdown on this node offers an operator outside the protocol, so + // only a stored `value` that skipped its zod validation reaches this + // branch. Handing the group on would make the declared `FilterGroup` + // false, and a throw here would crash a click in the user's hands, so the + // emit is skipped and the console names the operator. if (!isAuthoredFilterGroup(group)) { const offending = group.conditions.find((condition) => !PROTOCOL_OPERATOR_IDS.has(condition.operator)); - throw new Error( - `filter-builder: a row holds the operator "${offending?.operator}", which is outside the protocol's ` + - 'view-filter operators, so the FilterGroup that onChange hands over cannot carry it.', + console.error( + `filter-builder: onChange not called. A row holds the operator "${offending?.operator}", which is ` + + "outside the protocol's view-filter operators, so the FilterGroup that onChange hands over cannot carry it. " + + 'Fix the stored filter value.', ); + return; } onChange(group); };