From d06f4e81378179e8baef5f672044a5e431e30244 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 08:33:35 +0000 Subject: [PATCH] fix(fields): a gated option list keeps the field's stored value on mount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four fixed-option widgets and the form renderer each end their cascade resolution with a "drop what is no longer offered" clear. Both read `resolveCascadingOptions`, which returns an EMPTY offered set whenever the list is gated (a declared `dependsOn` parent is still empty), so nothing the field held could be "still offered" and both paths wrote the field empty on MOUNT, with no interaction — while the control rendered its "select the parent first" hint beside it. Gated means UNKNOWN, not invalid. ADR-0058's cascade prunes on a USER-DRIVEN parent change; a withheld list on mount is missing information (a later-cleared parent, an import, a partially-migrated row), and that is not a reason to destroy stored data. Both clears now skip while gated, reading the resolver's own `gated` flag rather than re-deriving it from an empty offered set — which would collide with the distinct never-configured case guarded by #4220. Convergence is unchanged: once the parent IS chosen and the resolved set genuinely excludes the stored value the prune applies, including at the moment the gate lifts. The three states (never-configured / gated / resolved-and-excludes) are pinned apart across all four widgets and the form host. Fixes #4247 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../gated-options-keep-stored-value-4247.md | 14 + .../form-gated-options-keep-value.test.tsx | 142 ++++++++++ .../components/src/renderers/form/form.tsx | 20 +- .../fields/src/widgets/CheckboxesField.tsx | 5 + .../fields/src/widgets/MultiSelectField.tsx | 35 ++- packages/fields/src/widgets/RadioField.tsx | 5 + packages/fields/src/widgets/SelectField.tsx | 5 + .../optionWidgets.gatedOptions.test.tsx | 267 ++++++++++++++++++ 8 files changed, 480 insertions(+), 13 deletions(-) create mode 100644 .changeset/gated-options-keep-stored-value-4247.md create mode 100644 packages/components/src/renderers/form/__tests__/form-gated-options-keep-value.test.tsx create mode 100644 packages/fields/src/widgets/optionWidgets.gatedOptions.test.tsx diff --git a/.changeset/gated-options-keep-stored-value-4247.md b/.changeset/gated-options-keep-stored-value-4247.md new file mode 100644 index 000000000..b8b62852f --- /dev/null +++ b/.changeset/gated-options-keep-stored-value-4247.md @@ -0,0 +1,14 @@ +--- +'@object-ui/fields': patch +'@object-ui/components': patch +--- + +A dependency-gated option list no longer deletes the field's stored value on mount + +The four fixed-option widgets (`SelectField`, `MultiSelectField`, `CheckboxesField`, `RadioField`) end their cascade resolution with a "drop what is no longer offered" effect, and the form renderer runs an equivalent clear of its own over every option field. Both read `resolveCascadingOptions`, which returns an **empty** offered set whenever the list is *gated* — a declared `dependsOn` parent is still empty. Nothing the field held could be "still offered" against an empty set, so both paths wrote the field empty **on mount, with no interaction**, while the control rendered "Select Country first" beside it: it told the user it could not offer anything, and deleted what they had. + +Gated means **unknown**, not invalid. The cascade clear exists (ADR-0058) so a user-driven parent change prunes a now-invalid child; a withheld list on mount is missing information — the record simply arrived with its controlling field empty (a later-cleared parent, an import, a partially-migrated row) — and that is not a reason to destroy stored data. Both clears now skip while gated, reading the resolver's own `gated` flag rather than re-deriving it from an empty offered set, which would collide with the distinct never-configured case guarded separately in objectui#4220. + +Convergence stays exactly where it belongs: once the parent **is** chosen and the resolved set genuinely excludes the stored value, the prune applies unchanged — including at the moment the gate lifts, so picking a parent whose list does not contain the old value still clears it on that transition. The three states are pinned apart (never-configured / gated / resolved-and-excludes) across all four widgets and the form host, so a future edit cannot collapse them back into one empty-set test. + +Reachable on every host that mounts these widgets with a live record: the form renderer, the grid's inline cell editor, and the detail page's inline editor — where each `onChange` went straight into the record draft the save bar commits. diff --git a/packages/components/src/renderers/form/__tests__/form-gated-options-keep-value.test.tsx b/packages/components/src/renderers/form/__tests__/form-gated-options-keep-value.test.tsx new file mode 100644 index 000000000..5c86aa12e --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/form-gated-options-keep-value.test.tsx @@ -0,0 +1,142 @@ +/** + * 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. + */ + +/** + * A GATED option list never deletes the form's stored value (objectui#4247). + * + * The form renderer runs its OWN cascade clear (#2284) alongside the widgets': + * for every option field it resolves the offered set and `form.setValue(name, + * undefined)` when the current value is no longer offered. `resolveCascadingOptions` + * returns an EMPTY set whenever the list is gated — a declared `dependsOn` + * parent is still empty — so a record that simply ARRIVES with the parent empty + * (a later-cleared parent, an import, a partially-migrated row) had its + * dependent picklist wiped on mount, before any interaction, while the field + * rendered "select Country first" beside it. + * + * Ruling (#4247): **gated means UNKNOWN, not invalid.** Missing information is + * not a reason to destroy stored data. Convergence stays where ADR-0058 put it: + * once the parent IS chosen and the resolved set genuinely excludes the value, + * the prune applies — pinned as the control below. + * + * This is the FORM host of the same defect pinned per-widget in + * `@object-ui/fields` (`optionWidgets.gatedOptions.test.tsx`). The two clears + * are independent code paths reading the same resolver, so both are pinned: + * these components tests never load the fields package, so nothing but + * `form.tsx`'s own effect can move the value here. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +// Module-scope import (not `beforeAll`) — objectui#3010. +import '../../../renderers'; + +function renderForm(schema: Record) { + const Form = ComponentRegistry.get('form')!; + return render(
); +} + +const PROVINCE_OPTIONS = [ + { label: 'Zhejiang', value: 'zj', visibleWhen: "record.country == 'cn'" }, + { label: 'California', value: 'ca', visibleWhen: "record.country == 'us'" }, +]; + +const FIELDS = [ + { name: 'country', label: 'Country', type: 'input' }, + { + name: 'province', + label: 'Province', + type: 'select', + dependsOn: 'country', + options: PROVINCE_OPTIONS, + }, + { + name: 'provinces', + label: 'Provinces', + type: 'multiselect', + dependsOn: 'country', + options: PROVINCE_OPTIONS, + }, +]; + +async function submitAndRead(onSubmit: ReturnType) { + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + return onSubmit.mock.calls[0][0] as Record; +} + +describe('form renderer — a GATED option list keeps the stored value (#4247)', () => { + it('submits the record it was given when the controlling field arrives empty', async () => { + const onSubmit = vi.fn(); + renderForm({ + fields: FIELDS, + defaultValues: { country: '', province: 'zj', provinces: ['zj'] }, + onSubmit, + }); + + // The gate is up: the form is telling the user it cannot offer anything. + expect(screen.getAllByText(/select country first/i).length).toBeGreaterThan(0); + + const payload = await submitAndRead(onSubmit); + expect(payload.province).toBe('zj'); + expect(payload.provinces).toEqual(['zj']); + }); +}); + +describe('control — a RESOLVED list still prunes what it does not offer (ADR-0058)', () => { + it('clears the values the chosen parent excludes', async () => { + const onSubmit = vi.fn(); + renderForm({ + fields: FIELDS, + // Parent IS chosen, and `ca` is a US province — genuinely excluded. + defaultValues: { country: 'cn', province: 'ca', provinces: ['ca'] }, + onSubmit, + }); + + const payload = await submitAndRead(onSubmit); + expect(payload.province).toBeUndefined(); + expect(payload.provinces).toEqual([]); + }); + + it('keeps the values the chosen parent still offers', async () => { + const onSubmit = vi.fn(); + renderForm({ + fields: FIELDS, + defaultValues: { country: 'cn', province: 'zj', provinces: ['zj'] }, + onSubmit, + }); + + const payload = await submitAndRead(onSubmit); + expect(payload.province).toBe('zj'); + expect(payload.provinces).toEqual(['zj']); + }); +}); + +describe('transition — the gate lifting is what converges the value (#4247)', () => { + it('prunes only once the user picks a parent that excludes the stored value', async () => { + const onSubmit = vi.fn(); + renderForm({ + fields: FIELDS, + defaultValues: { country: '', province: 'zj', provinces: ['zj'] }, + onSubmit, + }); + + // Gated on mount — nothing staged yet. + expect(screen.getAllByText(/select country first/i).length).toBeGreaterThan(0); + + // The user picks a country whose list does NOT contain the stored value. + fireEvent.change(screen.getByLabelText(/country/i), { target: { value: 'us' } }); + await waitFor(() => + expect(screen.queryAllByText(/select country first/i)).toHaveLength(0), + ); + + const payload = await submitAndRead(onSubmit); + expect(payload.province).toBeUndefined(); + expect(payload.provinces).toEqual([]); + }); +}); diff --git a/packages/components/src/renderers/form/form.tsx b/packages/components/src/renderers/form/form.tsx index 09e859c10..1f00b8469 100644 --- a/packages/components/src/renderers/form/form.tsx +++ b/packages/components/src/renderers/form/form.tsx @@ -920,10 +920,22 @@ ComponentRegistry.register('form', if (!hasOptionPredicate && !dependsOn) continue; const current = form.getValues(name); if (current === undefined || current === null || current === '') continue; - // While gated (a dependency is empty) the whole list is withheld — clear - // any prior value so it can't linger past a parent reset. Same shared - // resolver the widgets use, so gating/filtering stays in lockstep. - const { options: visible } = resolveCascadingOptions(opts, ruleRecord, dependsOn, predicateScope); + // Same shared resolver the widgets use, so gating/filtering stays in + // lockstep — including the guard below. + const { options: visible, gated } = resolveCascadingOptions(opts, ruleRecord, dependsOn, predicateScope); + // Gated ≠ invalid (objectui#4247). While a dependency is empty the whole + // list is WITHHELD, so `visible` is empty for a reason that says nothing + // about the stored value — and this effect runs on mount, so a record + // that merely ARRIVES with its controlling field empty (a later-cleared + // parent, an import, a partially-migrated row) had the dependent + // picklist wiped before the user touched anything, while the field + // rendered "select the parent first" beside it. Gating is missing + // information, not a verdict; the cascade converges when the parent IS + // chosen and the resolved set genuinely excludes the value, which is the + // clear below, unchanged. The four option WIDGETS carry the matching + // guard — this effect is an independent second clear on the form host, + // and the widgets' fix does not reach it. + if (gated) continue; if (!isValueStillOffered(current, visible)) { form.setValue(name, Array.isArray(current) ? [] : undefined, { shouldValidate: false, diff --git a/packages/fields/src/widgets/CheckboxesField.tsx b/packages/fields/src/widgets/CheckboxesField.tsx index dd30b5bfc..c954f88a9 100644 --- a/packages/fields/src/widgets/CheckboxesField.tsx +++ b/packages/fields/src/widgets/CheckboxesField.tsx @@ -55,6 +55,11 @@ export function CheckboxesField({ // Never configured → nothing to prune against; see `MultiSelectField`'s // copy of this guard for the measured failure (objectui#4220). if (rawOptions.length === 0) return; + // Gated → the authored list is withheld until the `dependsOn` parent is + // chosen, so the empty offered set is missing information, not a verdict on + // the stored value; clearing here fired on mount (objectui#4247). Same + // reasoning, at length, in `MultiSelectField`. + if (gated) return; if (selected.length === 0) return; const stillOffered = selected.filter((v) => options.some((o) => o.value === v)); if (stillOffered.length !== selected.length) onChange(stillOffered); diff --git a/packages/fields/src/widgets/MultiSelectField.tsx b/packages/fields/src/widgets/MultiSelectField.tsx index 1e7a91a94..ce914bedc 100644 --- a/packages/fields/src/widgets/MultiSelectField.tsx +++ b/packages/fields/src/widgets/MultiSelectField.tsx @@ -53,16 +53,33 @@ export function MultiSelectField({ // the scalar case we prune per-element rather than clearing the whole field. useEffect(() => { if (readonly) return; - // Nothing was ever CONFIGURED to prune against (objectui#4220). An empty - // offered set has two very different causes: a list that cascaded down to - // zero (a real decision — clear), and a field authored with no `options` at - // all (no decision — the widget renders its "unfillable" state below). In - // the second case pruning is not a cascade, it is deleting the stored value - // of a field the user was only ever shown a hint for — measured on the - // detail page's inline editor, which stages that empty array into the - // record draft the moment the row enters edit mode, and on the grid's - // inline cell editor, which has always taken this path. + // An empty offered set has THREE very different causes, and only one of them + // is a decision to prune against. This is the canonical copy of the guards; + // the other three option widgets carry the same pair. + // + // 1. Nothing was ever CONFIGURED (objectui#4220) — a field authored with no + // `options` at all. No decision was made anywhere, and the widget renders + // its "unfillable" state below. Pruning here is not a cascade, it is + // deleting the stored value of a field the user was only ever shown a + // hint for — measured on the detail page's inline editor, which stages + // that empty array into the record draft the moment the row enters edit + // mode, and on the grid's inline cell editor, which has always taken this + // path. if (rawOptions.length === 0) return; + // 2. The list is GATED (objectui#4247) — an authored list withheld because a + // declared `dependsOn` parent is still empty, so `resolveCascadingOptions` + // returns nothing at all. Gated means UNKNOWN, not invalid: the widget has + // no information about which stored values are valid, which is exactly + // what the `OptionsEmptyState` below tells the user ("select Country + // first"). Clearing here fired on MOUNT with no interaction — a record + // that merely ARRIVES with the parent empty (a later-cleared parent, an + // import, a partially-migrated row) had its picklist silently staged for + // deletion. Read from the resolver's own `gated` flag rather than + // re-derived from `options.length === 0`, which would collide with case 1. + if (gated) return; + // 3. The list RESOLVED and genuinely excludes the value — a real cascade + // (the parent changed, a predicate flipped). That is the ADR-0058 + // contract and it still prunes, below. if (selected.length === 0) return; const stillOffered = selected.filter((v) => options.some((o) => o.value === v)); if (stillOffered.length !== selected.length) onChange(stillOffered); diff --git a/packages/fields/src/widgets/RadioField.tsx b/packages/fields/src/widgets/RadioField.tsx index 9ae71375d..f8f4a2458 100644 --- a/packages/fields/src/widgets/RadioField.tsx +++ b/packages/fields/src/widgets/RadioField.tsx @@ -53,6 +53,11 @@ export function RadioField({ // Never configured → nothing to prune against; see `MultiSelectField`'s // copy of this guard for the measured failure (objectui#4220). if (rawOptions.length === 0) return; + // Gated → the authored list is withheld until the `dependsOn` parent is + // chosen, so the empty offered set is missing information, not a verdict on + // the stored value; clearing here fired on mount (objectui#4247). Same + // reasoning, at length, in `MultiSelectField`. + if (gated) return; if (value === undefined || value === null || (value as unknown) === '') return; if (!isValueStillOffered(value, options)) onChange?.(undefined as unknown as string); // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/packages/fields/src/widgets/SelectField.tsx b/packages/fields/src/widgets/SelectField.tsx index 8480a5a6b..5af68aae7 100644 --- a/packages/fields/src/widgets/SelectField.tsx +++ b/packages/fields/src/widgets/SelectField.tsx @@ -112,6 +112,11 @@ function SingleSelectField({ // cascade prunes, and all four render the same `OptionsEmptyState` when // there is none. if (rawOptions.length === 0) return; + // Gated → the authored list is withheld until the `dependsOn` parent is + // chosen, so the empty offered set is missing information, not a verdict on + // the stored value; clearing here fired on mount (objectui#4247). Same + // reasoning, at length, in `MultiSelectField`. + if (gated) return; if (value === undefined || value === null || (value as unknown) === '') return; if (!isValueStillOffered(value, options)) onChange?.(undefined as unknown as string); // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/packages/fields/src/widgets/optionWidgets.gatedOptions.test.tsx b/packages/fields/src/widgets/optionWidgets.gatedOptions.test.tsx new file mode 100644 index 000000000..83a2b6d71 --- /dev/null +++ b/packages/fields/src/widgets/optionWidgets.gatedOptions.test.tsx @@ -0,0 +1,267 @@ +/** + * 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. + */ + +/** + * A GATED option list never deletes the stored value (objectui#4247). + * + * The four fixed-option widgets end their cascade resolution with a "drop what + * is no longer offered" effect (ADR-0058 / #2715). `resolveCascadingOptions` + * returns an EMPTY offered set whenever the list is gated — a declared + * `dependsOn` parent is still empty — so on mount the effect found nothing + * "still offered" and wrote the field empty, while the widget was rendering + * `OptionsEmptyState` ("select Country first") right next to it: the control + * tells the user it cannot offer anything, and deletes what they had. + * + * Ruling (#4247): **gated means UNKNOWN, not invalid.** The cascade clear exists + * so a USER-DRIVEN parent change prunes a now-invalid child; a gated set on + * MOUNT is missing information — the record simply arrived with the parent + * empty (a later-cleared parent, an import, a partially-migrated row) — and + * destroying stored data on missing information is silent data loss. Convergence + * stays where it belongs: once the parent IS chosen and the resolved set + * genuinely excludes the stored value, the existing prune applies. + * + * Three states, kept apart here so a future edit cannot collapse them: + * + * 1. never-configured — no authored `options` at all. Guarded since #4220; + * control only, pinned in `optionWidgets.unconfiguredOptions.test.tsx`. + * 2. gated — an authored list withheld behind an unmet `dependsOn`. NEW: the + * value survives. + * 3. resolved-and-excludes — the parent IS chosen and the resolved set does not + * offer the stored value. Clears; that is the ADR-0058 contract. + * + * The gate is read from the resolver's own `gated` flag, never re-derived from + * `options.length === 0` — that would collide with state 1, which is a + * different case with a different guard. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { SelectField } from './SelectField'; +import { MultiSelectField } from './MultiSelectField'; +import { CheckboxesField } from './CheckboxesField'; +import { RadioField } from './RadioField'; + +/** Options that exist for exactly one country — the cascade the gate withholds. */ +const PROVINCE_OPTIONS = [ + { label: 'Zhejiang', value: 'zj', visibleWhen: "record.country == 'cn'" }, + { label: 'California', value: 'ca', visibleWhen: "record.country == 'us'" }, +]; + +const provinceField = (type: string) => + ({ name: 'province', type, dependsOn: 'country', options: PROVINCE_OPTIONS }) as any; + +describe('option widgets — a GATED list never deletes the stored value (#4247)', () => { + it('MultiSelectField keeps its array and shows the parent-first hint', () => { + const onChange = vi.fn(); + render( + , + ); + expect(screen.getByTestId('multiselect-empty-province')).toHaveTextContent( + /select country first/i, + ); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('CheckboxesField keeps its array and shows the parent-first hint', () => { + const onChange = vi.fn(); + render( + , + ); + expect(screen.getByTestId('checkboxes-empty-province')).toHaveTextContent( + /select country first/i, + ); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('RadioField keeps its value and shows the parent-first hint', () => { + const onChange = vi.fn(); + render( + , + ); + expect(screen.getByTestId('radio-empty-province')).toHaveTextContent(/select country first/i); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('SelectField keeps its value and shows the parent-first hint', () => { + const onChange = vi.fn(); + render( + , + ); + expect(screen.getByTestId('select-empty-province')).toHaveTextContent(/select country first/i); + expect(onChange).not.toHaveBeenCalled(); + }); +}); + +describe('controls — a RESOLVED list still prunes what it does not offer (ADR-0058)', () => { + it('MultiSelectField drops the element the chosen parent excludes', () => { + const onChange = vi.fn(); + render( + , + ); + expect(onChange).toHaveBeenCalledWith(['zj']); + }); + + it('CheckboxesField drops the element the chosen parent excludes', () => { + const onChange = vi.fn(); + render( + , + ); + expect(onChange).toHaveBeenCalledWith(['zj']); + }); + + it('RadioField clears a value the chosen parent excludes', () => { + const onChange = vi.fn(); + render( + , + ); + expect(onChange).toHaveBeenCalledWith(undefined); + }); + + it('SelectField clears a value the chosen parent excludes', () => { + const onChange = vi.fn(); + render( + , + ); + expect(onChange).toHaveBeenCalledWith(undefined); + }); +}); + +describe('transition — a gated field converges the moment its parent is chosen (#4247)', () => { + it('MultiSelectField: gated keeps, then prunes once the resolved set excludes', () => { + const onChange = vi.fn(); + const { rerender } = render( + , + ); + // Gated: the value survives, nothing is staged. + expect(onChange).not.toHaveBeenCalled(); + + // The user picks the parent — the set resolves and genuinely excludes 'zj'. + rerender( + , + ); + expect(onChange).toHaveBeenCalledWith([]); + }); + + it('MultiSelectField: a resolved set that INCLUDES the stored value keeps it', () => { + const onChange = vi.fn(); + const { rerender } = render( + , + ); + rerender( + , + ); + expect(onChange).not.toHaveBeenCalled(); + expect(screen.getByTestId('multiselect-option-zj')).toBeInTheDocument(); + }); + + it('SelectField: gated keeps, then clears once the resolved set excludes', () => { + const onChange = vi.fn(); + const { rerender } = render( + , + ); + expect(onChange).not.toHaveBeenCalled(); + + rerender( + , + ); + expect(onChange).toHaveBeenCalledWith(undefined); + }); + + it('SelectField: a resolved set that INCLUDES the stored value keeps it', () => { + const onChange = vi.fn(); + const { rerender } = render( + , + ); + rerender( + , + ); + expect(onChange).not.toHaveBeenCalled(); + }); +});