Skip to content

Commit 993445d

Browse files
os-zhuangclaude
andcommitted
fix(form): harden the spec↔runtime form-field chokepoint, derive SelectOption, complete FormFieldSchema (#3090)
normalizeSectionField dropped four spec-17 FormField keys — worst of all the ADR-0089 canonical `visibleWhen` spelling (the deprecated `visibleOn` worked): route it into the view-level slot so it ANDs with the object-level rule, carry `dependsOn`/`keyField`/`disclosure`, and fold the same slot into WizardForm's final-submit gate so a view-hidden required field can no longer block submission from off-screen. A behavioral parity gate now walks the spec FormFieldSchema key set both ways (added key = unmapped failure, retired key = stale failure), mutation-tested. SelectOptionSchema derives from @objectstack/spec/data by reference — it used to strip `color` (which @object-ui/fields renders), `default`, and the per-option `visibleWhen` gate — with pinned divergences (value widened for UI forms, visibleWhen on the #2212 wire contract) and documented UI-only extensions; ledger 124 → 122 (SelectOption, SelectOptionSchema → ALLOW with drift guards). FormFieldSchema now covers every key the FormField interface declares and makes `type` optional to match it, so `objectui validate` stops rejecting metadata the renderer accepts and stops stripping typo'd predicates; `{ field: … }` (spec vocabulary) stays rejected, pinning the two-layer boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 8f82c61 commit 993445d

11 files changed

Lines changed: 580 additions & 12 deletions
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@object-ui/types": patch
3+
"@object-ui/plugin-form": patch
4+
---
5+
6+
fix(form): the spec↔runtime form-field chokepoint stops dropping spec 17 vocabulary, and the validator stops contradicting the renderer — #3090
7+
8+
`normalizeSectionField` — the one translation point between the spec's authored
9+
form-field shape (`field` = object-field reference) and the runtime shape
10+
(`name` = data path) — silently dropped four spec keys, worst of all the
11+
ADR-0089 **canonical** `visibleWhen` spelling while the deprecated `visibleOn`
12+
worked. Now:
13+
14+
- view-level `visibleWhen` routes into the view-level slot (`visibleOn`) so it
15+
ANDs with the object-level rule instead of clobbering it, and the wizard's
16+
final-submit gate folds the same slot into its verdict (before, a required
17+
field the view itself hides could block submission from off-screen);
18+
- `dependsOn`, `keyField`, and `disclosure` carry through;
19+
- a behavioral parity gate walks the spec `FormFieldSchema` key set — a key the
20+
spec adds fails as unmapped, a key it retires fails as stale.
21+
22+
`SelectOptionSchema` is now derived from `@objectstack/spec/data` by reference
23+
(it used to strip `color` — which `@object-ui/fields` renders — plus `default`
24+
and the per-option `visibleWhen` gate), with pinned divergences (`value`
25+
widened for UI forms, `visibleWhen` on the #2212 wire contract) and documented
26+
UI-only extensions (`disabled`, `icon`). `SelectOption` (TS) gains `color` and
27+
`default`.
28+
29+
`FormFieldSchema` (the runtime vocabulary `objectui validate` enforces) now
30+
covers every key the `FormField` interface declares — `widget`, `dependsOn`,
31+
`hidden`, `readonly`, `visibleOn`/`visibleWhen`/`readonlyWhen`/`requiredWhen`,
32+
`span` — and `type` is optional, matching the interface. A typo'd predicate now
33+
fails loudly instead of being stripped; spec-shape fields (`{ field: … }`) are
34+
still rejected, pinning the two-layer boundary.

packages/plugin-form/src/WizardForm.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import React, { useState, useCallback, useMemo } from 'react';
1717
import type { FormField, DataSource } from '@object-ui/types';
1818
import { Button, cn, toast } from '@object-ui/components';
1919
import { AlertCircle, Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-react';
20-
import { resolveFieldRuleState } from '@object-ui/core';
20+
import { resolveFieldRuleState, evalFieldPredicate } from '@object-ui/core';
2121
import { createSafeTranslation } from '@object-ui/i18n';
2222
import { FormSection } from './FormSection';
2323
import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react';
@@ -352,8 +352,16 @@ export const WizardForm: React.FC<WizardFormProps> = ({
352352
record,
353353
{ required: !!field.required, readonly: (field as any).readonly === true },
354354
);
355+
// View-level FormField.visibleOn hides the field the same way a
356+
// field-level visibleWhen does — fold it into the verdict exactly
357+
// like the form renderer (form.tsx) does, or the gate demands a
358+
// field the view itself hides (#3090). Since #3090 this slot also
359+
// receives the ADR-0089 canonical view-level `visibleWhen` spelling.
360+
const viewVisible =
361+
(field as any).visibleOn == null ||
362+
evalFieldPredicate((field as any).visibleOn, record, true);
355363
// A hidden or read-only field is not the user's to fill in.
356-
if (!state.visible || state.readonly || !state.required) continue;
364+
if (!viewVisible || !state.visible || state.readonly || !state.required) continue;
357365
if (!isEmptyValue(record[name])) continue;
358366
out.set(index, [...(out.get(index) ?? []), (field as any).label || name]);
359367
}
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
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+
* `normalizeSectionField` ↔ `@objectstack/spec` FormFieldSchema coverage gate
11+
* (#3090, objectstack#4115).
12+
*
13+
* The normalizer is THE chokepoint between the two form-field vocabularies:
14+
* the spec's authored shape (`field` = object-field reference, presentation
15+
* deltas only) and the runtime shape (`name` = data path, self-contained
16+
* widget config). It cannot be a derivation — every key is a *translation*
17+
* with a destination of its own — so the gate is behavioral instead:
18+
*
19+
* - TABLE below holds one row per spec key: a sample authored value and an
20+
* assertion that the normalized output reflects it at its documented
21+
* destination. Deleting a mapping line in `sectionFields.ts` fails the
22+
* row — the rows are mutation-tested coverage, not a mirror list.
23+
* - The key sets are then compared BOTH ways against the spec's own
24+
* enumeration: a key the spec adds fails as UNMAPPED (decide: map it or
25+
* exempt it with a reason); a key the spec retires fails as STALE.
26+
*
27+
* Until this gate existed the drift was real: the ADR-0089 canonical
28+
* `visibleWhen` spelling, `dependsOn`, `keyField` and `disclosure` were all
29+
* silently dropped while the DEPRECATED `visibleOn` spelling worked.
30+
*/
31+
32+
import { describe, it, expect } from 'vitest';
33+
import { FormFieldSchema as SpecFormFieldSchema } from '@objectstack/spec/ui';
34+
import { mapFieldTypeToFormType } from '@object-ui/fields';
35+
import { normalizeSectionField } from './sectionFields';
36+
37+
const objectSchema = {
38+
name: 'crm_account',
39+
fields: {
40+
industry: {
41+
type: 'select',
42+
label: 'Industry',
43+
options: [{ label: 'Tech', value: 'tech' }],
44+
},
45+
},
46+
};
47+
48+
const ctx = {
49+
objectSchema,
50+
objectName: 'crm_account',
51+
fieldLabel: (_obj: string, _name: string, fallback: string) => fallback || _name,
52+
};
53+
54+
/** Normalize a spec field def against a schema that does NOT define `ghost`,
55+
* so every override is observable verbatim (no object-metadata interference). */
56+
const norm = (def: Record<string, unknown>) =>
57+
normalizeSectionField({ field: 'ghost', ...def }, ctx) as any;
58+
59+
/**
60+
* One row per spec FormFieldSchema key: sample authored value → where the
61+
* runtime FormField must carry it. Non-obvious destinations are the point:
62+
* `helpText`→`description`, `readonly`→`disabled`, and BOTH visibility
63+
* spellings→the view-level `visibleOn` slot (the runtime `visibleWhen` slot
64+
* belongs to the object-level rule and must not be clobbered).
65+
*/
66+
const TABLE: Record<string, (f: (def: Record<string, unknown>) => any) => void> = {
67+
field: () => {
68+
const out = normalizeSectionField({ field: 'industry' }, ctx) as any;
69+
expect(out.name).toBe('industry'); // identity key translated to the data path
70+
expect(out.type).toBe(mapFieldTypeToFormType('select')); // object schema merged in
71+
},
72+
type: (f) => expect(f({ type: 'text' }).type).toBe(mapFieldTypeToFormType('text')),
73+
options: (f) =>
74+
expect(f({ options: [{ label: 'A', value: 'a' }] }).options).toEqual([
75+
{ label: 'A', value: 'a' },
76+
]),
77+
reference: (f) => {
78+
const out = f({ reference: 'accounts' });
79+
expect(out.reference).toBe('accounts');
80+
expect(out.reference_to).toBe('accounts'); // both spellings stamped (#2407)
81+
},
82+
maxLength: (f) => expect(f({ maxLength: 10 }).maxLength).toBe(10),
83+
minLength: (f) => expect(f({ minLength: 2 }).minLength).toBe(2),
84+
min: (f) => expect(f({ min: 1 }).min).toBe(1),
85+
max: (f) => expect(f({ max: 9 }).max).toBe(9),
86+
precision: (f) => expect(f({ precision: 10 }).precision).toBe(10),
87+
scale: (f) => expect(f({ scale: 2 }).scale).toBe(2),
88+
multiple: (f) => expect(f({ multiple: true }).multiple).toBe(true),
89+
label: (f) => expect(f({ label: 'Custom' }).label).toBe('Custom'),
90+
placeholder: (f) => expect(f({ placeholder: 'Type…' }).placeholder).toBe('Type…'),
91+
helpText: (f) => expect(f({ helpText: 'Hint' }).description).toBe('Hint'),
92+
readonly: (f) => expect(f({ readonly: true }).disabled).toBe(true),
93+
immutable: (f) => expect(f({ immutable: true }).immutable).toBe(true),
94+
required: (f) => expect(f({ required: true }).required).toBe(true),
95+
hidden: (f) => expect(f({ hidden: true }).hidden).toBe(true),
96+
colSpan: (f) => expect(f({ colSpan: 2 }).colSpan).toBe(2),
97+
span: (f) => expect(f({ span: 'full' }).span).toBe('full'),
98+
widget: (f) => expect(f({ widget: 'rating' }).widget).toBe('rating'),
99+
language: (f) => expect(f({ language: 'sql' }).language).toBe('sql'),
100+
fields: (f) =>
101+
expect(f({ fields: [{ field: 'inner' }] }).fields).toEqual([{ field: 'inner' }]),
102+
keyField: (f) =>
103+
expect(f({ keyField: { field: 'name' } }).keyField).toEqual({ field: 'name' }),
104+
dependsOn: (f) => expect(f({ dependsOn: 'country' }).dependsOn).toBe('country'),
105+
visibleWhen: (f) =>
106+
expect(f({ visibleWhen: 'record.a == 1' }).visibleOn).toBe('record.a == 1'),
107+
visibleOn: (f) =>
108+
expect(f({ visibleOn: 'record.b == 2' }).visibleOn).toBe('record.b == 2'),
109+
disclosure: (f) => expect(f({ disclosure: 'popover' }).disclosure).toBe('popover'),
110+
};
111+
112+
/** Spec keys the normalizer deliberately does not carry, each with a reason.
113+
* Empty today — add entries here (never silently) when the spec grows a key
114+
* that genuinely has no runtime destination. */
115+
const EXEMPT: Record<string, string> = {};
116+
117+
describe('normalizeSectionField covers the spec FormFieldSchema key set', () => {
118+
// `.strict().transform()` wraps the object in a ZodPipe; `.in` is the
119+
// strict object carrying the authoring shape.
120+
const specKeys = Object.keys((SpecFormFieldSchema as any).in.shape).sort();
121+
122+
it('has a behavioral row (or an exemption with a reason) for every spec key', () => {
123+
const covered = [...Object.keys(TABLE), ...Object.keys(EXEMPT)].sort();
124+
expect(covered).toEqual(specKeys);
125+
});
126+
127+
it('has no stale rows for keys the spec has retired', () => {
128+
for (const key of [...Object.keys(TABLE), ...Object.keys(EXEMPT)]) {
129+
expect(specKeys, `'${key}' is no longer a spec FormField key`).toContain(key);
130+
}
131+
});
132+
133+
for (const [key, assertRow] of Object.entries(TABLE)) {
134+
it(`carries spec '${key}' to its runtime destination`, () => {
135+
assertRow(norm);
136+
});
137+
}
138+
});

packages/plugin-form/src/sectionFields.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,71 @@ describe('normalizeSectionField', () => {
106106
expect((f as any).visibleOn).toEqual(expr);
107107
});
108108

109+
// ── Spec 17 late-added / renamed keys (#3090) ─────────────────────────────
110+
// ADR-0089 renamed the view-level predicate to `visibleWhen` — which is also
111+
// the runtime slot for the OBJECT-level rule. The view predicate must land in
112+
// the view-level slot (`visibleOn`) so the renderer ANDs both layers
113+
// (form.tsx evaluates the two slots independently) instead of one clobbering
114+
// the other. Before the fix the canonical spelling was silently dropped while
115+
// the DEPRECATED spelling worked.
116+
117+
it('routes a view-level `visibleWhen` (canonical spelling) into the view-level slot', () => {
118+
const f = normalizeSectionField(
119+
{ field: 'name', visibleWhen: "record.stage == 'won'" },
120+
ctx,
121+
) as any;
122+
expect(f.visibleOn).toBe("record.stage == 'won'");
123+
});
124+
125+
it('carries a `{ dialect, source }` view-level visibleWhen expression', () => {
126+
const expr = { dialect: 'cel', source: "record.priority == 'urgent'" };
127+
const f = normalizeSectionField({ field: 'name', visibleWhen: expr }, ctx) as any;
128+
expect(f.visibleOn).toEqual(expr);
129+
});
130+
131+
it('layers the view predicate OVER the object-level rule instead of clobbering it', () => {
132+
const rulesCtx = {
133+
...ctx,
134+
objectSchema: {
135+
...objectSchema,
136+
fields: {
137+
...objectSchema.fields,
138+
paid_on: { type: 'date', label: 'Paid on', visibleWhen: "record.status == 'paid'" },
139+
},
140+
},
141+
};
142+
const f = normalizeSectionField(
143+
{ field: 'paid_on', visibleWhen: 'record.amount > 0' },
144+
rulesCtx,
145+
) as any;
146+
expect(f.visibleWhen).toBe("record.status == 'paid'"); // object-level rule intact
147+
expect(f.visibleOn).toBe('record.amount > 0'); // view predicate in the view slot
148+
});
149+
150+
it('prefers the canonical spelling when both visibleWhen and deprecated visibleOn are authored', () => {
151+
// `saveMeta` persists verbatim, so served metadata can carry either (or,
152+
// after a partial migration, both). Canonical wins.
153+
const f = normalizeSectionField(
154+
{ field: 'name', visibleWhen: "record.a == 1", visibleOn: "record.b == 2" },
155+
ctx,
156+
) as any;
157+
expect(f.visibleOn).toBe("record.a == 1");
158+
});
159+
160+
it('carries a view-level dependsOn (spec cascading declaration)', () => {
161+
const f = normalizeSectionField({ field: 'industry', dependsOn: 'country' }, ctx) as any;
162+
expect(f.dependsOn).toBe('country');
163+
});
164+
165+
it('carries keyField and disclosure through for record/composite widgets', () => {
166+
const f = normalizeSectionField(
167+
{ field: 'billing_address', keyField: { field: 'name', immutable: true }, disclosure: 'popover' },
168+
ctx,
169+
) as any;
170+
expect(f.keyField).toEqual({ field: 'name', immutable: true });
171+
expect(f.disclosure).toBe('popover');
172+
});
173+
109174
it('copies field-level conditional rules from the object schema (#2212)', () => {
110175
const rulesSchema = {
111176
...objectSchema,

packages/plugin-form/src/sectionFields.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,25 @@ export function normalizeSectionField(
152152
if (fd.scale != null) base.scale = fd.scale;
153153
if (fd.language != null) base.language = fd.language;
154154
if (Array.isArray(fd.fields)) base.fields = fd.fields;
155+
// View-level cascading declaration (spec FormField.dependsOn, a bare parent
156+
// name). The form renderer gates + recomputes options off the runtime
157+
// field's top-level `dependsOn`; without this copy the declaration vanished.
158+
if (fd.dependsOn != null) base.dependsOn = fd.dependsOn;
159+
// Record/composite widget config (ADR-0007). Pass-through: no widget reads
160+
// them from the runtime field yet, but dropping them here would make that
161+
// support impossible to ship metadata-first.
162+
if (fd.keyField != null) base.keyField = fd.keyField;
163+
if (fd.disclosure != null) base.disclosure = fd.disclosure;
155164

156-
return attachVisibility(base as FormField, fd.visibleOn);
165+
// View-level visibility predicate. ADR-0089 renamed it to `visibleWhen` —
166+
// which collides with the runtime slot holding the OBJECT-level rule (copied
167+
// from the object schema in `fromObjectSchema`). Route the view predicate
168+
// into the view-level slot (`visibleOn`) instead: the renderer evaluates the
169+
// two slots independently and ANDs them, so layering is preserved and the
170+
// object rule is never clobbered. Canonical spelling wins over the
171+
// deprecated one when both are authored (`saveMeta` persists verbatim, so
172+
// served metadata can carry either).
173+
return attachVisibility(base as FormField, fd.visibleWhen ?? fd.visibleOn);
157174
}
158175

159176
/** Normalize every field def in a section. */

packages/plugin-form/src/wizardSkipValidation.test.tsx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,36 @@ describe('WizardForm allowSkip — required fields on a skipped step (#2959)', (
175175
expect(stepIndicator(1)).not.toHaveAttribute('data-error');
176176
});
177177

178+
it('does not demand a required field the view itself hides (view-level visibleWhen false)', async () => {
179+
// The section field def carries the SPEC vocabulary: `field` + a view-level
180+
// `visibleWhen` (ADR-0089 canonical spelling). `normalizeSectionField`
181+
// routes that predicate into the view-level slot (`visibleOn`, #3090) and
182+
// the renderer hides the field — so the submit gate must fold the same
183+
// slot into its verdict, or it demands a field the user cannot see.
184+
const dataSource = renderWizard({
185+
allowSkip: true,
186+
sections: [
187+
{ name: 's1', label: 'Step 1', fields: ['subject'] },
188+
{
189+
name: 's2',
190+
label: 'Step 2',
191+
fields: [{ field: 'owner', visibleWhen: "record.subject == 'urgent'" }],
192+
},
193+
{ name: 's3', label: 'Step 3', fields: ['notes'] },
194+
],
195+
});
196+
197+
await waitFor(() => expect(document.body.querySelector('[data-field="subject"]')).toBeTruthy());
198+
fill('subject', 'routine'); // predicate false → owner is view-hidden
199+
fireEvent.click(stepIndicator(2));
200+
await waitFor(() => expect(document.body.querySelector('[data-field="notes"]')).toBeTruthy());
201+
fill('notes', 'S3');
202+
fireEvent.click(screen.getByRole('button', { name: /Create/i }));
203+
204+
await waitFor(() => expect(dataSource.create).toHaveBeenCalledTimes(1));
205+
expect(stepIndicator(1)).not.toHaveAttribute('data-error');
206+
});
207+
178208
it('still lets the sequential wizard submit (the gate is not a new blocker)', async () => {
179209
const dataSource = renderWizard();
180210

0 commit comments

Comments
 (0)