diff --git a/.changeset/8069-field-rule-fault-direction-declared.md b/.changeset/8069-field-rule-fault-direction-declared.md new file mode 100644 index 0000000000..9ff69391f0 --- /dev/null +++ b/.changeset/8069-field-rule-fault-direction-declared.md @@ -0,0 +1,48 @@ +--- +'@object-ui/core': minor +--- + +A field-rule predicate the author **declared and left blank** is no longer silent, and the +three fault directions in `resolveFieldRuleState` are now named and documented instead of +being bare positional booleans (objectui#8069). **No fallback value moves.** + +**The blank hole.** `''` and `' '` are authorable — `ExpressionWireSchema` is a bare +`z.string()` with no `.min(1)`, and `resolveFieldRuleState`'s own guard is `!= null`, so a +blank predicate passes both. `evalFieldPredicate` then returned the caller's fallback on its +first line, *before* `warnPredicateFailure` or `onFault` could fire. That is a third state, +not a spelling of either neighbour: the key is present (so it is not "the author wrote no +rule") and nothing evaluates (so no engine fault is raised). The one state an author reaches +by *starting* a rule and not finishing it was the one state that said nothing at all — which +is exactly what objectui#4051 / objectstack#5149 ruled out for every other fault. + +Both spellings now report `[blank] the predicate is declared but empty — nothing to evaluate` +through the same single reporting site as every other fault, on both channels (the built-in +`console.warn` and the `onFault` passback, so the fault-probing callers that pass +`warn: false` are not silenced either). **Every verdict is unchanged**, including the +envelope spelling: `{ source: '' }` used to reach the engine and come back +"AST-only evaluation not yet supported; persist `source`" and `{ source: ' ' }` +"Unexpected token: EOF" — two misleading reasons for one author mistake, both already +resolving to the same fallback this change keeps. Blankness is decided by +`isBlankPredicateText` (`evaluator/declaredPredicate.ts`), the repo's one definition of that +question since objectui#3960, now exported for this second consumer rather than copied. + +For this one fault class the once-per-predicate dedupe key joins the caller's **locator**: a +blank predicate has no distinguishing text, so every blank rule in an app shares the key +`""` and the first one would silence every other author's. Non-blank keys are unchanged. + +**The named directions.** `resolveFieldRuleState` passed `true` / `false` / `false` as bare +third arguments and answered the adjacent "no rule declared" case with the *same* literal — +so every permissive value was written twice, and the "the rule broke" answer was chosen by +aligning it with the "the rule is absent" answer beside it. Six module-private constants now +spell the two questions apart (`*_WHEN_FAULTED` / `*_WHEN_ABSENT`), with one docblock +recording the direction, the fact that all three point the permissive way so a single +mistyped column yields a form that shows more, locks less and demands less at once, and what +the history does and does not record about why (objectui#1578 and ADR-0036 both carry a +per-key "a fault is safe" rationale; no commit puts the case where all three faults arrive +from one typo). `evalFieldPredicate`'s docblock gains the call-site policy table — five +distinct fault policies share this one helper, two of which detect a fault by calling it +twice with *opposite* fallbacks and therefore depend on `fallback` staying freely +specifiable. + +Whether the direction belongs in the authored contract, and whether a loud-but-safe middle +should exist, remain open on objectui#8069. diff --git a/packages/core/src/evaluator/__tests__/fieldRules.test.ts b/packages/core/src/evaluator/__tests__/fieldRules.test.ts index e442ca23d9..19be61d89e 100644 --- a/packages/core/src/evaluator/__tests__/fieldRules.test.ts +++ b/packages/core/src/evaluator/__tests__/fieldRules.test.ts @@ -248,11 +248,15 @@ describe('failure diagnostics — loud fail-open (objectstack#5149, appeal 2)', expect(String(warn.mock.calls[0][0])).toContain(JSON.stringify(pred)); }); - it('does not warn for a healthy, absent, or blank predicate', () => { + // A BLANK predicate used to be listed here too — it was the negative + // baseline objectui#8069 measured, and it is now loud; the case moved to the + // blank-predicate describe below rather than being edited in place, because + // what it pinned is the branch that card removes. An ABSENT predicate keeps + // its silence: nothing was authored, so there is nothing to report. + it('does not warn for a healthy or absent predicate', () => { expect(evalFieldPredicate("record.ok_5149 == 'y'", { ok_5149: 'y' }, false)).toBe(true); expect(evalFieldPredicate(undefined, {}, true)).toBe(true); expect(evalFieldPredicate(null, {}, false)).toBe(false); - expect(evalFieldPredicate(' ', {}, true)).toBe(true); expect(warn).not.toHaveBeenCalled(); }); @@ -356,3 +360,187 @@ describe('failure diagnostics — loud fail-open (objectstack#5149, appeal 2)', expect(String(warn.mock.calls[0][0])).toContain('[throw] engine exploded'); }); }); + +/** + * objectui#8069, deliverable 1. The three fault directions are now named + * constants; these pin the VALUES through the public surface, so a future edit + * that moves one goes red here instead of shipping. ⛔ They are not an + * endorsement of the direction — that is the card's open question. Each case + * pairs the FAULTED verdict with the ABSENT one to keep visible the fact the + * card turns on: the two questions have equal answers today, and only because + * one was copied from the other. + */ +describe('fault directions — what a BROKEN rule decides (objectui#8069)', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => warn.mockRestore()); + + const broken = 'unbound_direction_8069 =='; + + it('a faulted visibleWhen SHOWS the field — same verdict as no visibleWhen at all', () => { + expect(resolveFieldRuleState({ visibleWhen: broken }, {}, {}).visible).toBe(true); + expect(resolveFieldRuleState({}, {}, {}).visible).toBe(true); + }); + + it('a faulted readonlyWhen leaves the field EDITABLE — same verdict as no readonlyWhen', () => { + expect(resolveFieldRuleState({ readonlyWhen: broken }, {}, {}).readonly).toBe(false); + expect(resolveFieldRuleState({}, {}, {}).readonly).toBe(false); + }); + + it('a faulted requiredWhen demands NOTHING — same verdict as no requiredWhen', () => { + expect(resolveFieldRuleState({ requiredWhen: broken }, {}, {}).required).toBe(false); + expect(resolveFieldRuleState({}, {}, {}).required).toBe(false); + }); + + it('all three compose from ONE broken predicate: shows more, locks less, demands less', () => { + // The card's thesis in one assertion — the three faults do not cancel. + expect( + resolveFieldRuleState( + { visibleWhen: broken, readonlyWhen: broken, requiredWhen: broken }, + {}, + {}, + ), + ).toEqual({ visible: true, readonly: false, required: false }); + }); +}); + +/** + * objectui#8069, deliverable 2. A predicate the author DECLARED and left blank + * is a third state: the key is present, so it is not "no rule", and nothing + * evaluates, so no engine fault is raised. It used to return the permissive + * fallback before any warning could fire — silent, which is the one thing + * objectui#4051 / objectstack#5149 ruled out. The VERDICT is deliberately + * unchanged; only the silence is. + * + * Every case below uses a UNIQUE context locator: for this class the locator + * joins the once-per-predicate dedupe key (a blank predicate has no + * distinguishing text), so a reused locator would spend its warning earlier. + */ +describe('blank predicate — declared but empty (objectui#8069)', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => warn.mockRestore()); + + it('a whitespace-only bare string warns instead of defaulting in silence', () => { + expect( + evalFieldPredicate(' ', {}, true, undefined, undefined, { context: 'blank_ws_8069' }), + ).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + const msg = String(warn.mock.calls[0][0]); + expect(msg).toContain('[blank]'); + expect(msg).toContain('blank_ws_8069'); + }); + + it('the empty-string spelling warns too', () => { + expect(evalFieldPredicate('', {}, true, undefined, undefined, { context: 'blank_empty_8069' })).toBe( + true, + ); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain('[blank]'); + }); + + it('the ENVELOPE spelling reports the same reason, not the engine parse fault it used to', () => { + // Before: `{ source: '' }` reached the engine and came back + // "AST-only evaluation not yet supported; persist `source`", and + // `{ source: ' ' }` came back "Unexpected token: EOF" — two different + // reasons for one author mistake, neither of them naming it. + expect( + evalFieldPredicate({ dialect: 'cel', source: ' ' }, {}, true, undefined, undefined, { + context: 'blank_envelope_8069', + }), + ).toBe(true); + const msg = String(warn.mock.calls[0][0]); + expect(msg).toContain('[blank]'); + expect(msg).not.toContain('AST-only'); + expect(msg).not.toContain('Unexpected token'); + }); + + it('the VERDICT is unchanged — a blank predicate still returns the caller\'s fallback either way', () => { + expect( + evalFieldPredicate(' ', {}, true, undefined, undefined, { context: 'blank_verdict_t_8069' }), + ).toBe(true); + expect( + evalFieldPredicate(' ', {}, false, undefined, undefined, { context: 'blank_verdict_f_8069' }), + ).toBe(false); + }); + + it('`onFault` carries the blank reason, so a caller that silenced the warning is not silenced', () => { + // The fault-probing callers (`evalCel`, `ExpressionEvaluator` under + // `throwOnError`) pass `warn: false` and print their own line; without the + // passback a blank predicate would stay silent for exactly those surfaces. + const reasons: string[] = []; + evalFieldPredicate(' ', {}, true, undefined, undefined, { + warn: false, + onFault: (r) => reasons.push(r), + }); + expect(warn).not.toHaveBeenCalled(); + expect(reasons).toHaveLength(1); + expect(reasons[0]).toContain('[blank]'); + }); + + it('two blank predicates at DIFFERENT locators each warn — text alone would silence the second', () => { + evalFieldPredicate('', {}, true, undefined, undefined, { context: 'blank_site_A_8069' }); + evalFieldPredicate('', {}, true, undefined, undefined, { context: 'blank_site_B_8069' }); + expect(warn).toHaveBeenCalledTimes(2); + }); + + it('the SAME blank predicate at the same locator still warns once across re-renders', () => { + const diag = { context: 'blank_rerender_8069' }; + evalFieldPredicate(' ', {}, true, undefined, undefined, diag); + evalFieldPredicate(' ', {}, true, undefined, undefined, diag); + evalFieldPredicate(' ', {}, true, undefined, undefined, diag); + expect(warn).toHaveBeenCalledTimes(1); + }); + + // ── controls: a control that reds when the subject reds is not a control ── + + it('control — an ABSENT predicate stays silent and keeps its verdict', () => { + expect(evalFieldPredicate(undefined, {}, true, undefined, undefined, { context: 'absent_8069' })).toBe( + true, + ); + expect(evalFieldPredicate(null, {}, false, undefined, undefined, { context: 'absent_8069' })).toBe( + false, + ); + expect(warn).not.toHaveBeenCalled(); + }); + + it('control — a HEALTHY predicate is silent and its verdict ignores the fallback', () => { + const pred = "record.state_8069 == 'paid'"; + expect(evalFieldPredicate(pred, { state_8069: 'paid' }, false)).toBe(true); + expect(evalFieldPredicate(pred, { state_8069: 'paid' }, true)).toBe(true); + expect(evalFieldPredicate(pred, { state_8069: 'draft' }, true)).toBe(false); + expect(evalFieldPredicate(pred, { state_8069: 'draft' }, false)).toBe(false); + expect(warn).not.toHaveBeenCalled(); + }); + + it('control — a genuinely BROKEN predicate still warns with the engine reason, not [blank]', () => { + const pred = 'still_faults_8069 =='; + expect(evalFieldPredicate(pred, {}, true)).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + const msg = String(warn.mock.calls[0][0]); + expect(msg).toContain('Reason: ['); + expect(msg).not.toContain('[blank]'); + }); + + it('resolveFieldRuleState: a blank visibleWhen is reported with its field locator, verdict unchanged', () => { + const state = resolveFieldRuleState( + { visibleWhen: ' ' }, + { status: 'x' }, + {}, + undefined, + undefined, + "field 'amount_8069'", + ); + // The `!= null` guard lets `''` / `' '` through — that is how the blank + // reached the permissive fallback in the first place. + expect(state.visible).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + const msg = String(warn.mock.calls[0][0]); + expect(msg).toContain("visibleWhen of field 'amount_8069'"); + expect(msg).toContain('[blank]'); + }); +}); diff --git a/packages/core/src/evaluator/declaredPredicate.ts b/packages/core/src/evaluator/declaredPredicate.ts index 24fef85432..5b19815ba5 100644 --- a/packages/core/src/evaluator/declaredPredicate.ts +++ b/packages/core/src/evaluator/declaredPredicate.ts @@ -25,8 +25,16 @@ import { toPredicateInput } from './predicateInput.js'; * return true`), `evalRowPredicate` (`listConditional.ts`) — brought to the one * place that answers "is there a condition at all?", so the two halves cannot * disagree about the same blank. + * + * Exported since objectui#8069 for its second consumer, `evalFieldPredicate` + * (`evaluator/fieldRules.ts`), which is the entry this docblock's list did NOT + * name because it did not apply the rule: it trimmed the STRING spelling only, + * so `{ dialect: 'cel', source: ' ' }` went to the engine and came back as a + * parse fault while `' '` returned the caller's fallback in silence. Reusing + * this definition rather than writing a fourth `trim()` is the whole point of + * the paragraph above. */ -function isBlankPredicateText(value: unknown): boolean { +export function isBlankPredicateText(value: unknown): boolean { if (typeof value === 'string') return value.trim() === ''; if (value !== null && typeof value === 'object') { const source = (value as { source?: unknown }).source; diff --git a/packages/core/src/evaluator/fieldRules.ts b/packages/core/src/evaluator/fieldRules.ts index fb3a940f54..6c005f69a1 100644 --- a/packages/core/src/evaluator/fieldRules.ts +++ b/packages/core/src/evaluator/fieldRules.ts @@ -62,10 +62,23 @@ * logs `treating the field as LOCKED` instead). The *default* stays * fail-open on purpose — flipping it is a shipped-behavior change tracked * separately in objectstack#5149 (appeal 1, undecided). + * + * A predicate the author DECLARED and left BLANK is loud too, since + * objectui#8069 — and it did not used to be. It is a third state, not a + * spelling of either neighbour: the key is present (so it is not "no rule") + * and nothing evaluates (so no engine fault is raised), and it used to return + * the caller's permissive fallback before any warning could fire. `''` and + * `' '` are authorable — `ExpressionWireSchema` is a bare `z.string()`, and + * `resolveFieldRuleState`'s own guard is `!= null` — so the one state an author + * reaches by *starting* a rule and not finishing it was the one state that said + * nothing at all. Both spellings now report `[blank]` through the same single + * site as every other fault; the VERDICT is unchanged for every input. */ import { ExpressionEngine } from '@objectstack/formula'; import type { Expression } from '@objectstack/spec'; +import { isBlankPredicateText } from './declaredPredicate.js'; + /** A field-rule predicate as authored in metadata. */ export type FieldRulePredicate = string | { dialect?: string; source: string }; @@ -109,12 +122,29 @@ export interface FieldPredicateDiagnostic { const warnedPredicates = new Set(); +/** + * The `reason` reported for a predicate the author DECLARED and left blank. + * + * Tagged like an engine reason (`[parse]`, `[type]`, `[throw]`) because it + * travels the same two channels — the built-in warning and the `onFault` + * passback — and a caller that routes on the tag must be able to tell this + * apart from a predicate that faulted with text in it: the fix for a blank one + * is to finish it or delete the key, never to debug its syntax. + */ +const BLANK_PREDICATE_REASON = '[blank] the predicate is declared but empty — nothing to evaluate'; + /** * One-time warning for a predicate that could not be evaluated. Deduped per * predicate TEXT (dialect + source): a broken predicate is re-evaluated on * every render/keystroke, and the point is one loud line, not a scrolling * wall. The dedupe key is JSON-encoded — never a control-character separator * (objectstack#5450 made a sibling of this file binary to grep that way). + * + * ⚠️ One class joins the LOCATOR to that key: a BLANK predicate. The text is + * what identifies the authoring site for every other fault — it carries the + * typo — but every blank predicate in an app shares the key `""`, so text + * alone would let the first blank rule silence every other author's. Non-blank + * keys are unchanged (objectui#8069). */ function warnPredicateFailure( expr: Expression, @@ -122,7 +152,11 @@ function warnPredicateFailure( reason: string, context?: string, ): void { - const key = JSON.stringify([expr.dialect, expr.source]); + // `Expression['source']` is optional in the spec; an absent source is blank + // by the same rule as a whitespace-only one, and lands in the same key shape. + const key = expr.source?.trim() + ? JSON.stringify([expr.dialect, expr.source]) + : JSON.stringify([expr.dialect, expr.source, context ?? '']); if (warnedPredicates.has(key)) return; warnedPredicates.add(key); console.warn( @@ -138,12 +172,52 @@ function warnPredicateFailure( /** * Evaluate a field-rule CEL predicate against a record. * + * ## `fallback` is a per-CALL-SITE policy, not a default this helper owns + * + * The parameter is required and carries no default value, so every call site + * states a direction of its own — and the directions in this repo do not + * agree. Five distinct fault policies share this one helper today, which is + * readable nowhere but here (objectui#8069): + * + * 1. **Fixed permissive, paired with an equal "no rule" literal.** + * {@link resolveFieldRuleState} (`true` / `false` / `false`) and + * `resolveVisibleOptions` (`evaluator/optionRules.ts`, `true`). A fault + * produces the permissive verdict, indistinguishable from the verdict for + * "the author wrote no rule" — see the `…_FAULTED` / `…_ABSENT` note below. + * 2. **Fixed permissive, unguarded — ONE literal answers absent *and* + * faulted.** Every visibility call site outside core: the form renderer + * (`components/renderers/form/form.tsx`), console's `FormPage`, app-shell's + * `ScreenView`, `plugin-form`'s `WizardForm`. All of them pass `true`, and + * none has a separate absent branch at all. + * 3. **Caller-parameterised.** `evalRowPredicate`'s single-eval fast route + * (`evaluator/listConditional.ts`) forwards `opts.fallback`, so the + * direction is the mounting surface's to state. + * 4. **Divergence probe → fault FLAG.** `evalCel` + * (`evaluator/listConditional.ts`) calls this helper once per direction with + * `warn: false` + `onFault`: a verdict that tracks the fallback in BOTH runs + * is a fault, and the caller emits its own labelled warning and returns its + * own fallback. + * 5. **Divergence probe → THROW.** `ExpressionEvaluator.evaluateCelCondition` + * under `throwOnError` runs the same two-call trick and converts the + * disagreement into `throw new Error('CEL predicate failed to evaluate: …')`. + * + * ⚠️ The last two policies exist **because** `fallback` is free to specify: they + * detect a fault by disagreeing with themselves. Any proposal to fix a + * direction *inside* this helper — objectui#8069's open question — removes the + * mechanism they are built on, so read them before writing one. + * * @param pred The `visibleWhen` / `readonlyWhen` / `requiredWhen` predicate. * @param record The live form values (overlays prior persisted record). - * @param fallback Value to return when the predicate is absent or fails to - * evaluate. Pick the *safe* default for the caller: - * `false` for readonly/required (don't lock/block on error), - * `true` for visibility (don't hide on error). + * @param fallback Value to return when the predicate is ABSENT (`null` / + * `undefined`), and — separately — the value returned when a + * present predicate cannot be evaluated. ⚠️ Those are two + * questions this one parameter answers with one value; a + * caller that wants different answers must branch before the + * call, as {@link resolveFieldRuleState} does. The historic + * advice is to pick the *safe* default (`false` for + * readonly/required — don't lock/block on error; `true` for + * visibility — don't hide on error); whether "safe" is the + * right axis is objectui#8069's open question. * @param previous The prior persisted record, if any (for `previous.*` refs). * @param scope Extra top-level scope variables bound alongside `record` — * e.g. `{ parent }` so an inline line-item cell can reference @@ -162,26 +236,39 @@ export function evalFieldPredicate( scope?: Record, diagnostic?: FieldPredicateDiagnostic, ): boolean { - if (pred == null || (typeof pred === 'string' && !pred.trim())) return fallback; + if (pred == null) return fallback; const expr = toExpression(pred); - // The two fault sources — a not-ok verdict and a throw that slipped past the - // engine's "never throws" contract — converge on one reason string and one - // reporting site below, so the built-in warning and the `onFault` passback - // can never describe the failure differently. + // The fault sources — a blank predicate, a not-ok verdict, and a throw that + // slipped past the engine's "never throws" contract — converge on one reason + // string and one reporting site below, so the built-in warning and the + // `onFault` passback can never describe the failure differently. let reason: string | undefined; let value = fallback; - try { - const res = ExpressionEngine.evaluate(expr, { - record, - previous, - ...(scope ? { extra: scope } : {}), - }); - // Parse error, type error, unbound identifier, engine fault … — every - // not-ok verdict resolves to the fallback, but never silently (#5149). - if (!res.ok) reason = `[${res.error.kind}] ${res.error.message}`; - else value = res.value === true; - } catch (err) { - reason = `[throw] ${err instanceof Error ? err.message : String(err)}`; + if (isBlankPredicateText(pred)) { + // Declared-and-blank, in EITHER spelling. Answered here rather than by the + // engine for two reasons: the bare-string spelling never reached the engine + // at all (this line used to `return fallback` before any warning could + // fire — objectui#8069), and the envelope spelling reached it and came back + // with `AST-only evaluation not yet supported; persist \`source\`` for + // `{ source: '' }`, a reason that sends the author looking for a + // serialization bug. `isBlankPredicateText` is the repo's one definition of + // this question (objectui#3960) — a fourth local `trim()` here is exactly + // the drift it was consolidated to stop. + reason = BLANK_PREDICATE_REASON; + } else { + try { + const res = ExpressionEngine.evaluate(expr, { + record, + previous, + ...(scope ? { extra: scope } : {}), + }); + // Parse error, type error, unbound identifier, engine fault … — every + // not-ok verdict resolves to the fallback, but never silently (#5149). + if (!res.ok) reason = `[${res.error.kind}] ${res.error.message}`; + else value = res.value === true; + } catch (err) { + reason = `[throw] ${err instanceof Error ? err.message : String(err)}`; + } } if (reason !== undefined) { if (diagnostic?.warn !== false) { @@ -195,6 +282,60 @@ export function evalFieldPredicate( return value; } +/** + * The verdict {@link resolveFieldRuleState} applies when a rule's predicate + * CANNOT BE EVALUATED — parse error, unbound identifier, engine fault, or a + * predicate the author left blank. + * + * ⚠️ These answer **"what should the form do when this rule is BROKEN?"** — + * *not* "what should the form do when the author declared NO rule?" That + * second question is answered separately, by the `…_ABSENT` set below. The two + * sets hold pairwise EQUAL values today, and that equality is a copy, not an + * implication: each `rules. != null` ternary answers the absent case with + * its own literal and hands `evalFieldPredicate` the same literal as the fault + * fallback beside it, so every permissive value is written twice and the "it + * broke" answer was chosen by aligning it with the "it is absent" answer next + * to it. Nothing was *inherited* from a shared default — `evalFieldPredicate`'s + * `fallback` parameter is required and has none (objectui#8069 measured both + * halves). + * + * All three directions are the PERMISSIVE one, so a single mistyped column in + * one authored predicate yields a form that shows more, locks less and demands + * less — three faults from one typo, composing rather than cancelling. That is + * objectui#8069's open question and it is deliberately NOT decided here. + * + * **What the history records** (read on full, unshallowed history — a shallow + * clone answers this with one commit and no warning): the direction *was* + * reasoned about, once, when the helper landed (objectui#1578). This module's + * head and ADR-0036 both record the same rationale — the fallbacks are "chosen + * so a fault is *safe*: `true` for visibility (don't hide content on error), + * `false` for required/readonly (don't block submit or lock a field on + * error)". What no commit records is the COMPOSITION: every recorded argument + * is per-key, and the case where all three faults arrive from one typo was + * never put. objectstack#5149 then removed the SILENCE and left the direction + * explicitly undecided ("appeal 1"). + * + * ⛔ These values are shipped behaviour. Changing one is not a refactor, it is + * objectui#8069's decision — and objectui#6958 leans on the `visibleWhen` half + * staying fail-open (a broken predicate must never silently null a stored + * column). + */ +const VISIBLE_WHEN_FAULTED = true; +const READONLY_WHEN_FAULTED = false; +const REQUIRED_WHEN_FAULTED = false; + +/** + * The verdict {@link resolveFieldRuleState} applies when the author declared NO + * rule for that key at all — the `: ` arm of each `!= null` ternary. + * + * Spelled apart from the `…_FAULTED` set above because they answer a different + * question, not because they differ: changing one of THESE changes what an + * unconditional field does, which is a louder and quite separate decision. + */ +const VISIBLE_WHEN_ABSENT = true; +const READONLY_WHEN_ABSENT = false; +const REQUIRED_WHEN_ABSENT = false; + /** * Resolve the effective `{ visible, readonly, required }` state for a field * given its conditional rules and the live record. Each `*When` rule, when @@ -243,14 +384,28 @@ export function resolveFieldRuleState( const visible = rules.visibleWhen != null - ? evalFieldPredicate(rules.visibleWhen, record, true, previous, scope, diag('visibleWhen')) - : true; + ? evalFieldPredicate( + rules.visibleWhen, + record, + VISIBLE_WHEN_FAULTED, + previous, + scope, + diag('visibleWhen'), + ) + : VISIBLE_WHEN_ABSENT; const readonly = statics.readonly === true || (rules.readonlyWhen != null - ? evalFieldPredicate(rules.readonlyWhen, record, false, previous, scope, diag('readonlyWhen')) - : false); + ? evalFieldPredicate( + rules.readonlyWhen, + record, + READONLY_WHEN_FAULTED, + previous, + scope, + diag('readonlyWhen'), + ) + : READONLY_WHEN_ABSENT); // Short-circuited, not evaluated-and-discarded: the verdict cannot depend on // the predicate, so running it would only spend an engine call per field per @@ -262,8 +417,15 @@ export function resolveFieldRuleState( ? false : statics.required === true || (rules.requiredWhen != null - ? evalFieldPredicate(rules.requiredWhen, record, false, previous, scope, diag('requiredWhen')) - : false); + ? evalFieldPredicate( + rules.requiredWhen, + record, + REQUIRED_WHEN_FAULTED, + previous, + scope, + diag('requiredWhen'), + ) + : REQUIRED_WHEN_ABSENT); return { visible, readonly, required }; }