fix(components)!: the filter builder's row takes the spec rule's value, so an authored group reaches it without a cast

.changeset/26306-filter-builder-spec-row-value.md

@@ -0,0 +1,25 @@
+---
+'@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
+
+Clause-②: yes (narrowing)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (runtime-interface-only ui/packages/components/src/custom/filter-builder.tsx#FilterBuilderRow) A published runtime TypeScript interface of a React component with no metadata surface: no Zod schema, no spec declaration and no stored representation, so `objectstack migrate meta` has nothing to rewrite; the affected party is a TypeScript consumer, reached by its own compiler. -->
+
+**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.
+- **Value `null`.** The spec admits it; the builder's member did not. It does now.
+
+The operator vocabulary needed no change: every spec operator id is a builder id.
+
+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 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` (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.

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"}

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'] | (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
 }
 ```
 

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,23 @@ 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<Equal<FilterBuilderRow['value'], string | number | boolean | (string | number | boolean)[]>>();
-// @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), 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<Equal<FilterBuilderRow['value'], ViewFilterRule['value'] | (string | number | boolean)[]>>();
+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 +134,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');
   });

ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx

@@ -0,0 +1,132 @@
+/**
+ * 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 (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
+ * 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 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.
+ */
+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 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<A, B> =
+  (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
+function expectType<T extends true>(_: T = true as T): void { /* compile-time only */ }
+type Assignable<From, To> = [From] extends [To] ? true : false;
+
+/* ── The shapes ───────────────────────────────────────────────────────────── */
+
+// 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<Equal<FilterBuilderRow['value'], ViewFilterRule['value'] | (string | number | boolean)[]>>();
+expectType<Assignable<ViewFilterRule['value'], FilterBuilderRow['value']>>();
+// 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<Assignable<FilterGroup, NonNullable<FilterBuilderProps['value']>>>();
+expectType<Assignable<FilterField[], NonNullable<FilterBuilderProps['fields']>>>();
+
+const FIELDS: NonNullable<FilterBuilderProps['fields']> = [
+  { value: 'title', label: 'Title', type: 'text' },
+  { value: 'amount', label: 'Amount', type: 'number' },
+  { value: 'quota', label: 'Quota', type: 'currency' },
+];
+
+/** 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(<FilterBuilder fields={FIELDS} value={value} onChange={onChange} />);
+  return onChange;
+}
+
+function lastRow(onChange: ReturnType<typeof vi.fn>): 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). */
+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();
+  });
+
+});

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,19 @@ 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`, 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`
@@ -81,7 +90,7 @@ import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover"
 export interface FilterBuilderRow
   extends Omit<AuthoredFilterBuilderCondition, "operator" | "value"> {
   operator: FilterBuilderOperator
-  value: string | number | boolean | (string | number | boolean)[]
+  value?: ViewFilterRule["value"] | (string | number | boolean)[]
 }
 
 /**
@@ -874,6 +883,11 @@ export function retypeFilterValue(
     }
     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
+      // 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) ?? ""
     }
   }
@@ -1340,7 +1354,7 @@ function offeredOperatorsForRow(
  */
 function isValueUnset(
   value: FilterBuilderRow["value"] | undefined | null,
-): boolean {
+): value is undefined | null | "" {
   return value === undefined || value === null || value === ""
 }
 
@@ -1361,7 +1375,7 @@ function displayScalarValue(value: FilterBuilderRow["value"]): string {
 /** Normalize a filter value into an array for multi-select scenarios */
 function normalizeToArray(value: FilterBuilderRow["value"]): (string | number | boolean)[] {
   if (Array.isArray(value)) return value
-  if (!isValueUnset(value)) return [value as string | number | boolean]
+  if (!isValueUnset(value)) return [value]
   return []
 }
 

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',
           </label>
         )}
         <FilterBuilder
-          fields={(schema.fields || []) as any}
+          fields={schema.fields || []}
           value={schema.value || props.value}
           onChange={handleChange}
           className={className}