Skip to content

Commit 0489214

Browse files
fix(components)!: the filter builder's row takes the spec rule's value, so an authored group reaches it without a cast (#26338)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9PNEMvJYp8jBTbfJYQEk5NM)_ Fixes #26306 Clause-②: yes (narrowing) Metadata-protocol: none **BREAKING** type-surface change, compile-time only. `FilterBuilderRow` is exported from `@object-ui/components` (`export * from './custom'`). What the builder accepts widens; what a reader of `row.value` (a host's `onChange` handler) may assume narrows, because the member is now optional and admits `null`. The changeset carries the reader's migration and the ADR-0087 disposition `not-required (runtime-interface-only ui/packages/components/src/custom/filter-builder.tsx#FilterBuilderRow)`. It ships `patch` (`Metadata-protocol: none`). ## What changed The `filter-builder` renderer hands `schema.value` (the authored `FilterGroup` from `@object-ui/types`) to `FilterBuilder`, which is typed on its own `FilterBuilderRowGroup`. The builder's row now takes the spec rule's value by reference, so the authored group is assignable to the builder's group and the renderer needs no cast. - **`FilterBuilderRow.value`** (`ui/packages/components/src/custom/filter-builder.tsx`): `ViewFilterRule['value']` from `@objectstack/spec/ui`, plus the builder's own list member `(string | number | boolean)[]`, and optional. It was `string | number | boolean | (string | number | boolean)[]`, required. - **`retypeFilterValue`**: a scalar with no value (`undefined`, `null`, `""`) is returned as it is instead of being converted. `isValueUnset` became a type predicate, which also removes the cast inside `normalizeToArray`. - **The renderer** (`src/renderers/complex/filter-builder.tsx`): declares `value?: FilterGroup` on its props, because a host that binds the node forwards the group it holds. Both casts go: `value` was never cast on `main` (the `any` index signature carried it), and `fields={(schema.fields || []) as any}` becomes `fields={schema.fields || []}` (`FilterField[]` was already assignable). - `docs/FilterBuilder.md` shows the new member. Changeset: `.changeset/26306-filter-builder-spec-row-value.md` (`patch`, BREAKING banner, migration for readers of `row.value`). ## Measurement: the spec's rule against both shapes Spec: `ViewFilterRuleSchema` in `packages/spec/src/ui/view.zod.ts`, read at `61ba04f68`. Types: `FilterBuilderCondition` / `FilterGroup` in `ui/packages/types/src/complex.ts`. Builder: `FilterBuilderRow` / `FilterBuilderRowGroup`. | case | spec `ViewFilterRule` | types `FilterBuilderCondition` | builder `FilterBuilderRow` on `main` | here | |---|---|---|---|---| | value omitted (value-less operator) | `value?` | `value?: any` | required | optional, pinned | | value `null` | admitted | admitted (`any`) | refused | admitted, pinned | | boolean entry in a list or pair | refused | admitted (`any`) | admitted | **unchanged** (see Acceptance notes) | | operator | `VIEW_FILTER_OPERATORS` | `ViewFilterOperator` by reference | those + opt-in `exists` / `notExists` | unchanged: every spec id is a builder id; the opt-in pair is the emit-side extension objectui#9559 ruling B keeps | | row `id` | none (a console decoration the write path strips) | required (objectui#8415) | taken from types | unchanged: types and builder agree | | group | none in the spec (a rule array) | `{ id?, logic, conditions }` | `conditions` restated only | unchanged | The spec row itself needs no change, so nothing here goes to `domain:spec`. The two value cases were not only type gaps. A field switch ran the absent value through `convertScalarToFamily`: a value-less row switched to a number column threw `TypeError: Cannot read properties of undefined (reading 'trim')` (`null` likewise), and on a text column it came back as the string `"undefined"`. ## Tests New file `src/__tests__/filter-builder-spec-row-26306.test.tsx`, one pin per case this PR changes, driven through the real field dropdown with an authored group typed `FilterGroup` (no cast): - value omitted: `{ field: 'title', operator: 'is_null' }` switched to a number column keeps no value. - value `null`: `{ field: 'amount', operator: 'equals', value: null }` draws a blank input and keeps `null` through a switch to a currency column. - Type pins (`tsc -p tsconfig.test.json`): `FilterBuilderRow['value']` equals the spec's value plus the list member; `FilterGroup` and `FilterField[]` are assignable to the builder's props. `filter-builder-name-authority-9306.test.tsx`: the value-extension pin now states the new member; the `@ts-expect-error` on a value-less row became a live fixture, and a new `@ts-expect-error` holds that an object value is still refused (the member is not `any`). ## Measured (head `6a63ec93`, `origin/main` `da99a4bd` merged in; the source under test is unchanged since `becb866e`) - **Renderer with #26293's signature, no cast.** #26293's `renderer-host-props.ts`, its `base.ts` line (`[key: string]: unknown`) and its renderer annotation (`[key: string]: unknown } & RendererHostProps`) applied locally on top of this branch, with no `as` in the renderer: `tsc --noEmit` in `@object-ui/components` exits 0. Control leg, same mutation over `main`'s builder: exit 2, `filter-builder.tsx(49,11): error TS2322: Type 'FilterGroup | undefined' is not assignable to type 'FilterBuilderRowGroup | undefined'`. Neither change is pushed. - **Reverse verification.** The new suite against `main`'s `custom/filter-builder.tsx` (byte-identical, hash checked; restored and checked against the HEAD blob): 2 of 2 red, `TypeError: Cannot read properties of undefined (reading 'trim')` and `… of null (reading 'trim')`. - **Tests.** `pnpm --dir ui exec vitest run --maxWorkers=2` over every `packages/components/src/__tests__/filter-builder-*` file, `inline-locale-label-read-sites`, `share-filter-sort-spec-parity`, `record-picker-malformed-filter-10789`, plugin-list's two `convertFilterGroupToAST` suites and app-shell's `userFilterUrlState.listState-11860`: Test Files 26 passed (26), Tests 505 passed (505). Narrowed to the row's readers; the full package suites are CI's `Test UI` legs. - **Type-check.** `pnpm --filter @object-ui/components --filter @object-ui/plugin-list --filter @object-ui/plugin-view --filter @object-ui/app-shell run type-check` (both programs each, after building their dependency closure): exit 0. These are the packages that import `FilterBuilderRow` / `FilterBuilderRowGroup`. - **Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 45 commands at `6a63ec93`; all 45 exit 0, and `--ran` reconciles 45 run, 0 NOT-MEASURED, 0 UNRUN. Among them `check-sdui-manifest.mjs`, `check-changeset-no-major.mjs --base origin/main` and `check-adr-0087-registration.mjs --base origin/main`, whose verdict line reads: `✓ check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.` (`[BREAKING+bang+clause-②-narrowing] not-required (runtime-interface-only) -- verified: ui/packages/components/src/custom/filter-builder.tsx#FilterBuilderRow (interface)`). - **ADR-0087 dispositions measured** (each committed on a throwaway commit at `3abe2733`, gate run, commit dropped): `runtime-interface-only` with the prose migration: exit 0. `runtime-interface-only` with a FROM / TO code block: exit 1, refused as a migration prescription (D7 inherits that refusal). `type-surface-only`, with either migration: exit 1, predicate 4 (`FilterBuilderRow` was already concrete at the merge base). So the migration for readers is prose, not a FROM / TO block. - **Dogfood** (owed for the `(narrowing)` arm), `6a63ec93`, `pnpm --filter @objectstack/dogfood test` under the lock after building its closure: Test Files 297 passed, 3 skipped (300); Tests 2528 passed, 18 skipped (2546); `VERDICT command-exit 0`. - **Console.** `pnpm exec turbo run build --filter=@object-ui/console --concurrency=2` at `becb866e`: 71 tasks successful, `VERDICT command-exit 0`. - **sdui.** `node scripts/gen-sdui-manifest-node.mjs` re-recorded `scripts/sdui-manifest.sources/components.json` only; `sdui.manifest.json` unchanged. #26293 (held) and #26274 also move that record, so whichever lands later merges `main` and re-records. ## Acceptance notes - **The boolean list member is kept.** The spec's list holds strings and numbers; the builder's admits booleans and can emit one: an authored `equals true` row on a select column, switched to `in`, comes back as `[true]`, which `ViewFilterRuleSchema` refuses. Narrowing the member to the spec's was built and measured on this branch first: it is a breaking change to a published type, and `@object-ui/plugin-list`'s `UserFilters` fails to compile against it (it hands `LookupValuePicker` a `(string | number | boolean)[]`). It needs a file outside this claim, and its writers (`UserFilters`) would need a FROM → TO rewrite, which no ADR-0087 category admits for a type that was already concrete at the base (`type-surface-only` fails predicate 4, `runtime-interface-only` refuses a prescription; both measured, see below). So it is reported for its own card rather than made here. The builder's doc comment and the new test name the width as it is. - `FilterBuilderSchema.onChange` is declared `(filter: FilterGroup) => void`, but the renderer calls the host's `onChange` with `{ target: { name, value } }`. Unchanged here; noted. - #26293 drops its `as any` on `value` once this lands; its renderer annotation then type-checks as measured above. --- _Generated by [Claude Code](https://claude.ai/code/session_01QGA6bzuch27y29tUbeb1y9)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent af13616 commit 0489214

7 files changed

Lines changed: 211 additions & 22 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@object-ui/components': patch
3+
---
4+
5+
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
6+
7+
Clause-②: yes (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- 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. -->
11+
12+
**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.
13+
14+
`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:
15+
16+
- **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.
17+
- **Value `null`.** The spec admits it; the builder's member did not. It does now.
18+
19+
The operator vocabulary needed no change: every spec operator id is a builder id.
20+
21+
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.
22+
23+
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.
24+
25+
**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.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"package":"ui/packages/components","sources":226,"sha256":"cd1d23767a76e6b4dcf2c0d4ca61b10ba27b336eeab990b22bfb0f7d10bb0424","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"}
1+
{"package":"ui/packages/components","sources":226,"sha256":"75ed8c66878430e296c285bb0885304449f091b00da2209889cb99f634ef1690","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"}

‎ui/packages/components/docs/FilterBuilder.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ interface Field {
122122
The schema's `value` is `FilterGroup` from `@object-ui/types`, the authored group.
123123
The React component's `value` / `onChange` use `FilterBuilderRowGroup` from
124124
`@object-ui/components`: the same group, derived from `FilterGroup`, whose rows
125-
are `FilterBuilderRow` (the builder's own operator ids and a required value).
125+
are `FilterBuilderRow` (the builder's own operator ids and the spec rule's value),
126+
so an authored group passes to the component as it is.
126127

127128
```typescript
128129
interface FilterBuilderRowGroup {
@@ -135,7 +136,9 @@ interface FilterBuilderRow {
135136
id: string; // Row identifier (required)
136137
field: string; // Field value
137138
operator: FilterBuilderOperator; // Operator id (see below)
138-
value: string | number | boolean | (string | number | boolean)[]; // Filter value
139+
value?: ViewFilterRule['value'] | (string | number | boolean)[];
140+
// `ViewFilterRule` from `@objectstack/spec/ui`: optional (value-less operators
141+
// carry none) and may be null; the builder's own list may also hold booleans
139142
}
140143
```
141144

‎ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*
2121
* - `operator` — `FilterBuilderOperator`, the protocol's ids plus the opt-in
2222
* `exists` / `notExists` (objectui#9559 ruling B);
23-
* - `value` — required and narrowed to what this builder edits.
23+
* - `value` — the spec rule's value, by reference (#26306).
2424
*
2525
* Everything else comes from the authority, and the group restates only
2626
* `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';
4141
import React from 'react';
4242
import { render, screen, fireEvent } from '@testing-library/react';
4343
import '@testing-library/jest-dom';
44-
import type { ViewFilterOperator } from '@objectstack/spec/ui';
44+
import type { ViewFilterOperator, ViewFilterRule } from '@objectstack/spec/ui';
4545
import type {
4646
FilterBuilderCondition as AuthorityCondition,
4747
FilterGroup as AuthorityGroup,
@@ -97,12 +97,23 @@ const offVocabulary: FilterBuilderRow = {
9797
value: 1,
9898
};
9999

100-
// The value extension: the builder's own scalar-or-list union, REQUIRED —
101-
// ruled over the authority's `value?: any`, which would switch the typing of
102-
// every exported value helper off in silence.
103-
expectType<Equal<FilterBuilderRow['value'], string | number | boolean | (string | number | boolean)[]>>();
104-
// @ts-expect-error `value` is required on the builder's row
105-
const rowWithoutValue: FilterBuilderRow = { id: 'c1', field: 'a', operator: 'equals' };
100+
// The value extension: the SPEC rule's value, by reference (#26306), plus the
101+
// builder's own boolean-bearing list — never the
102+
// authority's `value?: any`, which would switch the typing of every exported
103+
// value helper off in silence. Optional, as the spec's is: a value-less
104+
// operator is authored with no value, and a required member refused the
105+
// authored group the renderer hands over. The per-case pins live in
106+
// `filter-builder-spec-row-26306.test.tsx`.
107+
expectType<Equal<FilterBuilderRow['value'], ViewFilterRule['value'] | (string | number | boolean)[]>>();
108+
const rowWithoutValue: FilterBuilderRow = { id: 'c1', field: 'a', operator: 'is_null' };
109+
// …and still not `any`: a value the spec refuses is refused here.
110+
const objectValue: FilterBuilderRow = {
111+
id: 'c1',
112+
field: 'a',
113+
operator: 'equals',
114+
// @ts-expect-error `value` is the spec rule's value, not an object
115+
value: { $gt: 1 },
116+
};
106117

107118
/* ── The group: only `conditions` is restated ─────────────────────────────── */
108119

@@ -123,7 +134,8 @@ describe('objectui#9306 — the builder derives its row and group from @object-u
123134
it('the type-level pins above compile (see `tsc -p tsconfig.test.json`); the fixtures are live values', () => {
124135
expect(rowWithoutId.field).toBe('a');
125136
expect(offVocabulary.id).toBe('c1');
126-
expect(rowWithoutValue.operator).toBe('equals');
137+
expect(rowWithoutValue.operator).toBe('is_null');
138+
expect(objectValue.id).toBe('c1');
127139
expect(groupWithoutId.conditions).toEqual([]);
128140
expect(nested.logic).toBe('and');
129141
});
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* The builder accepts the spec-derived group the `filter-builder` renderer
11+
* hands it (#26306).
12+
*
13+
* The renderer passes `schema.value` — the authored `FilterGroup` from
14+
* `@object-ui/types`, whose row takes its operator from the spec's
15+
* `ViewFilterOperator` — to a builder typed on its own `FilterBuilderRowGroup`.
16+
* The two were assignable in neither direction, and the renderer props' `any`
17+
* index signature hid that. Measured against the spec's `ViewFilterRule`
18+
* (`@objectstack/spec/ui`), the row diverged in three VALUE cases and in no
19+
* operator case a consumer can author:
20+
*
21+
* | case | spec rule | builder row (before) |
22+
* |--------------------------------|------------------------|---------------------------------|
23+
* | value omitted (unary operator) | `value?` | `value` required |
24+
* | value `null` | admitted | refused |
25+
* | boolean entry in a list / pair | refused | admitted (kept, see below) |
26+
* | operator | `VIEW_FILTER_OPERATORS` | those + opt-in `exists` / `notExists` |
27+
*
28+
* The operator row is not a divergence on the way IN: every spec id is a
29+
* builder id. The opt-in pair is the builder's emit-side extension (objectui#9559
30+
* ruling B) and is pinned in `filter-builder-name-authority-9306.test.tsx`.
31+
*
32+
* The builder's row now takes the spec's value by reference
33+
* (`ViewFilterRule["value"]`), so the two cases that kept an authored group out
34+
* of the builder have one pin each below. Neither was only a type gap: a field
35+
* switch converted the ABSENT value, so a value-less row on a number column
36+
* threw on `.trim()` and on a text column came back as the string
37+
* `"undefined"`.
38+
*
39+
* The third case runs the other way — the builder is WIDER than the spec, so it
40+
* does not keep an authored group out — and is left as it is here: the row
41+
* keeps its boolean list member beside the spec's value, and narrowing it is a
42+
* breaking change to a published type with a consumer in `@object-ui/plugin-list`
43+
* (`UserFilters` hands `LookupValuePicker` a boolean-bearing list). The pin
44+
* below states the width as it is, so the narrowing has to change it.
45+
*
46+
* Type-level pins are judged by `tsc -p tsconfig.test.json`; a green vitest run
47+
* says nothing about them.
48+
*/
49+
import { describe, it, expect, vi } from 'vitest';
50+
import React from 'react';
51+
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
52+
import '@testing-library/jest-dom';
53+
import type { ViewFilterRule } from '@objectstack/spec/ui';
54+
import type { FilterField, FilterGroup } from '@object-ui/types';
55+
import {
56+
FilterBuilder,
57+
retypeFilterValue,
58+
type FilterBuilderProps,
59+
type FilterBuilderRow,
60+
} from '../custom/filter-builder';
61+
62+
type Equal<A, B> =
63+
(<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
64+
function expectType<T extends true>(_: T = true as T): void { /* compile-time only */ }
65+
type Assignable<From, To> = [From] extends [To] ? true : false;
66+
67+
/* ── The shapes ───────────────────────────────────────────────────────────── */
68+
69+
// The row's value IS the spec rule's value — by reference, not a copy of it —
70+
// plus the builder's own wider list member.
71+
expectType<Equal<FilterBuilderRow['value'], ViewFilterRule['value'] | (string | number | boolean)[]>>();
72+
expectType<Assignable<ViewFilterRule['value'], FilterBuilderRow['value']>>();
73+
// The renderer's two reads cross with no cast: the authored group and the
74+
// authored field list are what the builder's props take.
75+
expectType<Assignable<FilterGroup, NonNullable<FilterBuilderProps['value']>>>();
76+
expectType<Assignable<FilterField[], NonNullable<FilterBuilderProps['fields']>>>();
77+
78+
const FIELDS: NonNullable<FilterBuilderProps['fields']> = [
79+
{ value: 'title', label: 'Title', type: 'text' },
80+
{ value: 'amount', label: 'Amount', type: 'number' },
81+
{ value: 'quota', label: 'Quota', type: 'currency' },
82+
];
83+
84+
/** Render ONE authored row, typed as the authored group — no cast. */
85+
function renderAuthored(row: FilterGroup['conditions'][number]) {
86+
const onChange = vi.fn();
87+
// Hoisted out of the JSX: the sync effect re-seeds state when the `value`
88+
// prop's identity changes, which would undo the interaction under test.
89+
const value: FilterGroup = { logic: 'and', conditions: [row] };
90+
render(<FilterBuilder fields={FIELDS} value={value} onChange={onChange} />);
91+
return onChange;
92+
}
93+
94+
function lastRow(onChange: ReturnType<typeof vi.fn>): FilterBuilderRow {
95+
expect(onChange.mock.calls.length, 'the builder never called onChange').toBeGreaterThan(0);
96+
return onChange.mock.calls.at(-1)?.[0].conditions[0];
97+
}
98+
99+
/** Drive the row's REAL dropdown at `index` (0 = field). */
100+
async function pick(index: number, label: string) {
101+
fireEvent.keyDown(screen.getAllByRole('combobox')[index], { key: 'ArrowDown' });
102+
const option = await waitFor(() => {
103+
const found = screen.getAllByRole('option').find((o) => o.textContent === label);
104+
expect(found, `no "${label}" option in dropdown ${index}`).toBeTruthy();
105+
return found!;
106+
});
107+
fireEvent.click(option);
108+
}
109+
110+
describe('#26306 — the builder accepts the spec-derived row', () => {
111+
it('value omitted: a value-less row keeps no value through a field switch (it used to throw on a number column)', async () => {
112+
const onChange = renderAuthored({ id: 'c1', field: 'title', operator: 'is_null' });
113+
await pick(0, 'Amount');
114+
const row = lastRow(onChange);
115+
expect(row.field).toBe('amount');
116+
expect(row.operator).toBe('is_null');
117+
expect(row.value).toBeUndefined();
118+
// The text family, where the absence used to come back spelled out.
119+
expect(retypeFilterValue(undefined, 'text', 'is_null')).toBeUndefined();
120+
});
121+
122+
it('value null: a null row keeps null through a field switch (it used to throw on `.trim()`)', async () => {
123+
const onChange = renderAuthored({ id: 'c1', field: 'amount', operator: 'equals', value: null });
124+
// Drawn as unfilled, not as the string "null".
125+
expect((document.querySelector('input[placeholder="Value"]') as HTMLInputElement).value).toBe('');
126+
await pick(0, 'Quota');
127+
const row = lastRow(onChange);
128+
expect(row.field).toBe('quota');
129+
expect(row.value).toBeNull();
130+
});
131+
132+
});

‎ui/packages/components/src/custom/filter-builder.tsx‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
VIEW_FILTER_LIST_VALUE_OPERATORS,
1515
VIEW_FILTER_PAIR_VALUE_OPERATORS,
1616
normalizeFilterOperator,
17+
type ViewFilterRule,
1718
} from "@objectstack/spec/ui"
1819
import { expandEmptyOperator } from "@objectstack/spec/data"
1920
import { SchemaRendererContext } from "@object-ui/react"
@@ -64,11 +65,19 @@ import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover"
6465
* protocol has no member for (objectui#9559 ruling B keeps them opt-in and
6566
* unfolded). It was `string`, which let a caller hand the builder an id no
6667
* dropdown entry can hold.
67-
* - `value` — REQUIRED and narrowed to the scalar-or-list union this builder
68-
* edits, where the authority declares `value?: any`. Every exported helper
69-
* below that takes a row's value (`reshapeFilterValue`, `retypeFilterValue`,
68+
* - `value` — the SPEC's rule value, taken BY REFERENCE
69+
* (`ViewFilterRule["value"]`, `@objectstack/spec/ui`), where the authority
70+
* declares `value?: any`. Every exported helper below that takes a row's
71+
* value (`reshapeFilterValue`, `retypeFilterValue`,
7072
* `narrowFilterValueToOptions`, …) is typed on this member, so `any` here
71-
* would switch their checking off in silence.
73+
* would switch their checking off in silence. OPTIONAL and admitting
74+
* `null`, as the spec's is: a value-less operator (`is_null`, `is_empty`,
75+
* …) is authored with no `value`, and a required member refused the
76+
* authored `FilterGroup` the `filter-builder` renderer hands over. One
77+
* member is the builder's own and WIDER than the spec's: a list entry may
78+
* be a boolean, because a boolean scalar carried into a list operator
79+
* (`reshapeFilterValue`, `normalizeToArray`) stays a boolean. The spec's
80+
* list holds strings and numbers only.
7281
*
7382
* Its own NAME, not the authority's (objectui#6349, batch 12). Under the
7483
* authority's name this was a second declaration of `FilterBuilderCondition`
@@ -81,7 +90,7 @@ import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover"
8190
export interface FilterBuilderRow
8291
extends Omit<AuthoredFilterBuilderCondition, "operator" | "value"> {
8392
operator: FilterBuilderOperator
84-
value: string | number | boolean | (string | number | boolean)[]
93+
value?: ViewFilterRule["value"] | (string | number | boolean)[]
8594
}
8695

8796
/**
@@ -874,6 +883,11 @@ export function retypeFilterValue(
874883
}
875884
default: {
876885
const scalar = Array.isArray(value) ? (value[0] ?? "") : value
886+
// A row with no value — the spec's value-less operators are authored
887+
// without one — keeps having none. Converting the absence instead read
888+
// it as a value: the text family spelled `undefined` as the string
889+
// `"undefined"`, and the number family threw on `.trim()`.
890+
if (isValueUnset(scalar)) return scalar
877891
return convertScalarToFamily(scalar, family) ?? ""
878892
}
879893
}
@@ -1340,7 +1354,7 @@ function offeredOperatorsForRow(
13401354
*/
13411355
function isValueUnset(
13421356
value: FilterBuilderRow["value"] | undefined | null,
1343-
): boolean {
1357+
): value is undefined | null | "" {
13441358
return value === undefined || value === null || value === ""
13451359
}
13461360

@@ -1361,7 +1375,7 @@ function displayScalarValue(value: FilterBuilderRow["value"]): string {
13611375
/** Normalize a filter value into an array for multi-select scenarios */
13621376
function normalizeToArray(value: FilterBuilderRow["value"]): (string | number | boolean)[] {
13631377
if (Array.isArray(value)) return value
1364-
if (!isValueUnset(value)) return [value as string | number | boolean]
1378+
if (!isValueUnset(value)) return [value]
13651379
return []
13661380
}
13671381

‎ui/packages/components/src/renderers/complex/filter-builder.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { ComponentRegistry } from '@object-ui/core';
10-
import type { FilterBuilderSchema } from '@object-ui/types';
10+
import type { FilterBuilderSchema, FilterGroup } from '@object-ui/types';
1111
import { useDisplayLocale } from '@object-ui/i18n';
1212
// Aliased on import, following PR #4169's convention (and `AppSchemaRenderer`'s
1313
// use of it): this repo has its OWN `resolveKeyedI18nLabel` over a DIFFERENT
@@ -17,7 +17,10 @@ import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui
1717
import { FilterBuilder } from '../../custom/filter-builder';
1818

1919
ComponentRegistry.register('filter-builder',
20-
({ schema, className, onChange, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: (event: any) => void; [key: string]: any }) => {
20+
// `value` is declared because it is read by name: a host that binds the node
21+
// (a form field) forwards the group it holds, the same authored `FilterGroup`
22+
// the node's own `value` is.
23+
({ schema, className, onChange, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: (event: any) => void; value?: FilterGroup; [key: string]: any }) => {
2124
// Read-time resolution against the display locale (objectui#4580 revised
2225
// Q1-A). `BaseSchema.label` accepts `string | I18nLabel`; rendering the map
2326
// straight into a text node THREW "Objects are not valid as a React child".
@@ -41,7 +44,7 @@ ComponentRegistry.register('filter-builder',
4144
</label>
4245
)}
4346
<FilterBuilder
44-
fields={(schema.fields || []) as any}
47+
fields={schema.fields || []}
4548
value={schema.value || props.value}
4649
onChange={handleChange}
4750
className={className}

0 commit comments

Comments
 (0)