Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .changeset/11262-blank-gates-diagnosed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
---
'@object-ui/types': minor
'@object-ui/core': patch
'@object-ui/plugin-form': patch
---

A blank gate predicate is diagnosed on the three paths that still drew it in
silence, and objectui's two gate mirrors whose protocol key refuses a blank now
refuse it too (objectui#11262, ADR-0137 D4 and D1). **No drawn verdict moves:** a
blank gate is still "no gate".

**Diagnosed, verdict unchanged (`@object-ui/core`, `@object-ui/plugin-form`).**
ADR-0137 D4 says a blank gate predicate is "diagnosed, never a silent `true`".
objectui#8069 diagnosed the `{ dialect: 'cel' }` route; three paths stayed silent,
and each now reports through that same guard (`evalFieldPredicate`'s `[blank]`
report, deduped per blank text and locator):

- `ExpressionEvaluator.evaluateCondition`'s legacy path: a bare `''`, a
whitespace-only string, and an envelope without `dialect` whose `source` is
blank. These are the values `SchemaRenderer`'s visibility legs pass through
raw. The answer is still `true` in every mode, `throwOnError` included. A
caller that passes `onFault` receives the `[blank]` reason there; every other
caller gets the built-in warning. A blank on either route lands on one dedupe
key, so `''` and `{ dialect: 'cel', source: '' }` print one line.
- `evalRowPredicate`: a bare blank string returned the caller's fallback before
anything could say so. It now takes the route its blank-envelope twin already
took, with the same fallback and the same report (labelled on the
`warnOnError` route).
- `sectionFields`' `attachVisibility` (every sectioned `object-form` arm): a
blank view-level predicate is still dropped, so the field draws with no gate,
and the drop is reported, naming the field. A runtime field that already
carries its own blank `visibleOn` keeps it. The form renderer reports that one
when it evaluates it.

**Refused at authoring (`@object-ui/types`, minor, a narrowing).** The protocol
declares an option's `visibleWhen` and a form view field's `visibleWhen` /
`visibleOn` as `EvaluatedExpressionInputSchema`, which refuses a blank predicate
(since `@objectstack/spec` 17.5.0). objectui's `SelectOptionSchema.visibleWhen`
and `FormFieldSchema.visibleOn` override those keys to keep objectui's wire, and
they still accepted `''`, whitespace and a blank envelope `source`. Both now
refuse them at the key, with the protocol's own sentence, through the check the
field-rule triad already carries. The accepted shape is unchanged: the same wire
options, by reference, and the parsed value is the authored one.

`BaseSchema`'s `visible` / `hidden` / `disabled` have no protocol counterpart,
so a blank there still parses and is only diagnosed.
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,27 @@ describe('objectui#8069 — FormPage refuses a faulted visibleWhen at submit', (
expect(warn.mock.calls.some((c: unknown[]) => String(c[0]).includes('[blank]'))).toBe(true);
});

it('control — a blank VIEW-level visibleWhen is a layout gate, not a field rule: it submits', async () => {
// The view's section entry is normalized before this page builds its rows,
// and a blank view-level predicate does not survive that step — "no gate",
// and nothing on this path refuses it.
it('control — a blank VIEW-level visibleWhen is a layout gate, not a field rule: it submits, and the blank is said', async () => {
// This page carries the view's predicate onto its row verbatim and judges
// it at render (`isFieldVisible`, through `evalFieldPredicate`), so a blank
// one reads as "no gate" there — the field is drawn and nothing on this
// path refuses it — and it is REPORTED there, on the `[blank]` channel,
// naming the field (ADR-0137 D4; restated with the diagnostic by
// objectui#11262). It does not pass through `sectionFields`.
//
// On `status`, not `notes`: the stored-blank row above already printed the
// one line for `''` under the locator both of a field's `visibleWhen`
// slots share on this page ("visibleWhen of field 'notes'"), and the
// one-time dedupe is module state for the whole file.
renderForm(objectWith({}), {
sections: [{ label: 'Basics', fields: ['title', 'priority', { field: 'notes', visibleWhen: '' }, 'status'] }],
sections: [{ label: 'Basics', fields: ['title', 'priority', 'notes', { field: 'status', visibleWhen: '' }] }],
});
await waitFor(() => expect(screen.getByLabelText('Notes')).toBeInTheDocument());
await waitFor(() => expect(screen.getByLabelText('Status')).toBeInTheDocument());
await submit();
await waitFor(() => expect(writes()).toHaveLength(1));
expect(
warn.mock.calls.some((c: unknown[]) => String(c[0]).includes('[blank]') && String(c[0]).includes("'status'")),
).toBe(true);
});

it('a faulted requiredWhen is NOT refused on the client — it is the server’s to refuse (D2)', async () => {
Expand Down
12 changes: 9 additions & 3 deletions content/docs/guide/metadata-diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,15 @@ blank) is not "no rule" — it is refused, at the first place that can see it
parse, as the protocol's field schema does. One already stored is refused at
submit, like any rule that cannot be evaluated: the client refuses a blank
`visibleWhen`, the server a blank `requiredWhen` / `readonlyWhen`. A blank
**gate** is different: a blank `visible`, `hidden` or `disabled` is still read
as "no gate", with a one-time `[blank]` warning in the console, and a form
view's own blank field `visibleWhen` is likewise read as no gate, never refused.
**gate** is different: at runtime it is read as "no gate" and never refused —
a blank `visible`, `hidden` or `disabled`, a page node's `visibleWhen`, or a
form view's own field `visibleWhen` — but it is not silent either: it is
reported once in the console with the `[blank]` reason (ADR-0137 D4). Where the
protocol already refuses a blank gate at authoring, the form schema does too: a
form field's view-level `visibleOn` and a select option's `visibleWhen` reject
a blank predicate at parse, as their protocol counterparts do. `visible`,
`hidden` and `disabled` have no protocol counterpart, so a blank there parses
and is only diagnosed.

The same is now true of a **component node's own gate** — `visibleWhen` on a
page component (and its `visible` / `visibleOn` / `visibility` / `hidden` /
Expand Down
78 changes: 51 additions & 27 deletions packages/core/src/evaluator/ExpressionEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,21 @@ export class ExpressionEvaluator {
condition = (condition as any).source as string;
}

// No condition → default to visible/enabled (undefined, null, '').
// A BLANK gate — `''`, whitespace-only text, or an envelope whose `source`
// is either (unwrapped just above) — is "no gate": visible/enabled, as it
// always was here. It is answered by the SAME diagnosed guard the
// `{ dialect: 'cel' }` route above uses (objectui#11262, ADR-0137 D4: a
// blank gate predicate is "diagnosed, never a silent `true`"), so a bare
// blank, a dialect-less blank envelope and a blank CEL envelope are one
// report on one dedupe key. This used to be two silent returns — an
// `if (!condition)` and a whitespace-only `trim()` — on the path
// `SchemaRenderer`'s visibility legs reach with the raw authored value.
if (isBlankPredicateText(condition)) {
return this.answerBlankGate(condition as string, options);
}

// No condition at all → default to visible/enabled (`undefined`, `null`).
// Not blank TEXT, so nothing was declared here to report.
if (!condition) {
return true;
}
Expand All @@ -372,9 +386,6 @@ export class ExpressionEvaluator {
}

const trimmed = condition.trim();
if (!trimmed) {
return true; // Whitespace-only → treat as "no condition".
}

// A condition is semantically a single boolean expression. When it's a
// `${...}` template, evaluate via the template path. Otherwise treat the
Expand Down Expand Up @@ -420,29 +431,7 @@ export class ExpressionEvaluator {
* syntax) throws; a genuine `false` never throws.
*/
private evaluateCelCondition(source: string, options: EvaluationOptions): boolean {
if (isBlankPredicateText(source)) {
// A BLANK gate: no predicate → visible/enabled, exactly as before
// (objectui#3850 / #3960's verdict), in EVERY mode — `throwOnError`
// included, since the author wrote nothing that could fault. What
// changed is that it is no longer SILENT (ADR-0137 D4: a blank gate
// predicate is "diagnosed, never a silent `true`"; objectui#8069). The
// report is `evalFieldPredicate`'s own `[blank]` one — the channel every
// other predicate fault already uses — so a caller that passed `onFault`
// receives it there (and prints its own, node-named line), and every
// other caller gets the deduped built-in warning. No engine call is made:
// a blank predicate never reaches the engine.
evalFieldPredicate(
source,
{},
true,
undefined,
undefined,
options.onFault
? { warn: false, onFault: options.onFault }
: { context: 'a CEL gate predicate, read as no gate' },
);
return true;
}
if (isBlankPredicateText(source)) return this.answerBlankGate(source, options);
const bag = this.context.toObject();
const rec = bag.record;
const record = (rec && typeof rec === 'object' && !Array.isArray(rec))
Expand Down Expand Up @@ -480,6 +469,41 @@ export class ExpressionEvaluator {
return asTrue;
}

/**
* Answer a BLANK gate predicate — the ONE diagnosed guard of
* {@link evaluateCondition}, on both of its routes (objectui#8069 put it on
* the `{ dialect: 'cel' }` route; objectui#11262 routes the legacy path's
* blank here too, so the two are one report rather than two).
*
* The verdict is `true` — no predicate → visible/enabled, exactly as before
* (objectui#3850 / #3960's verdict) — in EVERY mode, `throwOnError` included,
* since the author wrote nothing that could fault. What it is not is SILENT
* (ADR-0137 D4: a blank gate predicate is "diagnosed, never a silent
* `true`"). The report is `evalFieldPredicate`'s own `[blank]` one — the
* channel every other predicate fault already uses — so a caller that passed
* `onFault` receives it there (and prints its own, node-named line), and
* every other caller gets the deduped built-in warning. No engine call is
* made: a blank predicate never reaches the engine.
*
* The locator names a CEL gate on both routes: a blank has no syntax to put
* it in either dialect, and a bare-string predicate is CEL by the spec's
* contract. One locator is also what keeps `''` and `{ dialect: 'cel',
* source: '' }` on one dedupe key.
*/
private answerBlankGate(text: string, options: EvaluationOptions): true {
evalFieldPredicate(
text,
{},
true,
undefined,
undefined,
options.onFault
? { warn: false, onFault: options.onFault }
: { context: 'a CEL gate predicate, read as no gate' },
);
return true;
}

/**
* Update the context with new data
*/
Expand Down
166 changes: 166 additions & 0 deletions packages/core/src/evaluator/__tests__/blankGateDiagnosed-11262.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/**
* 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.
*/

/**
* objectui#11262 — ADR-0137 D4 on the two CORE paths that still answered a
* blank gate in silence after objectui#8069: a gate predicate that is blank is
* "diagnosed, never a silent `true`".
*
* 1. `ExpressionEvaluator.evaluateCondition`'s LEGACY path. objectui#8069
* diagnosed the `{ dialect: 'cel' }` route only; a bare string and an
* envelope without `dialect` took the legacy path, whose `if (!condition)`
* and whitespace-only `trim()` returned `true` before anything could say
* so. `SchemaRenderer`'s `visibleWhen` / `visible` / `visibleOn` /
* `visibility` legs hand it the raw authored value, so this is the path a
* page author reaches.
* 2. `evalRowPredicate`'s bare-blank fallback (`listConditional.ts`). A blank
* ENVELOPE already reached `evalFieldPredicate`'s `[blank]` report; a
* blank STRING returned the caller's fallback one line earlier.
*
* ## What each row pins
*
* The VERDICT does not move (D3/D4: a blank gate is "no gate") and the silence
* ends, so every row asserts both halves. The report is the one channel
* objectui#8069 put on the `dialect: 'cel'` route — `evalFieldPredicate`'s
* `[blank]` reason — which the rows read by its tag, never by its prose.
*
* The "one diagnosis point" rows are what distinguish ONE guard from a second
* blank test with its own reporter: a blank spelled on the legacy path and the
* same blank spelled on the CEL route land on the same dedupe key, so they
* print one line between them. A second reporter prints two.
*
* ## Fresh module graph per case
*
* The one-time warning dedupe is MODULE state and the `unit` project runs with
* `isolate: false`, so a blank spelling another file already warned about
* would read as silence here. `vi.resetModules()` + fresh imports give every
* case its own dedupe `Set` — the shape `fieldRuleFaults-8069.test.ts` uses.
*/

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';

let ExpressionEvaluator: typeof import('../ExpressionEvaluator.js').ExpressionEvaluator;
let evalRowPredicate: typeof import('../listConditional.js').evalRowPredicate;
let warn: ReturnType<typeof vi.spyOn>;

beforeEach(async () => {
vi.resetModules();
({ ExpressionEvaluator } = await import('../ExpressionEvaluator.js'));
({ evalRowPredicate } = await import('../listConditional.js'));
warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
});
afterEach(() => warn.mockRestore());

const warnings = (): string[] => warn.mock.calls.map((call: unknown[]) => String(call[0]));
const blankLines = (): string[] => warnings().filter((w) => w.includes('[blank]'));

/** The three blank spellings the triage names, on every path. */
const BLANKS = [
['an empty string', ''],
['a whitespace string', ' \t '],
["{ source: '' }", { source: '' }],
] as const;

describe('path 1 — evaluateCondition’s legacy path diagnoses a blank gate (objectui#11262, ADR-0137 D4)', () => {
it.each(BLANKS)('%s: still true ("no gate"), and now said once', (_label, gate) => {
const ev = new ExpressionEvaluator({ record: {} });
expect(ev.evaluateCondition(gate)).toBe(true);
expect(ev.evaluateCondition(gate)).toBe(true);
expect(blankLines()).toHaveLength(1);
});

it.each(BLANKS)('%s: the [blank] reason goes to onFault instead, when the caller supplies one', (_label, gate) => {
const reasons: string[] = [];
const ev = new ExpressionEvaluator({ record: {} });
expect(ev.evaluateCondition(gate, { onFault: (r) => reasons.push(r) })).toBe(true);
expect(reasons).toHaveLength(1);
expect(reasons[0]).toMatch(/^\[blank\]/);
expect(warn).not.toHaveBeenCalled();
});

it.each(BLANKS)('%s: throwOnError does not throw on it — nothing was authored that could fault — and it is still said', (_label, gate) => {
const ev = new ExpressionEvaluator({ record: {} });
expect(ev.evaluateCondition(gate, { throwOnError: true })).toBe(true);
expect(blankLines()).toHaveLength(1);
});

it('one diagnosis point: a blank on the legacy path and its dialect: cel twin print ONE line between them', () => {
const ev = new ExpressionEvaluator({ record: {} });
expect(ev.evaluateCondition('')).toBe(true);
expect(blankLines()).toHaveLength(1);
expect(ev.evaluateCondition({ dialect: 'cel', source: '' })).toBe(true);
expect(ev.evaluateCondition({ source: '' })).toBe(true);
expect(blankLines()).toHaveLength(1);
});

it.each([
['undefined', undefined],
['null', null],
] as const)('control — %s is no gate at all, not blank TEXT: true and silent', (_label, gate) => {
const ev = new ExpressionEvaluator({ record: {} });
expect(ev.evaluateCondition(gate as never)).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

it('control — a written legacy gate answers its own verdict and is silent', () => {
const ev = new ExpressionEvaluator({ data: { stage: 'open' } });
expect(ev.evaluateCondition("${data.stage === 'open'}")).toBe(true);
expect(ev.evaluateCondition("${data.stage === 'won'}")).toBe(false);
expect(ev.evaluateCondition('1 > 2')).toBe(false);
expect(warn).not.toHaveBeenCalled();
});
});

describe('path 2 — evalRowPredicate diagnoses a bare blank instead of returning its fallback in silence (objectui#11262)', () => {
const ROUTES = [
['the single-eval route', false],
['the labelled fail-closed route (warnOnError)', true],
] as const;

describe.each(ROUTES)('%s', (_route, warnOnError) => {
it.each(BLANKS)('%s: the caller’s fallback, both directions, and a [blank] report', (_label, pred) => {
const opts = { warnOnError, label: 'row action "archive_11262"' };
expect(evalRowPredicate(pred as never, { a: 1 }, { ...opts, fallback: true })).toBe(true);
expect(evalRowPredicate(pred as never, { a: 1 }, { ...opts, fallback: false })).toBe(false);
// One line for both calls: the dedupe keys on the text and the label,
// never on the fallback.
const lines = blankLines();
expect(lines).toHaveLength(1);
expect(lines[0]).toContain('archive_11262');
});

it('one diagnosis point: the bare blank takes its envelope twin’s route — one line between them', () => {
const opts = { warnOnError, fallback: false, label: 'formatting rule 11262' };
evalRowPredicate('', { a: 1 }, opts);
expect(blankLines()).toHaveLength(1);
evalRowPredicate({ source: '' } as never, { a: 1 }, opts);
evalRowPredicate('', { a: 2 }, opts);
expect(blankLines()).toHaveLength(1);
});

it.each([
['undefined', undefined],
['null', null],
] as const)('control — %s is no predicate: the fallback, silent', (_label, pred) => {
expect(evalRowPredicate(pred, { a: 1 }, { warnOnError, fallback: true })).toBe(true);
expect(evalRowPredicate(pred, { a: 1 }, { warnOnError, fallback: false })).toBe(false);
expect(warn).not.toHaveBeenCalled();
});

it('control — a written predicate answers its own verdict and is silent', () => {
expect(evalRowPredicate('record.a == 1', { a: 1 }, { warnOnError, fallback: false })).toBe(true);
expect(evalRowPredicate('record.a == 1', { a: 2 }, { warnOnError, fallback: true })).toBe(false);
expect(warn).not.toHaveBeenCalled();
});
});

it('rowless (a param dialog) takes the same guard — the fallback, and the report', () => {
expect(evalRowPredicate(' ', null, { rowless: true, fallback: true, warnOnError: true, label: 'param "p_11262"' })).toBe(true);
expect(blankLines()).toHaveLength(1);
});
});
Loading
Loading