Skip to content

Commit 38ca8be

Browse files
os-zhuangclaude
andauthored
refactor(fields): drop the retired conditionalRequired alias (#2952)
`@objectstack/spec` 17 (objectstack#3855) retired `Field.conditionalRequired`, the deprecated alias of `requiredWhen`. It is tombstoned with `retiredKey()` (`z.never()`), so authoring it is a `tsc` error AND a hard parse rejection carrying the rename prescription — verified against 17.0.0-rc.0 at both `FieldSchema` and `ObjectSchema`. Spec-parsed metadata therefore cannot carry the key: an object declaring it fails to load rather than loading with the rule silently dropped. Keeping a renderer-side `requiredWhen ?? conditionalRequired` would have re-created the second de-facto contract the tombstone exists to prevent — the key would keep working in the UI while being rejected everywhere else, hiding the producer's bug (AGENTS.md #0.1). Backend-agnosticism (#1) does not argue for keeping it: `conditionalRequired` is an ObjectStack-spec-ism, so the only producers that emit it are ObjectStack producers on <=16, and the spec ships them `os migrate meta --from 16`. Removed from the `FormField` member (types), the `??` fallback and rules-param member in `resolveFieldRuleState` (core), three form-renderer pass-throughs (components), `ObjectForm` / `ModalForm` / `sectionFields` / `deriveMasterDetail` (plugin-form), and the field inspector's legacy read/auto-migrate plus the key's CEL-lint entry (app-shell). Studio authors lose nothing: draft validation parses against the spec's own `ObjectSchema`, so a draft carrying the key now surfaces the tombstone's rename prescription under the same `fields.<name>.conditionalRequired` path the CEL lint used to report — a better message than the silent auto-migration, and one the server agrees with. Pinned by a test. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 475afcf commit 38ca8be

18 files changed

Lines changed: 124 additions & 68 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@object-ui/types": minor
3+
"@object-ui/core": minor
4+
"@object-ui/components": minor
5+
"@object-ui/plugin-form": minor
6+
"@object-ui/app-shell": minor
7+
---
8+
9+
refactor(fields): `requiredWhen` is the only required-predicate slot — drop the retired `conditionalRequired` alias
10+
11+
`@objectstack/spec` 17 (objectstack#3855) **retired** `Field.conditionalRequired`,
12+
the long-deprecated alias of `requiredWhen`. ObjectUI carried a back-compat read
13+
for it in seven places; all of them are removed.
14+
15+
The removal is safe because the spec did not merely *stop emitting* the key — it
16+
made authoring it **fail loudly**. `retiredKey()` declares the key as
17+
`z.never()`, so:
18+
19+
- `z.input` types it as `never` — writing it is a `tsc` error at the authoring site;
20+
- the parse **rejects** it (verified against `17.0.0-rc.0`), at both `FieldSchema`
21+
and `ObjectSchema`, with the prescription as the message:
22+
23+
> `conditionalRequired` was removed in @objectstack/spec 17 (#3855) — use
24+
> `requiredWhen`. Rename the key; the value (a CEL predicate) is unchanged.
25+
> Run `os migrate meta --from 16` to rewrite it automatically.
26+
27+
So spec-parsed metadata cannot carry the key — an object declaring it fails to
28+
load rather than loading with the rule silently dropped. Keeping a renderer-side
29+
`requiredWhen ?? conditionalRequired` would have re-created exactly the second
30+
de-facto contract the tombstone exists to prevent: the key would have kept
31+
working in the UI while being rejected everywhere else, hiding the producer's bug
32+
(AGENTS.md #0.1). "Backend-agnostic" (#1) does not argue for keeping it either —
33+
`conditionalRequired` is an ObjectStack-spec-ism, so the only producers that ever
34+
emit it are ObjectStack producers on ≤16, and the spec ships them a converter.
35+
36+
Removed from:
37+
38+
| package | site |
39+
| :--- | :--- |
40+
| `@object-ui/types` | the `conditionalRequired?:` member on `FormField` |
41+
| `@object-ui/core` | the `??` fallback + rules-param member in `resolveFieldRuleState` |
42+
| `@object-ui/components` | three pass-throughs in the form renderer |
43+
| `@object-ui/plugin-form` | `ObjectForm`, `ModalForm`, `sectionFields`, `deriveMasterDetail` (×2) |
44+
| `@object-ui/app-shell` | the field inspector's legacy read/auto-migrate, and the key's entry in `clientValidation`'s CEL lint list |
45+
46+
**Studio authors lose nothing.** The object designer's draft validation parses
47+
against the spec's own `ObjectSchema`, so a draft carrying the key now surfaces
48+
the tombstone's rename prescription under the same `fields.<name>.conditionalRequired`
49+
path the CEL lint used to report — a better message than the inspector's silent
50+
auto-migration, and one the server agrees with. That behavior is pinned by a test.
51+
52+
**Migrating:** rename the key to `requiredWhen` (the CEL value is unchanged), or
53+
run `os migrate meta --from 16`.

content/docs/guide/metadata-diagnostics.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,9 @@ field disappears as soon as you start typing in it, then re-validates
139139
on save.
140140

141141
For **object** drafts the live validation goes beyond the Zod shape check:
142-
every field conditional rule (`visibleWhen` / `readonlyWhen` / `requiredWhen`,
143-
plus the deprecated `conditionalRequired` alias) is linted as a CEL predicate
144-
with the same `@objectstack/formula` validators the server uses. A predicate
142+
every field conditional rule (`visibleWhen` / `readonlyWhen` / `requiredWhen`)
143+
is linted as a CEL predicate with the same `@objectstack/formula` validators
144+
the server uses. A predicate
145145
that parses but references an unknown field, or references a field bare
146146
instead of as `record.<field>`, surfaces under its `fields.<field>.<rule>`
147147
path in the banner. The field inspector's *Conditional rules* editors give

docs/adr/0036-field-conditional-rules.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,17 @@ We express them as three optional CEL predicates on `Field`:
2323
| `readonlyWhen` | the field is read-only | **client + server**|
2424
| `requiredWhen` | the field is required | **client + server**|
2525

26-
`conditionalRequired` is a back-compat **alias of `requiredWhen`**.
26+
> **Amended (2026-07-29) — the `conditionalRequired` alias is gone.**
27+
> This ADR originally carried `conditionalRequired` as a back-compat **alias of
28+
> `requiredWhen`**. `@objectstack/spec` 17 (objectstack#3855) *retired* the key:
29+
> it is tombstoned via `retiredKey()`, so authoring it is both a `tsc` error and
30+
> a hard parse rejection whose message carries the rename (and
31+
> `os migrate meta --from 16` rewrites it automatically). Since the producer now
32+
> rejects the key outright, ObjectUI no longer reads it anywhere — keeping a
33+
> renderer-side `??` fallback would have re-created the second dialect the
34+
> tombstone exists to prevent (AGENTS.md #0.1). **`requiredWhen` is the only
35+
> required-predicate slot.** The paragraphs below are kept as the historical
36+
> record of the original decision.
2737
2838
## Why CEL, and why the *same* engine on both ends
2939

@@ -47,7 +57,8 @@ import dragged into the bundle.
4757
- **`requiredWhen`**`@objectstack/objectql`'s rule-validator evaluates the
4858
predicate over the *merged* record (`{ ...previous, ...patch }`) and pushes a
4959
`{ field, code: 'required' }` violation when it is TRUE and the value is
50-
missing. `conditionalRequired` is treated identically.
60+
missing. (Historical: `conditionalRequired` was treated identically until the
61+
spec retired the key — see the amendment above.)
5162
- **`readonlyWhen`**`stripReadonlyWhenFields` drops any field from an UPDATE
5263
payload whose predicate is TRUE for the merged record: the incoming change is
5364
**ignored** (the persisted value is kept), not rejected. Update paths fetch

packages/app-shell/README.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,10 @@ The object designer's field inspector (`ObjectFieldInspector`, Advanced →
248248
typing `record.` / `previous.` completes the object's own field names.
249249
- Values round-trip both wire shapes: a bare CEL string or the
250250
`{ dialect, source }` Expression envelope (envelope extras such as
251-
`meta.rationale` are preserved on edit). The deprecated
252-
`conditionalRequired` alias is read into the *Required when* editor and
253-
migrated to `requiredWhen` on the first edit.
251+
`meta.rationale` are preserved on edit). `requiredWhen` is the only
252+
required-predicate slot — the `conditionalRequired` alias was removed in
253+
`@objectstack/spec` 17 (#3855), so a draft carrying it is rejected by the
254+
spec parse itself, with the rename prescription, in the same issue banner.
254255
- The same lint also runs draft-wide in `clientValidation.ts`
255256
(`validateMetadataDraft('object', …)`), so an invalid predicate on any
256257
field — not just the selected one — surfaces in the editor's issue banner

packages/app-shell/src/views/metadata-admin/clientValidation.fieldRules.test.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { __setCelFormulaLoader } from './celAuthoring';
1717

1818
afterEach(() => __setCelFormulaLoader(undefined));
1919

20-
const RULE_PATH = /^fields\..+\.(visibleWhen|readonlyWhen|requiredWhen|conditionalRequired)$/;
20+
const RULE_PATH = /^fields\..+\.(visibleWhen|readonlyWhen|requiredWhen)$/;
2121

2222
const draftWith = (fields: unknown) => ({ name: 'account', label: 'Account', fields });
2323

@@ -59,12 +59,19 @@ describe('validateMetadataDraft — object field conditional rules (real engine)
5959
expect(res.issues.filter((i) => RULE_PATH.test(i.path))).toEqual([]);
6060
});
6161

62-
it('lints the legacy conditionalRequired alias too', async () => {
62+
// `conditionalRequired` is no longer one of OUR lint keys — @objectstack/spec
63+
// 17 (#3855) retired it, so the draft is rejected by the spec parse itself,
64+
// and the rejection carries the rename prescription. We assert the author
65+
// still gets told, and told by the spec (AGENTS.md #0.1 — one contract).
66+
it('rejects the retired conditionalRequired alias via the spec tombstone', async () => {
6367
const res = await validateMetadataDraft(
6468
'object',
65-
draftWith({ status: { type: 'text', conditionalRequired: '{status} == "x"' } }),
69+
draftWith({ status: { type: 'text', conditionalRequired: 'record.x == 1' } }),
6670
);
67-
expect(res.issues.some((i) => i.path === 'fields.status.conditionalRequired')).toBe(true);
71+
expect(res.ok).toBe(false);
72+
const issue = res.issues.find((i) => i.path === 'fields.status.conditionalRequired');
73+
expect(issue).toBeTruthy();
74+
expect(issue!.message).toMatch(/requiredWhen/);
6875
});
6976

7077
it('uses index paths for the ARRAY fields shape', async () => {

packages/app-shell/src/views/metadata-admin/clientValidation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ function nodeTypeAt(draft: unknown, index: number): string | undefined {
133133
* fail-opens at runtime, so we lint the CEL here — the same
134134
* `@objectstack/formula` verdict the field inspector's editor shows live.
135135
*/
136-
const FIELD_RULE_KEYS = ['visibleWhen', 'readonlyWhen', 'requiredWhen', 'conditionalRequired'] as const;
136+
const FIELD_RULE_KEYS = ['visibleWhen', 'readonlyWhen', 'requiredWhen'] as const;
137137

138138
/** Extract a predicate's CEL source from either wire shape (string | envelope). */
139139
function predicateSource(v: unknown): string | null {

packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.test.tsx

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -257,17 +257,14 @@ describe('ObjectFieldInspector — conditional rules (CEL editors, #1582)', () =
257257
expect(onPatch.mock.calls.at(-1)![0].fields.note.visibleWhen).toBeUndefined();
258258
});
259259

260-
it('reads legacy conditionalRequired into Required when and migrates it on edit', () => {
260+
// The retired `conditionalRequired` alias (@objectstack/spec 17, #3855) is no
261+
// longer read into the editor: it is not a required-predicate slot, and a
262+
// draft carrying it is rejected by the spec parse with the rename
263+
// prescription (see clientValidation.fieldRules.test.ts).
264+
it('does not read the retired conditionalRequired alias into Required when', () => {
261265
stubEngine();
262-
const { onPatch } = renderField(
263-
{ note: { type: 'text', conditionalRequired: 'record.x == 1' } },
264-
'note',
265-
);
266-
expect(controlFor('Required when')).toHaveValue('record.x == 1');
267-
fireEvent.change(controlFor('Required when'), { target: { value: 'record.x == 2' } });
268-
const field = onPatch.mock.calls.at(-1)![0].fields.note;
269-
expect(field.requiredWhen).toBe('record.x == 2');
270-
expect(field.conditionalRequired).toBeUndefined();
266+
renderField({ note: { type: 'text', conditionalRequired: 'record.x == 1' } }, 'note');
267+
expect(controlFor('Required when')).toHaveValue('');
271268
});
272269
});
273270

packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -641,15 +641,8 @@ export function ObjectFieldInspector({
641641
<CelPredicateField
642642
id={`field-rule-required-${entry.name}`}
643643
label={tr('designer.field.requiredWhen')}
644-
// Legacy alias: `conditionalRequired` (spec @deprecated) reads into
645-
// the same editor; the first edit migrates it to `requiredWhen`.
646-
value={readPredicate(def.requiredWhen ?? def.conditionalRequired)}
647-
onChange={(v) =>
648-
patchDef({
649-
requiredWhen: writePredicate(def.requiredWhen ?? def.conditionalRequired, v),
650-
conditionalRequired: undefined,
651-
})
652-
}
644+
value={readPredicate(def.requiredWhen)}
645+
onChange={(v) => patchDef({ requiredWhen: writePredicate(def.requiredWhen, v) })}
653646
disabled={readOnly}
654647
placeholder="record.amount > 10000"
655648
objectName={typeof (draft as any).name === 'string' ? ((draft as any).name as string) : undefined}

packages/components/src/renderers/form/form.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,6 @@ ComponentRegistry.register('form',
374374
visibleWhen: (f as any).visibleWhen,
375375
readonlyWhen: (f as any).readonlyWhen,
376376
requiredWhen: (f as any).requiredWhen,
377-
conditionalRequired: (f as any).conditionalRequired,
378377
},
379378
ruleRecord,
380379
{ required: !!f.required, readonly: (f as any).readonly === true },
@@ -733,7 +732,6 @@ ComponentRegistry.register('form',
733732
visibleWhen,
734733
readonlyWhen,
735734
requiredWhen,
736-
conditionalRequired,
737735
...fieldProps
738736
} = field;
739737

@@ -756,7 +754,7 @@ ComponentRegistry.register('form',
756754
// the UX and the persisted verdict agree. A field with no rules
757755
// resolves to its static flags unchanged.
758756
const ruleState = resolveFieldRuleState(
759-
{ visibleWhen, readonlyWhen, requiredWhen, conditionalRequired },
757+
{ visibleWhen, readonlyWhen, requiredWhen },
760758
ruleRecord,
761759
{ required: staticRequired, readonly: staticReadonly === true },
762760
);

packages/core/src/evaluator/__tests__/fieldRules.test.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,18 +106,17 @@ describe('resolveFieldRuleState', () => {
106106
expect(s.required).toBe(true);
107107
});
108108

109-
it('honors conditionalRequired as a requiredWhen alias', () => {
110-
const s = resolveFieldRuleState({ conditionalRequired: "record.status == 'sent'" }, { status: 'sent' }, {});
111-
expect(s.required).toBe(true);
112-
});
113-
114-
it('prefers requiredWhen over conditionalRequired when both present', () => {
115-
const s = resolveFieldRuleState(
116-
{ requiredWhen: "record.status == 'sent'", conditionalRequired: 'record.amount > 0' },
117-
{ status: 'draft', amount: 5 },
118-
{},
119-
);
120-
// requiredWhen (status=='sent') is FALSE → not required; the alias is ignored.
109+
// `conditionalRequired` was the pre-protocol-17 alias of `requiredWhen`.
110+
// @objectstack/spec 17 (#3855) tombstoned it with `retiredKey`, so authoring
111+
// it is now a `tsc` error AND a hard parse rejection carrying the rename
112+
// prescription — it can never reach this evaluator from parsed metadata.
113+
// `requiredWhen` is the only required-predicate slot (AGENTS.md #0.1).
114+
it('ignores the retired conditionalRequired alias', () => {
115+
// Deliberately a shape the parameter type no longer permits.
116+
const legacyRules = { conditionalRequired: "record.status == 'sent'" } as unknown as Parameters<
117+
typeof resolveFieldRuleState
118+
>[0];
119+
const s = resolveFieldRuleState(legacyRules, { status: 'sent' }, {});
121120
expect(s.required).toBe(false);
122121
});
123122

0 commit comments

Comments
 (0)