diff --git a/.changeset/visible-when-alias-guidance.md b/.changeset/visible-when-alias-guidance.md new file mode 100644 index 0000000000..7709c588db --- /dev/null +++ b/.changeset/visible-when-alias-guidance.md @@ -0,0 +1,49 @@ +--- +"@objectstack/spec": patch +--- + +docs(spec): alias guidance for `visible` / `showWhen` / `disabled` on the `visibleWhen` shapes (#7832) + +`ui/action.zod.ts` has carried this table in the OTHER direction since #3746: on +an action, `visible` and `disabled` are the canonical keys, so the aliases run +`visibleWhen → visible`, `showWhen → visible`, `disabledWhen → disabled`. The +reverse direction — an author who learned the action vocabulary writing it on a +shape whose canonical key is `visibleWhen` — was curated on some surfaces and +bare on others, and nothing recorded which was which. + +**Nothing changes about what parses.** Every key named here was rejected before +and is rejected after; only the message differs. `RowCrudActionOverrideSchema` +moves from a hand-written `.strict()` to the shared `strictObject` helper, which +is `z.object(shape, { error }).strict()` — the same door, now with a curated +error map behind it. + +What each surface says now: + +- **`userActions.edit` / `.delete` overrides** (`RowCrudActionOverrideSchema`) + produced zod's own bare `Unrecognized key: "visible"` — the surface unnamed + and no key to write instead. It now names the surface, renames `showWhen` onto + `visibleWhen`, and answers `visible` / `disabled` in prose naming BOTH landing + keys: `enabled: false` for the object-level switch, `visibleWhen` / + `disabledWhen` for the per-record predicate. Prose rather than a rename + because this shape splits into a boolean and a predicate what a custom row + action spells with one key, so a rename would have to guess which the author + meant. +- **Fields** rename `showWhen` onto `visibleWhen`, and answer `visible` in prose + naming both `hidden` and `visibleWhen` — including the inversion, since + `visible: false` is `hidden: true` and a rename would have the author ship the + opposite of what they wrote. `disabled → readonly` was already correct here: a + field has `readonlyWhen`, not `disabledWhen`. +- **Form fields** rename `disabled` onto `readonly`, the one shape in the + view/page family that declares a key for it. + +Three surfaces already answered `visible` / `showWhen` and gained no row: select +options rename both onto `visibleWhen`, and form sections and page components +answer them through the shared ADR-0089 conditional-visibility prescription. +Select options, form sections and page components declare no disabled-ish key at +all, so `disabled` there stays a bare rejection rather than being pointed at a +key the shape would reject next — pinned so a later sweep can tell a deliberate +gap from a missed one. + +Choosing ONE canonical spelling across the two vocabularies remains open +(#7816); if that ruling ever converges them, these rows become the migration +hint. diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index 91aee8aa35..803bba87a8 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -458,6 +458,12 @@ export const FieldSchema = lazySchema(() => strictObject({ decimals: 'scale', decimalPlaces: 'scale', digits: 'precision', isReadonly: 'readonly', disabled: 'readonly', isHidden: 'hidden', invisible: 'hidden', + // `showWhen` has only one reading — a predicate — so it renames. Its + // sibling `visible` has two on this surface and is answered in prose + // below; `disabled` already renames onto `readonly` above, which is the + // right target here because a field has `readonlyWhen`, not `disabledWhen` + // (#7832). + showWhen: 'visibleWhen', section: 'group', category: 'group', fieldset: 'group', component: 'widget', renderer: 'widget', control: 'widget', mimeTypes: 'accept', allowedTypes: 'accept', fileTypes: 'accept', @@ -485,6 +491,16 @@ export const FieldSchema = lazySchema(() => strictObject({ + '(ADR-0113). `required` is the WRITE contract and deliberately does not imply the column ' + 'constraint.', tracked: '`tracked` is not a field key — per-field timeline tracking is `trackHistory: true` (ADR-0052 §5b).', + // Prose rather than a rename, because this surface declares BOTH forms and + // the two answers have opposite polarity: renaming onto `visibleWhen` sends + // `visible: false` to a slot that wants a CEL string, and renaming onto + // `hidden` silently inverts the value the author already wrote. Naming both + // is the only answer that cannot be acted on wrongly (#7832 / #7816). + visible: + '`visible` is not a field key, and which key you want depends on the form: a static ' + + 'boolean is `hidden` — INVERTED, so `visible: false` is `hidden: true` — while a ' + + 'per-record CEL predicate is `visibleWhen` (shown only when TRUE). Its siblings are ' + + '`readonlyWhen` and `requiredWhen`.', }, }, { /** Identity */ diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index a7425a3458..decdc31b07 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1072,7 +1072,40 @@ export type ObjectExternalBindingParsed = z.infer` for a per-record predicate (FALSE hides ' + + 'that row\'s button).', + disabled: + '`disabled` is the CUSTOM row-action spelling (`actions[].disabled`). The per-record form ' + + 'here is `disabledWhen: ` (TRUE renders that row\'s button disabled); ' + + 'there is no boolean `disabled` — switch the affordance off with `enabled: false`.', + }, +}, { enabled: z.boolean().optional().describe( 'Object-level on/off for the generic affordance; same meaning as the bare boolean form. Omitted → managedBy bucket default.', ), @@ -1082,7 +1115,7 @@ export const RowCrudActionOverrideSchema = z.object({ disabledWhen: ExpressionInputSchema.optional().describe( 'Per-record CEL predicate; true → render the row button disabled for that record. Fail-soft.', ), -}).strict().describe('Boolean-or-predicates override for a built-in row CRUD affordance.'); +}).describe('Boolean-or-predicates override for a built-in row CRUD affordance.'); export type RowCrudActionOverride = z.input; /** Post-parse shape of {@link RowCrudActionOverride} — defaults applied, transforms run (ADR-0122). */ export type RowCrudActionOverrideParsed = z.infer; diff --git a/packages/spec/src/shared/visible-when-alias-guidance.test.ts b/packages/spec/src/shared/visible-when-alias-guidance.test.ts new file mode 100644 index 0000000000..1b71027cfd --- /dev/null +++ b/packages/spec/src/shared/visible-when-alias-guidance.test.ts @@ -0,0 +1,228 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7832 — `visible` / `showWhen` / `disabled` on the `visibleWhen` shapes. + * + * `ui/action.zod.ts` has carried this table in the OTHER direction since #3746: + * on an action, `visible` / `disabled` are the canonical keys and the aliases + * run `visibleWhen → visible`, `showWhen → visible`, `disabledWhen → disabled`. + * The reverse direction — an author who learned the action vocabulary writing + * it on a `visibleWhen` shape — was curated on some surfaces and bare on + * others, and nothing said which was which. This file is that inventory, + * asserted rather than asserted-in-prose. + * + * ## Why the answers differ per surface, and why that is not inconsistency + * + * The rule the six cases below encode: + * + * - ONE landing key ⇒ **alias** (a rename is unambiguous and reads best). + * - TWO landing keys, a boolean and a predicate ⇒ **guidance prose** naming + * both. A rename has to pick one, and picking wrong just relocates the + * confusion — #7816's own note, and the reason `RowCrudActionOverride`'s + * `visible` points at `enabled` *and* `visibleWhen` rather than at either. + * On `FieldSchema` the two answers additionally have OPPOSITE POLARITY + * (`visible: false` is `hidden: true`), so a rename onto `hidden` would + * have the author ship the inverse of what they wrote. + * - NO landing key ⇒ **nothing**. An alias whose target the shape does not + * declare is the ledger's finding-12 shape: the author is told to write + * the one key guaranteed to be rejected next. `alias-integrity.test.ts` + * fails such a row outright; the cases below record WHICH surfaces are in + * that bucket so the next sweep does not read them as missed. + * + * ## Acceptance is untouched + * + * Every key probed here is rejected before this change and rejected after it — + * only the message differs. That is the whole card: `strictObject` is + * `z.object(shape, { error }).strict()`, so converting a plain `.strict()` to + * it (`RowCrudActionOverrideSchema`) adds an error map and changes no verdict. + * Section 3 pins that directly. + */ + +import { describe, it, expect } from 'vitest'; + +import { RowCrudActionOverrideSchema } from '../data/object.zod'; +import { FieldSchema, SelectOptionSchema } from '../data/field.zod'; +import { FormFieldSchema, FormSectionSchema } from '../ui/view.zod'; +import { PageComponentSchema } from '../ui/page.zod'; + +/** + * The `unrecognized_keys` message for `value`, or a loud failure. + * + * Deliberately narrowed to that ONE issue code: several of these probes are + * minimal bodies that also miss a required key, and a whole-error stringify + * would let an assertion pass on text from an unrelated issue. + */ +function unknownKeyMessage( + schema: { safeParse: (v: unknown) => { success: boolean; error?: unknown } }, + value: unknown, +): string { + const r = schema.safeParse(value); + expect(r.success, `expected REJECTION, got a successful parse of ${JSON.stringify(value)}`).toBe(false); + const issues = (r.error as { issues?: Array<{ code?: string; message?: string }> }).issues ?? []; + const hit = issues.find((i) => i.code === 'unrecognized_keys'); + expect(hit, `no \`unrecognized_keys\` issue in ${JSON.stringify(issues)}`).toBeDefined(); + return hit?.message ?? ''; +} + +/** Minimal bodies that reach each surface's unknown-key path. */ +const FIELD = { name: 'probe', type: 'text' } as const; +const OPTION = { label: 'A', value: 'aa' } as const; +const SECTION = { fields: [] } as const; +const COMPONENT = { type: 'text' } as const; +const FORM_FIELD = { field: 'probe' } as const; + +// =========================================================================== +// 1. The surfaces this card CHANGED +// =========================================================================== +describe('#7832 — the curation added here', () => { + describe('`RowCrudActionOverrideSchema` — was a bare zod message, surface unnamed', () => { + it('names the surface at all (it did not before — `Unrecognized key: "visible"` was the whole message)', () => { + const m = unknownKeyMessage(RowCrudActionOverrideSchema, { visible: true }); + expect(m).toContain('this row CRUD override'); + }); + + it('`visible` names BOTH landing keys — the boolean one first, per #7816', () => { + const m = unknownKeyMessage(RowCrudActionOverrideSchema, { visible: true }); + expect(m).toMatch(/`enabled: false`/); + expect(m).toMatch(/`visibleWhen/); + // The claim the prose makes about the boolean form has to be true. + expect(RowCrudActionOverrideSchema.safeParse({ enabled: false }).success).toBe(true); + }); + + it('`disabled` points at `disabledWhen`, and says the boolean form is `enabled`', () => { + const m = unknownKeyMessage(RowCrudActionOverrideSchema, { disabled: true }); + expect(m).toMatch(/`disabledWhen/); + expect(m).toContain('`enabled: false`'); + expect(RowCrudActionOverrideSchema.safeParse({ disabledWhen: 'record.locked' }).success).toBe(true); + }); + + it('`showWhen` RENAMES — one reading, so it gets the rename channel, not prose', () => { + const m = unknownKeyMessage(RowCrudActionOverrideSchema, { showWhen: 'record.x' }); + expect(m).toContain('Did you mean `showWhen` → `visibleWhen`?'); + expect(RowCrudActionOverrideSchema.safeParse({ visibleWhen: 'record.x' }).success).toBe(true); + }); + }); + + describe('`FieldSchema` — `visible` and `showWhen` had no hint at all', () => { + it('`showWhen` renames onto `visibleWhen`', () => { + const m = unknownKeyMessage(FieldSchema, { ...FIELD, showWhen: 'record.x' }); + expect(m).toContain('Did you mean `showWhen` → `visibleWhen`?'); + expect(FieldSchema.safeParse({ ...FIELD, visibleWhen: 'record.x' }).success).toBe(true); + }); + + it('`visible` names both forms AND the inversion — the trap a rename would spring', () => { + const m = unknownKeyMessage(FieldSchema, { ...FIELD, visible: false }); + expect(m).toContain('`hidden`'); + expect(m).toContain('`visibleWhen`'); + expect(m).toContain('INVERTED'); + // Both keys the prose names are really accepted here. + expect(FieldSchema.safeParse({ ...FIELD, hidden: true }).success).toBe(true); + expect(FieldSchema.safeParse({ ...FIELD, visibleWhen: 'record.x' }).success).toBe(true); + }); + }); + + describe('`FormFieldSchema` — the one view/page shape that declares a landing key for `disabled`', () => { + it('`disabled` renames onto `readonly`', () => { + const m = unknownKeyMessage(FormFieldSchema, { ...FORM_FIELD, disabled: true }); + expect(m).toContain('Did you mean `disabled` → `readonly`?'); + expect(FormFieldSchema.safeParse({ ...FORM_FIELD, readonly: true }).success).toBe(true); + }); + + it('…and the row is filed HERE, not on the shared table — the siblings would be lying', () => { + // `VISIBILITY_STRICT_OPTIONS` is shared with the two shapes below, and + // neither declares `readonly`. This is the assertion that keeps a future + // tidy-up from hoisting the alias into the shared options. + expect(FormSectionSchema.safeParse({ ...SECTION, readonly: true }).success).toBe(false); + expect(PageComponentSchema.safeParse({ ...COMPONENT, readonly: true }).success).toBe(false); + }); + }); +}); + +// =========================================================================== +// 2. The surfaces that were ALREADY compliant — pinned so a sweep can tell +// "already answered" from "nobody got to it" +// =========================================================================== +describe('#7832 — already curated before this card, and why no row was added', () => { + it('`SelectOptionSchema` already renames `visible` and `showWhen` onto `visibleWhen`', () => { + expect(unknownKeyMessage(SelectOptionSchema, { ...OPTION, visible: true })) + .toContain('Did you mean `visible` → `visibleWhen`?'); + expect(unknownKeyMessage(SelectOptionSchema, { ...OPTION, showWhen: 'record.x' })) + .toContain('Did you mean `showWhen` → `visibleWhen`?'); + }); + + it.each([ + ['FormSectionSchema', FormSectionSchema, SECTION], + ['PageComponentSchema', PageComponentSchema, COMPONENT], + ['FormFieldSchema', FormFieldSchema, FORM_FIELD], + ])('%s answers `visible` / `showWhen` through the ADR-0089 guidance SET, not an alias', (_n, schema, base) => { + for (const key of ['visible', 'showWhen']) { + const m = unknownKeyMessage(schema, { ...base, [key]: 'record.x' }); + expect(m).toContain('the canonical key is `visibleWhen` (ADR-0089)'); + // The set consumes the key and `continue`s, so the rename channel never + // runs for it — which is precisely why adding an alias for either key on + // these three surfaces would be dead code, not a second opinion. + expect(m).not.toContain('Did you mean'); + } + }); +}); + +// =========================================================================== +// 3. Where NO row was added, because the target key does not exist +// =========================================================================== +describe('#7832 — the deliberate gaps (an alias here would name a key the shape rejects)', () => { + it.each([ + ['SelectOptionSchema', SelectOptionSchema, OPTION], + ['FormSectionSchema', FormSectionSchema, SECTION], + ['PageComponentSchema', PageComponentSchema, COMPONENT], + ])('%s declares no `disabledWhen` / `disabled` / `readonly`, so `disabled` stays uncurated', (_n, schema, base) => { + // Rejected — loudly, with the surface named — just without a pointer, + // because there is nothing on this shape to point at. If any of these ever + // gains a disabled-ish key, this assertion fails and the row becomes owed. + for (const target of ['disabledWhen', 'disabled', 'readonly']) { + expect( + schema.safeParse({ ...base, [target]: 'x' }).success, + `${_n} now declares \`${target}\` — file the \`disabled\` row`, + ).toBe(false); + } + expect(unknownKeyMessage(schema, { ...base, disabled: true })).toContain('`disabled`'); + }); + + it('`FieldSchema.disabled` was already pointed at `readonly`, which is right — a field has `readonlyWhen`, not `disabledWhen`', () => { + expect(unknownKeyMessage(FieldSchema, { ...FIELD, disabled: true })) + .toContain('Did you mean `disabled` → `readonly`?'); + expect(FieldSchema.safeParse({ ...FIELD, disabledWhen: 'record.x' }).success).toBe(false); + expect(FieldSchema.safeParse({ ...FIELD, readonlyWhen: 'record.x' }).success).toBe(true); + }); +}); + +// =========================================================================== +// 4. Acceptance is byte-identical — the constraint this card was scoped under +// =========================================================================== +describe('#7832 — no acceptance change', () => { + it('`RowCrudActionOverrideSchema` still accepts exactly its three declared keys', () => { + expect(RowCrudActionOverrideSchema.safeParse({}).success).toBe(true); + expect( + RowCrudActionOverrideSchema.safeParse({ + enabled: true, + visibleWhen: 'record.status != "closed"', + disabledWhen: 'record.locked', + }).success, + ).toBe(true); + }); + + it('…and still REJECTS every key it rejected before the error map was attached', () => { + for (const key of ['visible', 'disabled', 'showWhen', 'hidden', 'nonsenseKey']) { + expect( + RowCrudActionOverrideSchema.safeParse({ [key]: true }).success, + `\`${key}\` must stay rejected — this card curates messages, it does not widen the shape`, + ).toBe(false); + } + }); + + it('the surfaces that gained prose/aliases did not gain KEYS', () => { + for (const key of ['visible', 'showWhen']) { + expect(FieldSchema.safeParse({ ...FIELD, [key]: true }).success).toBe(false); + } + expect(FormFieldSchema.safeParse({ ...FORM_FIELD, disabled: true }).success).toBe(false); + }); +}); diff --git a/packages/spec/src/ui/view.zod.ts b/packages/spec/src/ui/view.zod.ts index cae392e1d1..15e3ba6b42 100644 --- a/packages/spec/src/ui/view.zod.ts +++ b/packages/spec/src/ui/view.zod.ts @@ -1760,7 +1760,20 @@ const FormFieldBaseSchema = lazySchema(() => { disclosure: z.enum(['inline', 'popover']).optional().describe('Composite rendering: inline bordered box (default) or a summary line + gear popover (progressive disclosure).'), }; return z.object(shape, { - error: strictObjectError({ ...VISIBILITY_STRICT_OPTIONS, extraKeys: ['fields'] }, shape), + error: strictObjectError({ + ...VISIBILITY_STRICT_OPTIONS, + extraKeys: ['fields'], + // The one member of the `visibleWhen` family that can answer `disabled` + // with a key of its own (#7832). `VISIBILITY_STRICT_OPTIONS` is shared + // with `FormSectionSchema` and `PageComponentSchema`, and neither of those + // declares a read-only or disabled slot, so this row belongs HERE rather + // than in the shared table — filed there it would name a key two of its + // three surfaces do not accept. `visible` / `showWhen` need nothing: they + // match `VISIBILITY_KEY_PATTERN` and are already answered by the shared + // ADR-0089 prescription, which consumes the key before the rename channel + // is consulted, so an alias for either would be dead on arrival. + aliases: { disabled: 'readonly' }, + }, shape), }); });