From 088fef0366b82a4ba298b9a307545794202db9db Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 21:09:04 +0000 Subject: [PATCH 1/5] wip(components): filter-builder row takes the spec's rule value Claude-Session: https://claude.ai/code/session_01QGA6bzuch27y29tUbeb1y9 Co-authored-by: Claude --- .../components/src/custom/filter-builder.tsx | 48 ++++++++++++++----- .../src/renderers/complex/filter-builder.tsx | 9 ++-- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/ui/packages/components/src/custom/filter-builder.tsx b/ui/packages/components/src/custom/filter-builder.tsx index 3d6f94a5397..758a7dae1d6 100644 --- a/ui/packages/components/src/custom/filter-builder.tsx +++ b/ui/packages/components/src/custom/filter-builder.tsx @@ -14,6 +14,7 @@ import { VIEW_FILTER_LIST_VALUE_OPERATORS, VIEW_FILTER_PAIR_VALUE_OPERATORS, normalizeFilterOperator, + type ViewFilterRule, } from "@objectstack/spec/ui" import { expandEmptyOperator } from "@objectstack/spec/data" import { SchemaRendererContext } from "@object-ui/react" @@ -64,11 +65,17 @@ import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover" * protocol has no member for (objectui#9559 ruling B keeps them opt-in and * unfolded). It was `string`, which let a caller hand the builder an id no * dropdown entry can hold. - * - `value` — REQUIRED and narrowed to the scalar-or-list union this builder - * edits, where the authority declares `value?: any`. Every exported helper - * below that takes a row's value (`reshapeFilterValue`, `retypeFilterValue`, + * - `value` — the SPEC's rule value, taken BY REFERENCE + * (`ViewFilterRule["value"]`, `@objectstack/spec/ui`), where the authority + * declares `value?: any`. Every exported helper below that takes a row's + * value (`reshapeFilterValue`, `retypeFilterValue`, * `narrowFilterValueToOptions`, …) is typed on this member, so `any` here - * would switch their checking off in silence. + * would switch their checking off in silence. OPTIONAL and admitting + * `null`, as the spec's is: a value-less operator (`is_null`, `is_empty`, + * …) is authored with no `value`, so a required member refused the + * authored `FilterGroup` the `filter-builder` renderer hands over. A list + * or pair holds strings and numbers only, as the spec's does — a boolean + * carried into one is spelled as its string (`normalizeToArray`). * * Its own NAME, not the authority's (objectui#6349, batch 12). Under the * authority's name this was a second declaration of `FilterBuilderCondition` @@ -81,7 +88,7 @@ import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover" export interface FilterBuilderRow extends Omit { operator: FilterBuilderOperator - value: string | number | boolean | (string | number | boolean)[] + value?: ViewFilterRule["value"] } /** @@ -859,21 +866,26 @@ export function retypeFilterValue( switch (filterValueArity(operator)) { case "list": { - const carried: (string | number | boolean)[] = [] + const carried: (string | number)[] = [] for (const entry of normalizeToArray(value)) { const converted = convertScalarToFamily(entry, family) - if (converted !== undefined && converted !== "") carried.push(converted) + if (converted !== undefined && converted !== "") carried.push(toListEntry(converted)) } return carried } case "pair": { const [min = "", max = ""] = normalizeToArray(value) - const lower = convertScalarToFamily(min, family) ?? "" - const upper = convertScalarToFamily(max, family) ?? "" + const lower = toListEntry(convertScalarToFamily(min, family) ?? "") + const upper = toListEntry(convertScalarToFamily(max, family) ?? "") return lower === "" && upper === "" ? [] : [lower, upper] } default: { const scalar = Array.isArray(value) ? (value[0] ?? "") : value + // A row with no value — the spec's value-less operators are authored + // without one — keeps having none. Converting the absence instead read + // `undefined` as a value: the text family spelled it `"undefined"` and + // the number family threw on `.trim()`. + if (isValueUnset(scalar)) return scalar return convertScalarToFamily(scalar, family) ?? "" } } @@ -1340,7 +1352,7 @@ function offeredOperatorsForRow( */ function isValueUnset( value: FilterBuilderRow["value"] | undefined | null, -): boolean { +): value is undefined | null | "" { return value === undefined || value === null || value === "" } @@ -1358,10 +1370,22 @@ function displayScalarValue(value: FilterBuilderRow["value"]): string { return isValueUnset(value) ? "" : String(value) } +/** + * One entry of a list or pair value, in the spec's element vocabulary: a + * `ViewFilterRule` list holds strings and numbers, never booleans. A boolean + * that reaches a list — an authored `equals true` row whose operator is then + * switched to `in` — is spelled as its string, which is also the key a static + * option compares on (`allowed.has(String(entry))`), so the entry the user + * chose stays the entry they see. + */ +function toListEntry(entry: string | number | boolean): string | number { + return typeof entry === "boolean" ? String(entry) : entry +} + /** Normalize a filter value into an array for multi-select scenarios */ -function normalizeToArray(value: FilterBuilderRow["value"]): (string | number | boolean)[] { +function normalizeToArray(value: FilterBuilderRow["value"]): (string | number)[] { if (Array.isArray(value)) return value - if (!isValueUnset(value)) return [value as string | number | boolean] + if (!isValueUnset(value)) return [toListEntry(value)] return [] } diff --git a/ui/packages/components/src/renderers/complex/filter-builder.tsx b/ui/packages/components/src/renderers/complex/filter-builder.tsx index 56a41793f00..1700d74514d 100644 --- a/ui/packages/components/src/renderers/complex/filter-builder.tsx +++ b/ui/packages/components/src/renderers/complex/filter-builder.tsx @@ -7,7 +7,7 @@ */ import { ComponentRegistry } from '@object-ui/core'; -import type { FilterBuilderSchema } from '@object-ui/types'; +import type { FilterBuilderSchema, FilterGroup } from '@object-ui/types'; import { useDisplayLocale } from '@object-ui/i18n'; // Aliased on import, following PR #4169's convention (and `AppSchemaRenderer`'s // use of it): this repo has its OWN `resolveKeyedI18nLabel` over a DIFFERENT @@ -17,7 +17,10 @@ import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui import { FilterBuilder } from '../../custom/filter-builder'; ComponentRegistry.register('filter-builder', - ({ schema, className, onChange, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: (event: any) => void; [key: string]: any }) => { + // `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]: any }) => { // 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". @@ -41,7 +44,7 @@ ComponentRegistry.register('filter-builder', )} Date: Mon, 26 Oct 2026 21:12:51 +0000 Subject: [PATCH 2/5] fix(components)!: filter-builder row takes the spec rule's value Claude-Session: https://claude.ai/code/session_01QGA6bzuch27y29tUbeb1y9 Co-authored-by: Claude --- .../26306-filter-builder-spec-row-value.md | 26 ++++ ui/packages/components/docs/FilterBuilder.md | 7 +- ...ilter-builder-name-authority-9306.test.tsx | 29 ++-- .../filter-builder-spec-row-26306.test.tsx | 134 ++++++++++++++++++ .../components/src/custom/filter-builder.tsx | 12 +- 5 files changed, 191 insertions(+), 17 deletions(-) create mode 100644 .changeset/26306-filter-builder-spec-row-value.md create mode 100644 ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx diff --git a/.changeset/26306-filter-builder-spec-row-value.md b/.changeset/26306-filter-builder-spec-row-value.md new file mode 100644 index 00000000000..3af21e527fb --- /dev/null +++ b/.changeset/26306-filter-builder-spec-row-value.md @@ -0,0 +1,26 @@ +--- +'@object-ui/components': patch +--- + +fix(components)!: the filter builder's row takes the spec rule's value, so the authored group the `filter-builder` node carries reaches the builder as it is + +Clause-②: yes (narrowing) +Metadata-protocol: none + + + +**BREAKING** type-surface change, compile-time only for well-formed rows. + +`FilterBuilderRow.value` is now `ViewFilterRule['value']` from `@objectstack/spec/ui`, taken by reference, instead of the builder's own `string | number | boolean | (string | number | boolean)[]`. Measured against the spec's rule, the old member diverged in three value cases: + +- **Value omitted.** The spec's value is optional, because a value-less operator (`is_null`, `is_empty`, …) carries none. The builder's was required, so the authored `FilterGroup` a `filter-builder` node holds was not assignable to the builder's group. The member is optional now. +- **Value `null`.** The spec admits it; the builder's member did not. It does now. +- **A boolean inside a list or pair.** The spec's list holds strings and numbers. The builder's admitted booleans and emitted them: an `equals true` row switched to `in` came back as `[true]`, which the spec's rule schema refuses. A boolean carried into a list or pair is now spelled as its string (`['true']`), the key a static option already compares on. + +The operator vocabulary needed no change: every spec operator id is a builder id. + +Two runtime fixes ride on the first two cases. Switching the field of a row with no value used to convert the absent value: on a number column it threw on `.trim()`, and on a text column it came back as the string `"undefined"`. A row with no value, or a `null` one, now keeps it through a field switch. + +The `filter-builder` renderer hands the authored `value` and `fields` to the builder with no cast. + +**Migration.** Code that reads `row.value` from a `FilterBuilderRow` now handles `undefined` and `null`, which the builder already passed through at runtime. Code that writes a boolean into a row's list value writes its string instead. diff --git a/ui/packages/components/docs/FilterBuilder.md b/ui/packages/components/docs/FilterBuilder.md index efa0ba3fc43..7c5180b652f 100644 --- a/ui/packages/components/docs/FilterBuilder.md +++ b/ui/packages/components/docs/FilterBuilder.md @@ -122,7 +122,8 @@ interface Field { The schema's `value` is `FilterGroup` from `@object-ui/types`, the authored group. The React component's `value` / `onChange` use `FilterBuilderRowGroup` from `@object-ui/components`: the same group, derived from `FilterGroup`, whose rows -are `FilterBuilderRow` (the builder's own operator ids and a required value). +are `FilterBuilderRow` (the builder's own operator ids and the spec rule's value), +so an authored group passes to the component as it is. ```typescript interface FilterBuilderRowGroup { @@ -135,7 +136,9 @@ interface FilterBuilderRow { id: string; // Row identifier (required) field: string; // Field value operator: FilterBuilderOperator; // Operator id (see below) - value: string | number | boolean | (string | number | boolean)[]; // Filter value + value?: ViewFilterRule['value']; // `@objectstack/spec/ui`: optional (value-less + // operators carry none), may be null; a list or + // pair holds strings and numbers } ``` diff --git a/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx b/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx index b5cb5dc7b63..a2ee5e64a12 100644 --- a/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx +++ b/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx @@ -20,7 +20,7 @@ * * - `operator` — `FilterBuilderOperator`, the protocol's ids plus the opt-in * `exists` / `notExists` (objectui#9559 ruling B); - * - `value` — required and narrowed to what this builder edits. + * - `value` — the spec rule's value, by reference (#26306). * * Everything else comes from the authority, and the group restates only * `conditions` (to hold this package's row), so its `id` is optional here as it @@ -41,7 +41,7 @@ import { describe, it, expect, vi } from 'vitest'; import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; import '@testing-library/jest-dom'; -import type { ViewFilterOperator } from '@objectstack/spec/ui'; +import type { ViewFilterOperator, ViewFilterRule } from '@objectstack/spec/ui'; import type { FilterBuilderCondition as AuthorityCondition, FilterGroup as AuthorityGroup, @@ -97,12 +97,22 @@ const offVocabulary: FilterBuilderRow = { value: 1, }; -// The value extension: the builder's own scalar-or-list union, REQUIRED — -// ruled over the authority's `value?: any`, which would switch the typing of -// every exported value helper off in silence. -expectType>(); -// @ts-expect-error `value` is required on the builder's row -const rowWithoutValue: FilterBuilderRow = { id: 'c1', field: 'a', operator: 'equals' }; +// The value extension: the SPEC rule's value, by reference (#26306) — never the +// authority's `value?: any`, which would switch the typing of every exported +// value helper off in silence. Optional, as the spec's is: a value-less +// operator is authored with no value, and a required member refused the +// authored group the renderer hands over. The per-case pins live in +// `filter-builder-spec-row-26306.test.tsx`. +expectType>(); +const rowWithoutValue: FilterBuilderRow = { id: 'c1', field: 'a', operator: 'is_null' }; +// …and still not `any`: a value the spec refuses is refused here. +const objectValue: FilterBuilderRow = { + id: 'c1', + field: 'a', + operator: 'equals', + // @ts-expect-error `value` is the spec rule's value, not an object + value: { $gt: 1 }, +}; /* ── The group: only `conditions` is restated ─────────────────────────────── */ @@ -123,7 +133,8 @@ describe('objectui#9306 — the builder derives its row and group from @object-u it('the type-level pins above compile (see `tsc -p tsconfig.test.json`); the fixtures are live values', () => { expect(rowWithoutId.field).toBe('a'); expect(offVocabulary.id).toBe('c1'); - expect(rowWithoutValue.operator).toBe('equals'); + expect(rowWithoutValue.operator).toBe('is_null'); + expect(objectValue.id).toBe('c1'); expect(groupWithoutId.conditions).toEqual([]); expect(nested.logic).toBe('and'); }); diff --git a/ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx b/ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx new file mode 100644 index 00000000000..808c65f8f01 --- /dev/null +++ b/ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx @@ -0,0 +1,134 @@ +/** + * 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 builder accepts the spec-derived group the `filter-builder` renderer + * hands it (#26306). + * + * The renderer passes `schema.value` — the authored `FilterGroup` from + * `@object-ui/types`, whose row takes its operator from the spec's + * `ViewFilterOperator` — to a builder typed on its own `FilterBuilderRowGroup`. + * The two were assignable in neither direction, and the renderer props' `any` + * index signature hid that. Measured against the spec's `ViewFilterRule` + * (`@objectstack/spec/ui`), the row diverged in three VALUE cases and in no + * operator case a consumer can author: + * + * | case | spec rule | builder row (before) | + * |--------------------------------|------------------------|---------------------------------| + * | value omitted (unary operator) | `value?` | `value` required | + * | value `null` | admitted | refused | + * | boolean entry in a list / pair | refused | admitted, and emitted | + * | operator | `VIEW_FILTER_OPERATORS` | those + opt-in `exists` / `notExists` | + * + * The operator row is not a divergence on the way IN: every spec id is a + * builder id. The opt-in pair is the builder's emit-side extension (objectui#9559 + * ruling B) and is pinned in `filter-builder-name-authority-9306.test.tsx`. + * + * The builder's row now takes the spec's value by reference + * (`ViewFilterRule["value"]`), so each value case has one pin below. The first + * two were not only type gaps: a field switch converted the ABSENT value, so a + * value-less row on a number column threw on `.trim()` and on a text column + * came back as the string `"undefined"`. + * + * Type-level pins are judged by `tsc -p tsconfig.test.json`; a green vitest run + * says nothing about them. + */ +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ViewFilterRuleSchema, type ViewFilterRule } from '@objectstack/spec/ui'; +import type { FilterField, FilterGroup } from '@object-ui/types'; +import { + FilterBuilder, + retypeFilterValue, + type FilterBuilderProps, + type FilterBuilderRow, +} from '../custom/filter-builder'; + +type Equal = + (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false; +function expectType(_: T = true as T): void { /* compile-time only */ } +type Assignable = [From] extends [To] ? true : false; + +/* ── The shapes ───────────────────────────────────────────────────────────── */ + +// The row's value IS the spec rule's value — by reference, not a copy of it. +expectType>(); +// The renderer's two reads cross with no cast: the authored group and the +// authored field list are what the builder's props take. +expectType>>(); +expectType>>(); + +const FIELDS: NonNullable = [ + { value: 'title', label: 'Title', type: 'text' }, + { value: 'amount', label: 'Amount', type: 'number' }, + { value: 'quota', label: 'Quota', type: 'currency' }, + { value: 'stage', label: 'Stage', type: 'select' }, +]; + +/** Render ONE authored row, typed as the authored group — no cast. */ +function renderAuthored(row: FilterGroup['conditions'][number]) { + const onChange = vi.fn(); + // Hoisted out of the JSX: the sync effect re-seeds state when the `value` + // prop's identity changes, which would undo the interaction under test. + const value: FilterGroup = { logic: 'and', conditions: [row] }; + render(); + return onChange; +} + +function lastRow(onChange: ReturnType): FilterBuilderRow { + expect(onChange.mock.calls.length, 'the builder never called onChange').toBeGreaterThan(0); + return onChange.mock.calls.at(-1)?.[0].conditions[0]; +} + +/** Drive the row's REAL dropdown at `index` (0 = field, 1 = operator). */ +async function pick(index: number, label: string) { + fireEvent.keyDown(screen.getAllByRole('combobox')[index], { key: 'ArrowDown' }); + const option = await waitFor(() => { + const found = screen.getAllByRole('option').find((o) => o.textContent === label); + expect(found, `no "${label}" option in dropdown ${index}`).toBeTruthy(); + return found!; + }); + fireEvent.click(option); +} + +describe('#26306 — the builder accepts the spec-derived row', () => { + it('value omitted: a value-less row keeps no value through a field switch (it used to throw on a number column)', async () => { + const onChange = renderAuthored({ id: 'c1', field: 'title', operator: 'is_null' }); + await pick(0, 'Amount'); + const row = lastRow(onChange); + expect(row.field).toBe('amount'); + expect(row.operator).toBe('is_null'); + expect(row.value).toBeUndefined(); + // The text family, where the absence used to come back spelled out. + expect(retypeFilterValue(undefined, 'text', 'is_null')).toBeUndefined(); + }); + + it('value null: a null row keeps null through a field switch (it used to throw on `.trim()`)', async () => { + const onChange = renderAuthored({ id: 'c1', field: 'amount', operator: 'equals', value: null }); + // Drawn as unfilled, not as the string "null". + expect((document.querySelector('input[placeholder="Value"]') as HTMLInputElement).value).toBe(''); + await pick(0, 'Quota'); + const row = lastRow(onChange); + expect(row.field).toBe('quota'); + expect(row.value).toBeNull(); + }); + + it('boolean in a list: a boolean carried into `in` is emitted as the list the spec accepts', async () => { + const onChange = renderAuthored({ id: 'c1', field: 'stage', operator: 'equals', value: true }); + await pick(1, 'In'); + const row = lastRow(onChange); + expect(row.operator).toBe('in'); + expect(row.value).toEqual(['true']); + // The emitted row is one the spec's rule schema parses — `[true]` was not. + expect( + ViewFilterRuleSchema.safeParse({ field: row.field, operator: row.operator, value: row.value }).success, + ).toBe(true); + }); +}); diff --git a/ui/packages/components/src/custom/filter-builder.tsx b/ui/packages/components/src/custom/filter-builder.tsx index 758a7dae1d6..2c2bb48d057 100644 --- a/ui/packages/components/src/custom/filter-builder.tsx +++ b/ui/packages/components/src/custom/filter-builder.tsx @@ -1112,7 +1112,7 @@ export function isFilterValueComplete( /** The two bounds a `pair` row edits, with the gaps filled in for rendering. */ function toPairBounds( value: FilterBuilderRow["value"], -): [string | number | boolean, string | number | boolean] { +): [string | number, string | number] { const [min = "", max = ""] = normalizeToArray(value) return [min, max] } @@ -1835,7 +1835,7 @@ function FilterBuilder({ // refusal. `|| undefined` so a complete pair carries NO attribute at all. const boundInvalid = (index: 0 | 1) => (missingBound === index ? true : undefined) const setBound = (index: 0 | 1, raw: string) => { - const next: [string | number | boolean, string | number | boolean] = [bounds[0], bounds[1]] + const next: [string | number, string | number] = [bounds[0], bounds[1]] // The same one reading as the field-switch path (objectui#4875), in // place of `parseFloat(raw) || 0`: a bound is a clean number or it is // not filled in (`""`). A half-read bound (`"42abc"` → 42) or an @@ -1843,7 +1843,7 @@ function FilterBuilder({ // silently narrows a RANGE, and `0` is a plausible-looking bound. // `convertScalarToFamily("", "number")` is `""`, so the old // `raw !== ""` guard is subsumed rather than dropped. - next[index] = numeric ? (convertScalarToFamily(raw, "number") ?? "") : raw + next[index] = numeric ? toListEntry(convertScalarToFamily(raw, "number") ?? "") : raw // Both bounds cleared → back to the "nothing filled in yet" shape the // write path drops, rather than persisting an empty range. updateCondition(condition.id, { @@ -2305,13 +2305,13 @@ function TemporalValueEditor({ // ============================================================================ interface MultiValueInputProps { - values: (string | number | boolean)[] + values: (string | number)[] /** The `` the field's own type asks for (`text` / `number` / …). */ inputType: string /** Numeric field → committed tokens are numbers, so the wire carries numbers. */ numeric: boolean testId: string - onChange: (next: (string | number | boolean)[]) => void + onChange: (next: (string | number)[]) => void } /** @@ -2346,7 +2346,7 @@ function MultiValueInput({ values, inputType, numeric, testId, onChange }: Multi if (next === undefined || next === "") return setDraft("") if (values.some((v) => String(v) === String(next))) return - onChange([...values, next]) + onChange([...values, toListEntry(next)]) } const removeAt = (index: number) => { From 2bba01fa09bd3a49ea06b45010365299e9b3cf19 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 21:26:39 +0000 Subject: [PATCH 3/5] fix(components): keep the builder's boolean list member; the row's value widens to the spec's Narrowing the list member to the spec's is a breaking change to a published type with a consumer in @object-ui/plugin-list (UserFilters hands LookupValuePicker a boolean-bearing list); it is reported, not made here. Claude-Session: https://claude.ai/code/session_01QGA6bzuch27y29tUbeb1y9 Co-authored-by: Claude --- .../26306-filter-builder-spec-row-value.md | 19 +++---- ui/packages/components/docs/FilterBuilder.md | 6 +-- ...ilter-builder-name-authority-9306.test.tsx | 5 +- .../filter-builder-spec-row-26306.test.tsx | 40 +++++++------- .../components/src/custom/filter-builder.tsx | 52 ++++++++----------- 5 files changed, 53 insertions(+), 69 deletions(-) diff --git a/.changeset/26306-filter-builder-spec-row-value.md b/.changeset/26306-filter-builder-spec-row-value.md index 3af21e527fb..f499559581e 100644 --- a/.changeset/26306-filter-builder-spec-row-value.md +++ b/.changeset/26306-filter-builder-spec-row-value.md @@ -2,25 +2,20 @@ '@object-ui/components': patch --- -fix(components)!: the filter builder's row takes the spec rule's value, so the authored group the `filter-builder` node carries reaches the builder as it is +fix(components): the filter builder's row takes the spec rule's value, so the authored group a `filter-builder` node carries reaches the builder as it is -Clause-②: yes (narrowing) +Clause-②: yes (widening) Metadata-protocol: none - +`FilterBuilderRow.value` is now `ViewFilterRule['value']` from `@objectstack/spec/ui`, taken by reference, plus the builder's own list member `(string | number | boolean)[]`. Before, it was the builder's own `string | number | boolean | (string | number | boolean)[]`, and it was required. Measured against the spec's rule, that refused two shapes an authored row takes: -**BREAKING** type-surface change, compile-time only for well-formed rows. - -`FilterBuilderRow.value` is now `ViewFilterRule['value']` from `@objectstack/spec/ui`, taken by reference, instead of the builder's own `string | number | boolean | (string | number | boolean)[]`. Measured against the spec's rule, the old member diverged in three value cases: - -- **Value omitted.** The spec's value is optional, because a value-less operator (`is_null`, `is_empty`, …) carries none. The builder's was required, so the authored `FilterGroup` a `filter-builder` node holds was not assignable to the builder's group. The member is optional now. +- **Value omitted.** The spec's value is optional, because a value-less operator (`is_null`, `is_empty`, …) carries none. The builder's was required, so the authored `FilterGroup` from `@object-ui/types` was not assignable to the builder's group. The member is optional now. - **Value `null`.** The spec admits it; the builder's member did not. It does now. -- **A boolean inside a list or pair.** The spec's list holds strings and numbers. The builder's admitted booleans and emitted them: an `equals true` row switched to `in` came back as `[true]`, which the spec's rule schema refuses. A boolean carried into a list or pair is now spelled as its string (`['true']`), the key a static option already compares on. The operator vocabulary needed no change: every spec operator id is a builder id. -Two runtime fixes ride on the first two cases. Switching the field of a row with no value used to convert the absent value: on a number column it threw on `.trim()`, and on a text column it came back as the string `"undefined"`. A row with no value, or a `null` one, now keeps it through a field switch. +Both cases also fix a field switch. Switching the field of a row with no value used to convert the absent value: on a number column it threw on `.trim()`, and on a text column it came back as the string `"undefined"`. A row with no value, or a `null` one, now keeps it through a field switch. -The `filter-builder` renderer hands the authored `value` and `fields` to the builder with no cast. +The `filter-builder` renderer now hands the authored `value` and `fields` to the builder with no cast. It declares `value` on its props, because a host that binds the node forwards the group it holds. -**Migration.** Code that reads `row.value` from a `FilterBuilderRow` now handles `undefined` and `null`, which the builder already passed through at runtime. Code that writes a boolean into a row's list value writes its string instead. +Code that reads `row.value` from a `FilterBuilderRow` now sees `undefined` and `null` in its type. The builder already passed both through at runtime. diff --git a/ui/packages/components/docs/FilterBuilder.md b/ui/packages/components/docs/FilterBuilder.md index 7c5180b652f..0e2824d9990 100644 --- a/ui/packages/components/docs/FilterBuilder.md +++ b/ui/packages/components/docs/FilterBuilder.md @@ -136,9 +136,9 @@ interface FilterBuilderRow { id: string; // Row identifier (required) field: string; // Field value operator: FilterBuilderOperator; // Operator id (see below) - value?: ViewFilterRule['value']; // `@objectstack/spec/ui`: optional (value-less - // operators carry none), may be null; a list or - // pair holds strings and numbers + value?: ViewFilterRule['value'] | (string | number | boolean)[]; + // `ViewFilterRule` from `@objectstack/spec/ui`: optional (value-less operators + // carry none) and may be null; the builder's own list may also hold booleans } ``` diff --git a/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx b/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx index a2ee5e64a12..35eb5e0df7b 100644 --- a/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx +++ b/ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx @@ -97,13 +97,14 @@ const offVocabulary: FilterBuilderRow = { value: 1, }; -// The value extension: the SPEC rule's value, by reference (#26306) — never the +// The value extension: the SPEC rule's value, by reference (#26306), plus the +// builder's own boolean-bearing list — never the // authority's `value?: any`, which would switch the typing of every exported // value helper off in silence. Optional, as the spec's is: a value-less // operator is authored with no value, and a required member refused the // authored group the renderer hands over. The per-case pins live in // `filter-builder-spec-row-26306.test.tsx`. -expectType>(); +expectType>(); const rowWithoutValue: FilterBuilderRow = { id: 'c1', field: 'a', operator: 'is_null' }; // …and still not `any`: a value the spec refuses is refused here. const objectValue: FilterBuilderRow = { diff --git a/ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx b/ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx index 808c65f8f01..e6267077c4c 100644 --- a/ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx +++ b/ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx @@ -22,7 +22,7 @@ * |--------------------------------|------------------------|---------------------------------| * | value omitted (unary operator) | `value?` | `value` required | * | value `null` | admitted | refused | - * | boolean entry in a list / pair | refused | admitted, and emitted | + * | boolean entry in a list / pair | refused | admitted (kept, see below) | * | operator | `VIEW_FILTER_OPERATORS` | those + opt-in `exists` / `notExists` | * * The operator row is not a divergence on the way IN: every spec id is a @@ -30,10 +30,18 @@ * ruling B) and is pinned in `filter-builder-name-authority-9306.test.tsx`. * * The builder's row now takes the spec's value by reference - * (`ViewFilterRule["value"]`), so each value case has one pin below. The first - * two were not only type gaps: a field switch converted the ABSENT value, so a - * value-less row on a number column threw on `.trim()` and on a text column - * came back as the string `"undefined"`. + * (`ViewFilterRule["value"]`), so the two cases that kept an authored group out + * of the builder have one pin each below. Neither was only a type gap: a field + * switch converted the ABSENT value, so a value-less row on a number column + * threw on `.trim()` and on a text column came back as the string + * `"undefined"`. + * + * The third case runs the other way — the builder is WIDER than the spec, so it + * does not keep an authored group out — and is left as it is here: the row + * keeps its boolean list member beside the spec's value, and narrowing it is a + * breaking change to a published type with a consumer in `@object-ui/plugin-list` + * (`UserFilters` hands `LookupValuePicker` a boolean-bearing list). The pin + * below states the width as it is, so the narrowing has to change it. * * Type-level pins are judged by `tsc -p tsconfig.test.json`; a green vitest run * says nothing about them. @@ -42,7 +50,7 @@ import { describe, it, expect, vi } from 'vitest'; import React from 'react'; import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom'; -import { ViewFilterRuleSchema, type ViewFilterRule } from '@objectstack/spec/ui'; +import type { ViewFilterRule } from '@objectstack/spec/ui'; import type { FilterField, FilterGroup } from '@object-ui/types'; import { FilterBuilder, @@ -58,8 +66,10 @@ type Assignable = [From] extends [To] ? true : false; /* ── The shapes ───────────────────────────────────────────────────────────── */ -// The row's value IS the spec rule's value — by reference, not a copy of it. -expectType>(); +// The row's value IS the spec rule's value — by reference, not a copy of it — +// plus the builder's own wider list member. +expectType>(); +expectType>(); // The renderer's two reads cross with no cast: the authored group and the // authored field list are what the builder's props take. expectType>>(); @@ -69,7 +79,6 @@ const FIELDS: NonNullable = [ { value: 'title', label: 'Title', type: 'text' }, { value: 'amount', label: 'Amount', type: 'number' }, { value: 'quota', label: 'Quota', type: 'currency' }, - { value: 'stage', label: 'Stage', type: 'select' }, ]; /** Render ONE authored row, typed as the authored group — no cast. */ @@ -87,7 +96,7 @@ function lastRow(onChange: ReturnType): FilterBuilderRow { return onChange.mock.calls.at(-1)?.[0].conditions[0]; } -/** Drive the row's REAL dropdown at `index` (0 = field, 1 = operator). */ +/** Drive the row's REAL dropdown at `index` (0 = field). */ async function pick(index: number, label: string) { fireEvent.keyDown(screen.getAllByRole('combobox')[index], { key: 'ArrowDown' }); const option = await waitFor(() => { @@ -120,15 +129,4 @@ describe('#26306 — the builder accepts the spec-derived row', () => { expect(row.value).toBeNull(); }); - it('boolean in a list: a boolean carried into `in` is emitted as the list the spec accepts', async () => { - const onChange = renderAuthored({ id: 'c1', field: 'stage', operator: 'equals', value: true }); - await pick(1, 'In'); - const row = lastRow(onChange); - expect(row.operator).toBe('in'); - expect(row.value).toEqual(['true']); - // The emitted row is one the spec's rule schema parses — `[true]` was not. - expect( - ViewFilterRuleSchema.safeParse({ field: row.field, operator: row.operator, value: row.value }).success, - ).toBe(true); - }); }); diff --git a/ui/packages/components/src/custom/filter-builder.tsx b/ui/packages/components/src/custom/filter-builder.tsx index 2c2bb48d057..208b54d36dd 100644 --- a/ui/packages/components/src/custom/filter-builder.tsx +++ b/ui/packages/components/src/custom/filter-builder.tsx @@ -72,10 +72,12 @@ import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover" * `narrowFilterValueToOptions`, …) is typed on this member, so `any` here * would switch their checking off in silence. OPTIONAL and admitting * `null`, as the spec's is: a value-less operator (`is_null`, `is_empty`, - * …) is authored with no `value`, so a required member refused the - * authored `FilterGroup` the `filter-builder` renderer hands over. A list - * or pair holds strings and numbers only, as the spec's does — a boolean - * carried into one is spelled as its string (`normalizeToArray`). + * …) is authored with no `value`, and a required member refused the + * authored `FilterGroup` the `filter-builder` renderer hands over. One + * member is the builder's own and WIDER than the spec's: a list entry may + * be a boolean, because a boolean scalar carried into a list operator + * (`reshapeFilterValue`, `normalizeToArray`) stays a boolean. The spec's + * list holds strings and numbers only. * * Its own NAME, not the authority's (objectui#6349, batch 12). Under the * authority's name this was a second declaration of `FilterBuilderCondition` @@ -88,7 +90,7 @@ import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover" export interface FilterBuilderRow extends Omit { operator: FilterBuilderOperator - value?: ViewFilterRule["value"] + value?: ViewFilterRule["value"] | (string | number | boolean)[] } /** @@ -866,25 +868,25 @@ export function retypeFilterValue( switch (filterValueArity(operator)) { case "list": { - const carried: (string | number)[] = [] + const carried: (string | number | boolean)[] = [] for (const entry of normalizeToArray(value)) { const converted = convertScalarToFamily(entry, family) - if (converted !== undefined && converted !== "") carried.push(toListEntry(converted)) + if (converted !== undefined && converted !== "") carried.push(converted) } return carried } case "pair": { const [min = "", max = ""] = normalizeToArray(value) - const lower = toListEntry(convertScalarToFamily(min, family) ?? "") - const upper = toListEntry(convertScalarToFamily(max, family) ?? "") + const lower = convertScalarToFamily(min, family) ?? "" + const upper = convertScalarToFamily(max, family) ?? "" return lower === "" && upper === "" ? [] : [lower, upper] } default: { const scalar = Array.isArray(value) ? (value[0] ?? "") : value // A row with no value — the spec's value-less operators are authored // without one — keeps having none. Converting the absence instead read - // `undefined` as a value: the text family spelled it `"undefined"` and - // the number family threw on `.trim()`. + // it as a value: the text family spelled `undefined` as the string + // `"undefined"`, and the number family threw on `.trim()`. if (isValueUnset(scalar)) return scalar return convertScalarToFamily(scalar, family) ?? "" } @@ -1112,7 +1114,7 @@ export function isFilterValueComplete( /** The two bounds a `pair` row edits, with the gaps filled in for rendering. */ function toPairBounds( value: FilterBuilderRow["value"], -): [string | number, string | number] { +): [string | number | boolean, string | number | boolean] { const [min = "", max = ""] = normalizeToArray(value) return [min, max] } @@ -1370,22 +1372,10 @@ function displayScalarValue(value: FilterBuilderRow["value"]): string { return isValueUnset(value) ? "" : String(value) } -/** - * One entry of a list or pair value, in the spec's element vocabulary: a - * `ViewFilterRule` list holds strings and numbers, never booleans. A boolean - * that reaches a list — an authored `equals true` row whose operator is then - * switched to `in` — is spelled as its string, which is also the key a static - * option compares on (`allowed.has(String(entry))`), so the entry the user - * chose stays the entry they see. - */ -function toListEntry(entry: string | number | boolean): string | number { - return typeof entry === "boolean" ? String(entry) : entry -} - /** Normalize a filter value into an array for multi-select scenarios */ -function normalizeToArray(value: FilterBuilderRow["value"]): (string | number)[] { +function normalizeToArray(value: FilterBuilderRow["value"]): (string | number | boolean)[] { if (Array.isArray(value)) return value - if (!isValueUnset(value)) return [toListEntry(value)] + if (!isValueUnset(value)) return [value] return [] } @@ -1835,7 +1825,7 @@ function FilterBuilder({ // refusal. `|| undefined` so a complete pair carries NO attribute at all. const boundInvalid = (index: 0 | 1) => (missingBound === index ? true : undefined) const setBound = (index: 0 | 1, raw: string) => { - const next: [string | number, string | number] = [bounds[0], bounds[1]] + const next: [string | number | boolean, string | number | boolean] = [bounds[0], bounds[1]] // The same one reading as the field-switch path (objectui#4875), in // place of `parseFloat(raw) || 0`: a bound is a clean number or it is // not filled in (`""`). A half-read bound (`"42abc"` → 42) or an @@ -1843,7 +1833,7 @@ function FilterBuilder({ // silently narrows a RANGE, and `0` is a plausible-looking bound. // `convertScalarToFamily("", "number")` is `""`, so the old // `raw !== ""` guard is subsumed rather than dropped. - next[index] = numeric ? toListEntry(convertScalarToFamily(raw, "number") ?? "") : raw + next[index] = numeric ? (convertScalarToFamily(raw, "number") ?? "") : raw // Both bounds cleared → back to the "nothing filled in yet" shape the // write path drops, rather than persisting an empty range. updateCondition(condition.id, { @@ -2305,13 +2295,13 @@ function TemporalValueEditor({ // ============================================================================ interface MultiValueInputProps { - values: (string | number)[] + values: (string | number | boolean)[] /** The `` the field's own type asks for (`text` / `number` / …). */ inputType: string /** Numeric field → committed tokens are numbers, so the wire carries numbers. */ numeric: boolean testId: string - onChange: (next: (string | number)[]) => void + onChange: (next: (string | number | boolean)[]) => void } /** @@ -2346,7 +2336,7 @@ function MultiValueInput({ values, inputType, numeric, testId, onChange }: Multi if (next === undefined || next === "") return setDraft("") if (values.some((v) => String(v) === String(next))) return - onChange([...values, toListEntry(next)]) + onChange([...values, next]) } const removeAt = (index: number) => { From ad8ecf130eeee93094ed0c9aac94ffc65b45276e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 21:31:28 +0000 Subject: [PATCH 4/5] chore(sdui): re-record the components package Claude-Session: https://claude.ai/code/session_01QGA6bzuch27y29tUbeb1y9 Co-authored-by: Claude --- scripts/sdui-manifest.sources/components.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/sdui-manifest.sources/components.json b/scripts/sdui-manifest.sources/components.json index f3006fdc5f9..abf8b2d57ee 100644 --- a/scripts/sdui-manifest.sources/components.json +++ b/scripts/sdui-manifest.sources/components.json @@ -1 +1 @@ -{"package":"ui/packages/components","sources":226,"sha256":"cd1d23767a76e6b4dcf2c0d4ca61b10ba27b336eeab990b22bfb0f7d10bb0424","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"} +{"package":"ui/packages/components","sources":226,"sha256":"75ed8c66878430e296c285bb0885304449f091b00da2209889cb99f634ef1690","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"} From 6a63ec936da68399a69a73d5641f520fd38f45fa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 21:51:30 +0000 Subject: [PATCH 5/5] chore(changeset): declare the row change a narrowing for readers of row.value Claude-Session: https://claude.ai/code/session_01QGA6bzuch27y29tUbeb1y9 Co-authored-by: Claude --- .changeset/26306-filter-builder-spec-row-value.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.changeset/26306-filter-builder-spec-row-value.md b/.changeset/26306-filter-builder-spec-row-value.md index f499559581e..c719cf03435 100644 --- a/.changeset/26306-filter-builder-spec-row-value.md +++ b/.changeset/26306-filter-builder-spec-row-value.md @@ -2,11 +2,15 @@ '@object-ui/components': patch --- -fix(components): the filter builder's row takes the spec rule's value, so the authored group a `filter-builder` node carries reaches the builder as it is +fix(components)!: the filter builder's row takes the spec rule's value, so the authored group a `filter-builder` node carries reaches the builder as it is -Clause-②: yes (widening) +Clause-②: yes (narrowing) Metadata-protocol: none + + +**BREAKING** type-surface change, compile-time only. `FilterBuilderRow` is exported from `@object-ui/components`. What the builder accepts widens, and what a reader of `row.value` may assume narrows: the member is now optional and admits `null`. Nothing changes at runtime for a reader: the builder already passed both through. + `FilterBuilderRow.value` is now `ViewFilterRule['value']` from `@objectstack/spec/ui`, taken by reference, plus the builder's own list member `(string | number | boolean)[]`. Before, it was the builder's own `string | number | boolean | (string | number | boolean)[]`, and it was required. Measured against the spec's rule, that refused two shapes an authored row takes: - **Value omitted.** The spec's value is optional, because a value-less operator (`is_null`, `is_empty`, …) carries none. The builder's was required, so the authored `FilterGroup` from `@object-ui/types` was not assignable to the builder's group. The member is optional now. @@ -18,4 +22,4 @@ Both cases also fix a field switch. Switching the field of a row with no value u The `filter-builder` renderer now hands the authored `value` and `fields` to the builder with no cast. It declares `value` on its props, because a host that binds the node forwards the group it holds. -Code that reads `row.value` from a `FilterBuilderRow` now sees `undefined` and `null` in its type. The builder already passed both through at runtime. +**Migration.** Code that reads `row.value` from a `FilterBuilderRow` (a host's `onChange` handler, for instance) handles `undefined` and `null` before using the value, or narrows first with an unset check (`value === undefined || value === null || value === ''`). Code that only writes rows needs no change.