diff --git a/.changeset/6038-production-predicate-fault-warning.md b/.changeset/6038-production-predicate-fault-warning.md new file mode 100644 index 0000000000..8110de9e00 --- /dev/null +++ b/.changeset/6038-production-predicate-fault-warning.md @@ -0,0 +1,70 @@ +--- +'@object-ui/core': patch +'@object-ui/react': patch +'@object-ui/components': patch +--- + +A node-gate visibility predicate that FAULTS now says so in a production build, once per +distinct predicate source (objectui#6038, maintainer ruling 2026-08-25, option B: "the +silence is no longer an accepted property"). Observability only — no verdict moves. + +`SchemaRenderer`'s visibility chain is fail-open: a predicate that cannot be evaluated +resolves to the same answer as one that said yes, so a gate that stops biting looks +exactly like a gate the author got right. The diagnostic that names it (objectui#5454 / +objectui#5687) sat behind a `__DEV__` short-circuit, because the only fault-detection +channel available was `throwOnError`, and on the CEL branch `evaluateCelCondition` +implements that by evaluating **twice** — too expensive to ship for every predicate of +every node. + +**What production actually printed before, measured per dialect on the built evaluator** +— the card's premise held for one dialect of three, and the other two failed in opposite +directions: + +| dialect | production console, before | +|---|---| +| bare string | **nothing** | +| `{ dialect: 'cel' }` envelope | one generic line, deduped per source | +| `${…}` template | one generic line **per evaluation**, never deduped | + +So the dialect objectstack#11254 measured a live gate breaking on was the silent one, +while the template dialect was the console flood the ruling's rate-limit clause exists to +prevent. + +**The fix reports the fault the evaluator already detected, at the same number of engine +calls.** `EvaluationOptions.onFault` is a new passback on `@object-ui/core`'s +`ExpressionEvaluator`: every fault site is already inside a `catch`, or already holds the +canonical engine's failure reason, so nothing is evaluated twice. It mirrors, one layer +up, the seam `FieldPredicateDiagnostic` already documents (`warn: false` plus a reason +passback), and supplying it transfers reporting to the caller so one fault stays one +line. Pinned: the CEL branch performs the same number of record reads with the passback +as without it, and strictly fewer than the `throwOnError` probe. + +`SchemaRenderer` passes it in production and reports through the **same** reporter the dev +branch uses — same message, same severity, same dedupe `Set`, same key. Development and +production now print the identical line for the identical fault; the `__DEV__` gate no +longer decides *whether* a fault is reported, only *how* it is detected. + +`page:tabs` item-level `visibleWhen` (`@object-ui/components`) is covered by the same +reporter and the same rate limit. It swallowed the identical fault under a different +helper, and it was the worse of the two: the node gate at least reported in development, +while a faulting item predicate was silent in *both* builds on a gate whose false verdict +removes an entire tab, header and panel. + +**Rate limit:** deduped per (node type, gate key, predicate source) — never per render and +never per node instance. A two-hundred-row list of one broken predicate is one line; a +second distinct predicate source still gets its own line. Both halves are pinned, because +a test that asserts only "a warning was emitted" is equally green on an implementation +that emitted fifty, and one that asserts only "exactly one" is equally green on an +implementation that suppresses everything. + +**Not changed by this card, deliberately:** the fail-open semantics themselves; the +objectui#5687 adapter-only `data.*` report, which stays development-only under its own +2026-08-22 ruling (that path is not a fault — the predicate evaluated perfectly, against +the wrong object); and the `/forms/:name` scope wiring of objectui#6262, which lands in +its own PR. + +`reportUnresolvableVisibilityPredicate`, `formatUnresolvableVisibilityMessage`, +`UNRESOLVABLE_VISIBILITY_PREFIX` and `__resetVisibilityPredicateWarnings` are now exported +from `@object-ui/react` so every surface that evaluates a node `visibleWhen` shares one +reporter and one rate limit — a second copy would mean a second dedupe `Set`, and one +authored predicate would be entitled to one line per package instead of one line. diff --git a/content/docs/guide/metadata-diagnostics.md b/content/docs/guide/metadata-diagnostics.md index 1661b0b0d6..27ab97bd36 100644 --- a/content/docs/guide/metadata-diagnostics.md +++ b/content/docs/guide/metadata-diagnostics.md @@ -168,6 +168,30 @@ fires while its field stays visible is the classic symptom — open the browser console and the broken predicate identifies itself (most often a bare field name where `record.` was meant). +The same is now true of a **component node's own gate** — `visibleWhen` on a +page component (and its `visible` / `visibleOn` / `visibility` / `hidden` / +`hiddenOn` siblings), plus a `page:tabs` item's `visibleWhen`. These used to +report in a development build only, so a gate that stopped biting in +production left nothing on the console at all. They now warn in **both** +builds, with the node type, the node id, the gate key, the predicate source +and the engine's reason: + +```text +[ObjectUI] A visibility predicate could not be evaluated - node "record:alert" (id: "a1") + visibleWhen: "nosuchroot.status == 'draft'" + Reason: Failed to evaluate expression "nosuchroot.status == 'draft'": nosuchroot is not defined +The node was treated as its safe default, which on this surface means the +gate did NOT bite - a predicate that cannot be evaluated reads on screen +exactly like one that said yes. +``` + +The line is **rate limited to one per distinct predicate source**, so a broken +predicate rendered down two hundred rows of a list is one line, not two +hundred — while a second, differently-broken predicate still gets its own. +The verdict is unchanged in every case: this is a diagnostic about a +predicate, not a change to what the gate decides. A node gate that fails open +renders exactly as it always did; the difference is that it now says so. + ### 4. Governance overview page `/apps//metadata/_diagnostics` — a single sortable table of every diff --git a/packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx b/packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx new file mode 100644 index 0000000000..e5f959c81f --- /dev/null +++ b/packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx @@ -0,0 +1,196 @@ +/** + * 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#6038, census site 2 — a `page:tabs` item's `visibleWhen` that FAULTS + * is reported, through the same reporter and the same rate limit as the node + * gate in `SchemaRenderer`. + * + * ## Why this file exists at all + * + * The card is written about `evaluateVisibilityPredicate`'s `__DEV__` + * short-circuit, and the dispatch's census clause is explicit that the shape — + * "a predicate evaluation caught and swallowed" — is what has to be covered, + * not the symbol. `PageTabsRenderer.isItemVisible` is that shape under another + * name: it calls the same `evaluateCondition`, on the same canonical key + * (`visibleWhen`), with the same fail-open contract its own comment declares + * ("the same semantics SchemaRenderer applies to component-level + * `visibleWhen`"). + * + * It was in fact the WORSE of the two. The node gate at least reported in a + * development build; an item-level `visibleWhen` that faulted here was silent + * in BOTH builds — and its false verdict removes an entire tab, header and + * panel, rather than one block. A tab that quietly stops disappearing (or + * quietly stops appearing) is the failure an author is least likely to notice, + * because a tab strip looks correct in every arrangement. + * + * ## One reporter, one rate limit — not one per package + * + * The report goes through `reportUnresolvableVisibilityPredicate`, exported + * from `@object-ui/react` for this card. A local copy would mean a second + * dedupe `Set`, and one authored predicate would then be entitled to one line + * per package instead of one line. The last case here is what pins that. + * + * ## Reverse verification (direction predicted BEFORE running) + * + * Dropping the `onFault` option from `isItemVisible` turns RED exactly the + * report cases and leaves every VERDICT case green — the tabs render + * identically either way, which is this card's observability-only constraint + * restated on this surface. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import React from 'react'; +import { + SchemaRenderer, + UNRESOLVABLE_VISIBILITY_PREFIX, + __resetVisibilityPredicateWarnings, +} from '@object-ui/react'; +import '../renderers'; + +const tabsSchema = (items: any[]) => ({ type: 'page:tabs', id: 'tabs', items }); + +/** Faults on every dialect this surface accepts (measured on the built evaluator). */ +const FAULT_BARE = 'nosuchroot.x > 1'; +const FAULT_BARE_2 = 'anotherbadroot.y == 3'; + +type WarnSpy = { mock: { calls: unknown[][] } }; +const spyWarn = () => vi.spyOn(console, 'warn').mockImplementation(() => {}); +const reports = (warn: WarnSpy): string[] => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX)); +const allWarnings = (warn: WarnSpy): string[] => warn.mock.calls.map((c) => String(c[0])); + +beforeEach(() => { + __resetVisibilityPredicateWarnings(); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('objectui#6038 — a faulting `page:tabs` item predicate is reported', () => { + it('POSITIVE CONTROL: the spy observes a line carrying the prefix', () => { + // Without it, the `toHaveLength(0)` cases below are equally green on a + // capture that observes nothing. + const warn = spyWarn(); + console.warn(`${UNRESOLVABLE_VISIBILITY_PREFIX} - synthetic control line`); + expect(reports(warn)).toHaveLength(1); + }); + + it('DEGENERATE CONTROL: unrelated console output does not satisfy the pin', () => { + const warn = spyWarn(); + console.warn('[object-ui] an entirely unrelated warning'); + expect(allWarnings(warn)).toHaveLength(1); + expect(reports(warn)).toHaveLength(0); + }); + + it('THE acceptance criterion: a faulting item `visibleWhen` warns, and the tab still renders', () => { + const warn = spyWarn(); + const { getByText } = render( + , + ); + // VERDICT UNCHANGED — fail-open, so the tab is still there. This card does + // not get to move that; it only gets to say so. + expect(getByText('Contracts')).toBeTruthy(); + + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('page:tabs'); + expect(lines[0]).toContain('visibleWhen'); + expect(lines[0]).toContain(FAULT_BARE); + expect(lines[0]).toContain('Reason:'); + }); + + it('a HEALTHY item predicate stays silent, on both verdicts', () => { + // The half that makes the loud half mean something: a false predicate is a + // verdict, not a fault, and must print nothing. + const warn = spyWarn(); + // THREE items, two of them surviving: `alwaysShowStrip` defaults to false, + // so a strip down to a single tab hides its header entirely and renders the + // panel bare — the assertion below would then be measuring that rule + // instead of this one. (Measured: this case first failed on exactly that.) + const { getByText, queryByText } = render( + , + ); + expect(getByText('Details')).toBeTruthy(); + expect(getByText('Related')).toBeTruthy(); + expect(queryByText('Contracts')).toBeNull(); + expect(allWarnings(warn)).toHaveLength(0); + }); + + it('deduped per predicate SOURCE: eight tabs sharing one broken predicate produce ONE line', () => { + // The "not per call-site instance" half — eight distinct items, eight + // evaluations, one authored mistake. + const warn = spyWarn(); + render( + ({ + label: `Tab ${i}`, + value: `t${i}`, + visibleWhen: FAULT_BARE, + children: [], + })), + )} + />, + ); + expect(reports(warn)).toHaveLength(1); + expect(allWarnings(warn)).toHaveLength(1); + }); + + it('a SECOND distinct predicate source still warns — a dedupe that suppressed everything would look identical', () => { + const warn = spyWarn(); + render( + , + ); + const lines = reports(warn); + expect(lines).toHaveLength(2); + expect(lines.some((l) => l.includes(FAULT_BARE))).toBe(true); + expect(lines.some((l) => l.includes(FAULT_BARE_2))).toBe(true); + }); + + it('ONE rate limit across packages: the node gate and the tab gate share a dedupe `Set`', () => { + // The property the shared export exists for. The SAME predicate source on + // the SAME node type must not be entitled to a second line just because a + // second package evaluated it. Here the tab strip reports first; a node + // gate of type `page:tabs` carrying the same `visibleWhen` source then + // finds the entry already present. + // + // (A different node TYPE legitimately reports again — the key is + // (type, key, source), and two types are two places an author has to go + // and fix. This case holds the type fixed, which is what isolates the + // cross-package question from the key-shape question.) + const warn = spyWarn(); + render( + , + ); + expect(reports(warn)).toHaveLength(1); + cleanup(); + render(); + expect(reports(warn)).toHaveLength(1); + }); +}); diff --git a/packages/components/src/renderers/layout/containers.tsx b/packages/components/src/renderers/layout/containers.tsx index 62297dc72c..f3c8e995e9 100644 --- a/packages/components/src/renderers/layout/containers.tsx +++ b/packages/components/src/renderers/layout/containers.tsx @@ -22,7 +22,7 @@ import React from 'react'; import { ComponentRegistry, ExpressionEvaluator, evalRowPredicate, getRecordDisplayName, toPredicateRecord } from '@object-ui/core'; import type { ComponentInput } from '@object-ui/core'; import { actionRendersAt } from '@object-ui/types'; -import { useRecordContext, useAction, useCapabilityGate, usePredicateScope, usePageVariables, useInlineEdit, useActionTextLocalizer } from '@object-ui/react'; +import { useRecordContext, useAction, useCapabilityGate, usePredicateScope, usePageVariables, useInlineEdit, useActionTextLocalizer, reportUnresolvableVisibilityPredicate } from '@object-ui/react'; import { renderChildren, cn } from '../../lib/utils'; import { LazyIcon } from '../../lib/lazy-icon'; import { RelatedCountStore, useRelatedCountVersion } from '../../hooks/related-count-store'; @@ -456,8 +456,22 @@ const PageTabsRenderer: React.FC = ({ schema, className, ...props }) => { page: pageVariables, }); // evaluateCondition is fail-open (unparseable predicate → visible) — the - // same semantics SchemaRenderer applies to component-level `visibleWhen`. - return evaluator.evaluateCondition(it.visibleWhen); + // same semantics SchemaRenderer applies to component-level `visibleWhen`, + // and objectui#6038 gives it the same VOICE. The verdict is untouched: a + // faulting predicate still resolves to `true` and the tab still renders. + // + // This site is in the census for the reason the card's census clause names + // — it swallows the identical fault under a different helper. It is worse + // than the node gate was, in fact: `SchemaRenderer` at least reported in + // development, while an item-level `visibleWhen` that faulted here was + // silent in BOTH builds, on a gate whose false verdict removes an entire + // tab (header and panel) rather than one block. Reported through the SAME + // reporter and the SAME dedupe `Set` as the node gate, so one authored + // predicate is one line no matter which surface evaluates it. + return evaluator.evaluateCondition(it.visibleWhen, { + onFault: (reason) => + reportUnresolvableVisibilityPredicate('page:tabs', schema?.id, 'visibleWhen', it.visibleWhen, reason), + }); }; const visibleFlags = rawItems.map(isItemVisible); // Keep the filtered array's identity stable while visibility is unchanged diff --git a/packages/core/src/evaluator/ExpressionEvaluator.ts b/packages/core/src/evaluator/ExpressionEvaluator.ts index 9b0ca4f555..298b84bc05 100644 --- a/packages/core/src/evaluator/ExpressionEvaluator.ts +++ b/packages/core/src/evaluator/ExpressionEvaluator.ts @@ -41,6 +41,70 @@ export interface EvaluationOptions { * @default true */ sanitize?: boolean; + + /** + * Fault passback: called with the failure reason when this evaluation could + * not be performed, at the moment the evaluator ALREADY knows it faulted. + * + * ## Why a passback and not a second evaluation (objectui#6038) + * + * The only fault-detection channel this class used to offer a fail-soft + * caller was `throwOnError`, and on the CEL branch `evaluateCelCondition` + * implements that by evaluating TWICE (once with each fallback — a value + * that tracks the fallback both times is a fault). A caller that wants to + * *observe* a fault while keeping the fail-soft verdict therefore had to pay + * for a second engine call per predicate per node per render. That price is + * exactly why the node gate in `SchemaRenderer` bought its diagnostic with a + * `__DEV__` gate and shipped production silent. + * + * This option costs nothing: every fault site below is already inside a + * `catch`, or already holds the engine's own failure reason. The verdict is + * untouched on every path — `onFault` is invoked for its side effect and its + * return value is ignored. + * + * ## Supplying it TRANSFERS reporting to the caller + * + * The built-in `console.warn`s on these paths are suppressed while it is + * set, so one fault stays one line — the caller's, which can name the node + * the predicate belongs to. This mirrors, one layer up, the contract + * `FieldPredicateDiagnostic` (`fieldRules.ts`) already documents for the + * canonical CEL engine: `warn: false` plus an `onFault` passback, so + * silencing the generic line never discards the description of *why* the + * predicate failed. On the CEL branch this option is forwarded to exactly + * that seam rather than reimplementing it. + * + * Independent of {@link throwOnError}, which converts a fault into a throw + * and is the fail-CLOSED contract; this one keeps the historical fail-soft + * answer and merely says so out loud. `throwOnError` still wins where both + * are set: the throw happens first and is the caller's own signal. + * + * Must not throw — it is invoked outside the evaluation guard, so an + * exception here propagates to the caller rather than being reported as an + * evaluation fault. + */ + onFault?: (reason: string) => void; +} + +/** + * One fault, one report: hand the reason to the caller's {@link + * EvaluationOptions.onFault} when it supplied one, otherwise fall back to this + * class's historical `console.warn`. + * + * Kept as a module-local function rather than repeating the ternary at each + * catch, so the "supplying `onFault` suppresses the built-in line" rule is + * stated once and cannot drift between the three sites that implement it. + */ +function reportEvaluationFault( + onFault: ((reason: string) => void) | undefined, + builtinMessage: string, + error: unknown, +): void { + const reason = error instanceof Error ? error.message : String(error); + if (onFault) { + onFault(reason); + return; + } + console.warn(builtinMessage, error); } /** @@ -91,7 +155,7 @@ export class ExpressionEvaluator { return expression; } - const { defaultValue, throwOnError = false, sanitize = true } = options; + const { defaultValue, throwOnError = false, sanitize = true, onFault } = options; try { // Check if string contains template expressions @@ -117,7 +181,7 @@ export class ExpressionEvaluator { if (throwOnError) { throw error; } - console.warn(`Expression evaluation failed for: ${expr}`, error); + reportEvaluationFault(onFault, `Expression evaluation failed for: ${expr}`, error); return match; // Return original if evaluation fails } }); @@ -125,7 +189,7 @@ export class ExpressionEvaluator { if (throwOnError) { throw error; } - console.warn(`Failed to evaluate expression: ${expression}`, error); + reportEvaluationFault(onFault, `Failed to evaluate expression: ${expression}`, error); return defaultValue ?? expression; } } @@ -266,6 +330,17 @@ export class ExpressionEvaluator { if (options.throwOnError) { throw error; } + // objectui#6038 — the dialect that reported NOTHING. Measured on the + // built evaluator against the other two: a `{ dialect: 'cel' }` envelope + // already warns here in production (`evalFieldPredicate`, deduped per + // source) and a `${…}` template already warns (the generic line above), + // while a BARE-STRING predicate that faults returned its fail-soft `true` + // in complete silence. That is the dialect objectstack#11254 measured a + // real gate breaking on, and the reason a node gate could stop biting + // with nothing on the console to say so. `onFault` is the only new + // channel: absent it this catch behaves exactly as it always has, so no + // existing caller's console output moves. + options.onFault?.(error instanceof Error ? error.message : String(error)); return true; } } @@ -287,8 +362,23 @@ export class ExpressionEvaluator { ? (rec as Record) : (bag as Record); if (!options.throwOnError) { - // Fast path: one evaluation, fail-soft to visible/enabled (legacy parity). - return evalFieldPredicate(source, record, true, undefined, bag); + // Fast path: ONE evaluation, fail-soft to visible/enabled (legacy parity). + // + // objectui#6038: when the caller passes `onFault`, forward it to the seam + // `evalFieldPredicate` already exposes for exactly this — `warn: false` + // plus the reason passback — rather than adding a second reporter. The + // caller then emits one line that can name the node; without `onFault` + // the built-in warning fires exactly as before. Either way this stays a + // SINGLE engine call: the `throwOnError` double-evaluation below is what + // this branch exists to avoid paying in production. + return evalFieldPredicate( + source, + record, + true, + undefined, + bag, + options.onFault ? { warn: false, onFault: options.onFault } : undefined, + ); } // Fail-closed callers need to tell a genuine `false` from a fault. The // canonical helper fails soft to the fallback, so a value that tracks the diff --git a/packages/core/src/evaluator/__tests__/ExpressionEvaluator.onFault.test.ts b/packages/core/src/evaluator/__tests__/ExpressionEvaluator.onFault.test.ts new file mode 100644 index 0000000000..a7704a5509 --- /dev/null +++ b/packages/core/src/evaluator/__tests__/ExpressionEvaluator.onFault.test.ts @@ -0,0 +1,187 @@ +/** + * 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#6038 — `EvaluationOptions.onFault`: observe a predicate fault + * WITHOUT a second evaluation and WITHOUT moving a verdict. + * + * ## The problem this seam exists to solve + * + * Before it, the only fault-detection channel a fail-soft caller had was + * `throwOnError`, and on the CEL branch `evaluateCelCondition` implements that + * by evaluating TWICE. So a caller that wanted to *report* a broken predicate + * while keeping the historical fail-open answer had to double the engine calls + * for every predicate of every node of every render — which is precisely why + * `SchemaRenderer`'s node gate bought its diagnostic with a `__DEV__` gate and + * shipped production silent. The maintainer's 2026-08-25 ruling (option B) + * retired that silence and kept the negligible-cost requirement, so the fault + * has to become observable at the SAME number of engine calls. + * + * ## What was actually silent — measured, per dialect + * + * Against `origin/main`, on the built evaluator, a faulting predicate with no + * options produced: bare string -> NOTHING; `{ dialect: 'cel' }` -> one generic + * line, deduped; `${…}` template -> one generic line PER EVALUATION. The first + * group of cases below pins all three converging on the passback, which is what + * lets one caller print one line for a fault in any dialect. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ExpressionEvaluator } from '../ExpressionEvaluator.js'; + +const SCOPE = { record: { status: 'open' }, data: { total: 99 }, page: {} }; +const evaluator = () => new ExpressionEvaluator(SCOPE); + +/** Faulting predicates, one per dialect. */ +const FAULT_BARE = 'nosuchroot.x > 1'; +const FAULT_TEMPLATE = '${nosuchroot.x > 1}'; +const FAULT_CEL = { dialect: 'cel', source: 'record.bad(' }; + +describe('#6038 — onFault fires on every dialect that can fault', () => { + it('BARE STRING: the dialect that reported nothing at all now hands back a reason', () => { + // objectstack#11254 measured a real gate breaking on exactly this dialect + // and produced no console line anywhere. + const reasons: string[] = []; + const verdict = evaluator().evaluateCondition(FAULT_BARE, { onFault: (r) => reasons.push(r) }); + expect(verdict).toBe(true); // fail-open, unchanged + expect(reasons).toHaveLength(1); + expect(reasons[0]).toContain('nosuchroot'); + }); + + it('CEL ENVELOPE: forwarded to `evalFieldPredicate`\'s existing passback, and its generic line is suppressed', () => { + // One fault, one report. Without `warn: false` the canonical engine would + // print its own line beside the caller's, so a production console would + // show two lines for one broken predicate. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const reasons: string[] = []; + const verdict = evaluator().evaluateCondition(FAULT_CEL, { onFault: (r) => reasons.push(r) }); + expect(verdict).toBe(true); + expect(reasons).toHaveLength(1); + expect(reasons[0]).toContain('parse'); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); + + it('${…} TEMPLATE: the generic per-evaluation line is replaced by one passback per evaluation', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const reasons: string[] = []; + const verdict = evaluator().evaluateCondition(FAULT_TEMPLATE, { onFault: (r) => reasons.push(r) }); + expect(verdict).toBe(true); + expect(reasons).toHaveLength(1); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); + + it('a HEALTHY predicate never calls it — on either verdict', () => { + // The silence that makes the signal mean something. A `false` verdict is an + // answer, not a fault, and a passback that fired on it would report every + // hiding gate in the repository. + const reasons: string[] = []; + const e = evaluator(); + expect(e.evaluateCondition("record.status == 'open'", { onFault: (r) => reasons.push(r) })).toBe(true); + expect(e.evaluateCondition("record.status == 'closed'", { onFault: (r) => reasons.push(r) })).toBe(false); + expect(e.evaluateCondition({ dialect: 'cel', source: "record.status == 'closed'" }, { onFault: (r) => reasons.push(r) })).toBe(false); + expect(reasons).toHaveLength(0); + }); +}); + +describe('#6038 — the seam moves no verdict and adds no evaluation', () => { + it('every predicate shape reaches the identical verdict with and without onFault', () => { + // The card is observability-only. This is that constraint as a measurement + // rather than a claim: same inputs, both call shapes, byte-identical answers. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const cases: Array<[unknown, string]> = [ + [FAULT_BARE, 'bare fault'], + [FAULT_CEL, 'cel fault'], + [FAULT_TEMPLATE, 'template fault'], + ["record.status == 'open'", 'bare true'], + ["record.status == 'closed'", 'bare false'], + [{ dialect: 'cel', source: "record.status == 'open'" }, 'cel true'], + [{ dialect: 'cel', source: "record.status == 'closed'" }, 'cel false'], + ['${record.status == "open"}', 'template true'], + ['${record.status == "closed"}', 'template false'], + [true, 'literal true'], + [false, 'literal false'], + [undefined, 'absent'], + ['', 'empty'], + [' ', 'whitespace'], + ]; + for (const [pred, label] of cases) { + const without = evaluator().evaluateCondition(pred as never); + const with_ = evaluator().evaluateCondition(pred as never, { onFault: () => {} }); + expect(`${label}: ${String(with_)}`).toBe(`${label}: ${String(without)}`); + } + } finally { + warn.mockRestore(); + } + }); + + it('the CEL branch still makes ONE engine call — the `throwOnError` probe is not smuggled in', () => { + // The whole point of the passback. `throwOnError` detects a CEL fault by + // evaluating twice (once with each fallback); if `onFault` had been built on + // top of it, production would pay double for every predicate of every node. + // A throwing getter on the record counts the reads the engine performs. + let reads = 0; + const probed = new ExpressionEvaluator({ + record: { + get status() { + reads += 1; + return 'open'; + }, + }, + }); + reads = 0; + probed.evaluateCondition({ dialect: 'cel', source: "record.status == 'open'" }, { onFault: () => {} }); + const withPassback = reads; + + reads = 0; + probed.evaluateCondition({ dialect: 'cel', source: "record.status == 'open'" }, { throwOnError: true }); + const withProbe = reads; + + // The passback path reads the record the same number of times a plain + // fail-soft call does; the `throwOnError` probe reads it strictly more. + reads = 0; + probed.evaluateCondition({ dialect: 'cel', source: "record.status == 'open'" }); + const plain = reads; + + expect(withPassback).toBe(plain); + expect(withProbe).toBeGreaterThan(plain); + }); + + it('without onFault, nothing about the existing console output moves', () => { + // The compatibility half: every caller that does not opt in must see the + // exact lines it saw before. A bare-string fault stays silent (it always + // was), and the two loud dialects keep their own generic lines. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect(evaluator().evaluateCondition(FAULT_BARE)).toBe(true); + expect(warn).not.toHaveBeenCalled(); + + expect(evaluator().evaluateCondition(FAULT_TEMPLATE)).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain('Failed to evaluate expression'); + } finally { + warn.mockRestore(); + } + }); + + it('`throwOnError` still wins where both are set — the fail-closed contract is unmoved', () => { + // Documented precedence. A caller that asked for a throw gets a throw; the + // passback is the fail-SOFT observation channel, not a second one. + expect(() => + evaluator().evaluateCondition(FAULT_BARE, { throwOnError: true, onFault: () => {} }), + ).toThrow(); + }); +}); diff --git a/packages/react/src/SchemaRenderer.tsx b/packages/react/src/SchemaRenderer.tsx index 619f23374b..1b16513bd8 100644 --- a/packages/react/src/SchemaRenderer.tsx +++ b/packages/react/src/SchemaRenderer.tsx @@ -623,7 +623,20 @@ export const SchemaRenderer: ForwardRefExoticComponent< * on which dialect they happened to write it in. * * Deduped per (node type, key, predicate source): a broken predicate is - * re-evaluated on every render, and the point is one line, not a wall. + * re-evaluated on every render, and the point is one line, not a wall. The + * key is the predicate SOURCE TEXT plus the gate it was authored on — never + * the render and never the schema object — so the same broken predicate + * rendered over two hundred rows reports once, and a SECOND distinct + * predicate still reports (objectui#6038 pins both halves). + * + * ## Production is loud too, since objectui#6038 + * + * It was `__DEV__`-only, and the maintainer's 2026-08-25 ruling retired + * that silence: a gate that stops biting in production used to leave + * nothing on the console for the bare-string dialect, so a class-1 defect + * could sit live and undiscovered (measured in objectstack#11254). The + * `__DEV__` gate below no longer decides WHETHER the fault is reported, + * only HOW it is detected — see the two branches. * * ## Defined HERE, ahead of the `properties` evaluation loop below * @@ -635,15 +648,33 @@ export const SchemaRenderer: ForwardRefExoticComponent< * on the POST-evaluation, POST-hoist schema, for the real verdict. */ const evaluateVisibilityPredicate = (raw: VisibilityPredicate, key: string): boolean => { - // PRODUCTION IS THE UNTOUCHED CALL. `throwOnError` is how the fault is - // detected, and on the CEL branch `evaluateCelCondition` implements it by - // evaluating TWICE (once with each fallback — a value that tracks the - // fallback both times is a fault). Spec-parsed metadata normalizes - // `visibleWhen` into a `{ dialect: 'cel' }` envelope, so that branch is - // the common one in production: paying for the probe unconditionally - // would double the engine calls for every predicate of every node, to - // build a message no production build ever prints. - if (!__DEV__) return evaluator.evaluateCondition(raw); + // PRODUCTION STILL MAKES THE SINGLE CALL — it just no longer makes it + // in silence (objectui#6038, maintainer ruling 2026-08-25, option B). + // + // `throwOnError` remains the DEV probe and remains too expensive to ship: + // on the CEL branch `evaluateCelCondition` implements it by evaluating + // TWICE (once with each fallback — a value that tracks the fallback both + // times is a fault), and spec-parsed metadata normalizes `visibleWhen` + // into a `{ dialect: 'cel' }` envelope, so that branch is the common one + // in production. Paying for the probe here would double the engine calls + // for every predicate of every node. + // + // `onFault` is the way out of that trade: the evaluator hands back the + // reason at the point it ALREADY knows the predicate faulted, inside the + // catch it already runs, so the fault becomes observable at ONE engine + // call. The verdict is `evaluateCondition(raw)`'s, unchanged — this card + // is observability only, and the fail-open semantics are not its to move. + // + // The reporter is the SAME one the dev branch below uses: same message, + // same severity, same dedupe `Set`, same key. Production and development + // now print the identical line for the identical fault, which is the + // property the `__DEV__` gate used to cost us. + if (!__DEV__) { + return evaluator.evaluateCondition(raw, { + onFault: (reason) => + reportUnresolvableVisibilityPredicate(newSchema.type, newSchema.id, key, raw, reason), + }); + } try { const verdict = evaluator.evaluateCondition(raw, { throwOnError: true }); // objectui#5687 — the NON-throwing half of the same silence, and it is diff --git a/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx b/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx index 66ce91480b..5bf006c48a 100644 --- a/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx +++ b/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx @@ -399,8 +399,15 @@ describe('#5687 group 3 — the `record.*` bucket is NOT this card, and cannot b describe('#5687 group 4 — production is untouched', () => { it('a PRODUCTION build reaches the SAME verdicts and prints nothing', async () => { - // The diagnostic is dev-only by the same `__DEV__` gate objectui#5454 put - // in front of the probe. A gate that changed the ANSWER would be a fork. + // THIS leg — objectui#5687's adapter-only `data.*` report — is dev-only, + // and stays so: its own ruling (2026-08-22, option A) scoped it there, and + // it does not describe a fault at all (the predicate evaluated perfectly, + // against the wrong object). The SIBLING leg no longer is: objectui#6038 + // took the `__DEV__` gate off `reportUnresolvableVisibilityPredicate`, so + // a FAULTING predicate is reported in production too. That is why every + // assertion below filters on `ADAPTER_ONLY_DATA_PREDICATE_PREFIX` rather + // than counting raw `console.warn` calls. A gate that changed the ANSWER + // would be a fork, and none of these verdicts moved. // // The dynamic import lives in the test BODY, not a hook: it has to read // module state that only exists after `resetModules` + `stubEnv`, which is diff --git a/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx b/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx new file mode 100644 index 0000000000..97460f05b8 --- /dev/null +++ b/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx @@ -0,0 +1,437 @@ +/** + * 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#6038 — a node-gate predicate that FAULTS is loud in a PRODUCTION + * build, once per distinct predicate source. + * + * Maintainer ruling, 2026-08-25, in-session batch adjudication (verbatim: + * 「就全部接受，然后继续下一批」) => option B: "production bundles emit a + * rate-limited, deduplicated warning (one per distinct failing predicate) when + * a `visibleWhen` / node-gate predicate faults, replacing the current + * `__DEV__`-only silence. […] A is rejected — the silence is no longer an + * accepted property." Observability only: the fail-open semantics are NOT this + * card's to change, so every case below pins the verdict alongside the line. + * + * ## What "the silence" actually was — measured, not assumed + * + * The card states production prints nothing. Measured on the built evaluator + * against `origin/main`, that is true of ONE dialect of three, and the other + * two fail in opposite directions: + * + * | dialect | production console, before | + * |------------------------|----------------------------| + * | bare string | NOTHING | + * | `{ dialect: 'cel' }` | 1 generic line, deduped | + * | `${…}` template | 1 generic line PER EVALUATION (never deduped) | + * + * So the bare-string dialect — the one objectstack#11254 measured a real gate + * breaking on — was silent, while the template dialect was the flood the + * ruling's rate-limit clause exists to prevent. Both are now ONE node-bearing + * line per distinct source, which is why several cases below pin the TOTAL + * `console.warn` count and not merely the count of matching lines: an + * implementation that added our line beside the generic one would satisfy + * "warned once" while doubling what a production console actually shows. + * + * ## The two halves of a rate limit, and why one of them is not optional + * + * "A warning was emitted" is green on an implementation that emitted fifty, and + * "exactly one warning was emitted" is green on an implementation that + * suppresses EVERYTHING after the first line ever printed. Neither pin alone + * can tell a working dedupe from a broken one, so every dedupe case here pins + * both: N evaluations of one source => exactly 1 line, AND a second DISTINCT + * source still => its own line. + * + * ## Controls + * + * - **Positive control** (`capture` group): the spy sees a line this test emits + * itself. Without it, every `toHaveLength(0)` in the file is also green when + * the capture is simply broken. + * - **Degenerate control** (`capture` group): unrelated `console.warn` output + * does NOT satisfy the pin — the assertions read a PREFIX-FILTERED view, so + * noise from anywhere else in the render cannot stand in for the diagnostic. + * + * ## Reverse verification (direction predicted BEFORE running) + * + * Restoring `if (!__DEV__) return evaluator.evaluateCondition(raw);` — the line + * this card replaces — turns RED every production case in groups 1-3 (their + * report counts fall to 0, except the CEL and template cases, whose GENERIC + * lines reappear and whose total-warn pins therefore also move), and leaves + * every verdict assertion and the whole of group 4 GREEN. That asymmetry is + * the card restated: the change moves the silence, not the answer. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import React from 'react'; +import { UNRESOLVABLE_VISIBILITY_PREFIX } from '../utils/visibilityDiagnostic'; + +const NAME = 'probe-6038'; +const TYPE = 'element:probe-6038'; + +const Probe = () =>
; + +/** The ambient scope app-shell's `ExpressionProvider` really mounts. */ +const APP_SCOPE = { + current_user: { id: 'u1' }, + user: { id: 'u1' }, + app: {}, + data: {}, + features: {}, +}; +const ADAPTER = { total: 99 }; +const ROW = { id: 'r1', status: 'open' }; + +/** + * Predicates that genuinely FAULT, one per dialect, measured on the built + * evaluator. An unbound ROOT identifier is the shape #11254 hit and the shape + * an AI-authored predicate reaches for; `record.bad(` is a parse error, which + * is the only fault the CEL engine reports as `[parse]`. + */ +const FAULT_BARE = 'nosuchroot.x > 1'; +const FAULT_BARE_2 = 'anotherbadroot.y == 3'; +const FAULT_TEMPLATE = '${nosuchroot.x > 1}'; +const FAULT_CEL = { dialect: 'cel', source: 'record.bad(' }; +/** A predicate that resolves cleanly — the silence that makes the noise mean something. */ +const HEALTHY = "record.status == 'open'"; + +type WarnSpy = { mock: { calls: unknown[][] } }; +const spyWarn = () => vi.spyOn(console, 'warn').mockImplementation(() => {}); +/** The PREFIX-FILTERED view every assertion reads. */ +const reports = (warn: WarnSpy): string[] => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX)); +/** Everything the console was asked to print, filtered by nothing. */ +const allWarnings = (warn: WarnSpy): string[] => warn.mock.calls.map((c) => String(c[0])); + +/** + * Mount `schemas` in a PRODUCTION module graph. + * + * `__DEV__` in `SchemaRenderer` is an IIFE evaluated at module load, so the env + * has to be stubbed before the import — hence `resetModules` and a dynamic + * import in the test BODY (the case `object-ui/no-dynamic-import-in-test-hook` + * exempts). The dedupe `Set` is module state of that same fresh graph, so the + * reset has to come from the fresh graph too: resetting the statically-imported + * copy would clear a DIFFERENT `Set` and every count below would be measuring + * leakage from the previous case. + */ +async function inProduction( + fn: (mount: (schemas: Record[]) => void) => void | Promise, +): Promise { + vi.resetModules(); + vi.stubEnv('NODE_ENV', 'production'); + try { + const [core, prod, ctx, rec, expr, diag] = await Promise.all([ + import('@object-ui/core'), + import('../SchemaRenderer'), + import('../context/SchemaRendererContext'), + import('../context/RecordContext'), + import('../hooks/useExpression'), + import('../utils/visibilityDiagnostic'), + ]); + diag.__resetVisibilityPredicateWarnings(); + core.ComponentRegistry.register(NAME, Probe as never, { + namespace: 'element', + skipFallback: true, + } as never); + const mount = (schemas: Record[]) => + render( + + + + {schemas.map((s, i) => ( + + ))} + + + , + ); + await fn(mount); + core.ComponentRegistry.unregister?.(NAME, 'element'); + } finally { + cleanup(); + vi.unstubAllEnvs(); + vi.resetModules(); + } +} + +const shownCount = () => screen.queryAllByTestId('probe').length; + +beforeEach(() => { + vi.restoreAllMocks(); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/* -------------------------------------------------------------------------- * + * Group 0 — the controls that decide whether the rest of the file means + * anything. Both are about the CAPTURE, not about the renderer. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 0 — capture controls', () => { + it('POSITIVE CONTROL: the spy really observes a line carrying the prefix', () => { + // Without this, every `toHaveLength(0)` in this file is equally green on a + // capture that observes nothing at all. + const warn = spyWarn(); + console.warn(`${UNRESOLVABLE_VISIBILITY_PREFIX} - synthetic control line`); + expect(reports(warn)).toHaveLength(1); + }); + + it('DEGENERATE CONTROL: unrelated console output does NOT satisfy the pin', () => { + // The assertions read a prefix-filtered view precisely so that noise — a + // React key warning, a deprecation notice, another diagnostic — can never + // stand in for this diagnostic. A test that counted raw `console.warn` + // calls would pass on a build that emitted only the noise. + const warn = spyWarn(); + console.warn('[object-ui] some entirely unrelated warning'); + console.warn('Warning: each child in a list should have a unique "key" prop.'); + expect(allWarnings(warn)).toHaveLength(2); + expect(reports(warn)).toHaveLength(0); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 1 — THE acceptance criterion, per dialect. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 1 — a faulting node gate is loud in production', () => { + it('THE acceptance criterion: a BARE-STRING `visibleWhen` fault warns in a production build (it printed NOTHING before)', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + + // Verdict FIRST — observability only. The gate still fails open, which + // is exactly the property that made the silence dangerous. + expect(shownCount()).toBe(1); + + const lines = reports(warn); + expect(lines).toHaveLength(1); + // The line has to identify the predicate, the gate and the node, or it + // is not a diagnostic — it is an alarm with no address. + expect(lines[0]).toContain(TYPE); + expect(lines[0]).toContain('n1'); + expect(lines[0]).toContain('visibleWhen'); + expect(lines[0]).toContain(FAULT_BARE); + expect(lines[0]).toContain('Reason:'); + // ONE fault, ONE line: nothing else was printed alongside it. + expect(allWarnings(warn)).toHaveLength(1); + }); + }); + + it('a `{ dialect: "cel" }` envelope fault warns ONCE — the generic line is replaced, not joined', async () => { + // Before this card the CEL branch already printed the canonical engine's + // own generic line here. The risk of this card was two lines for one fault; + // the total-count pin is what refuses it. + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_CEL }]); + expect(shownCount()).toBe(1); + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('record.bad('); + expect(allWarnings(warn)).toHaveLength(1); + }); + }); + + it('a `${…}` template fault warns ONCE across many evaluations — it used to print one line PER evaluation', async () => { + // The flood direction. Three nodes carrying the same broken template used + // to produce three generic lines and would produce three per re-render. + await inProduction((mount) => { + const warn = spyWarn(); + mount([ + { id: 'n1', visibleWhen: FAULT_TEMPLATE }, + { id: 'n2', visibleWhen: FAULT_TEMPLATE }, + { id: 'n3', visibleWhen: FAULT_TEMPLATE }, + ]); + expect(shownCount()).toBe(3); + expect(reports(warn)).toHaveLength(1); + expect(allWarnings(warn)).toHaveLength(1); + }); + }); + + it('the `hidden` leg — the NON-negated polarity — reports too, and still hides', async () => { + // `hidden` / `hiddenOn` are the two legs whose fail-soft `true` means HIDE. + // A fault there removes the node, which is the louder screen symptom and + // the quieter console one; it must not be a second silence. + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', hidden: FAULT_BARE }]); + expect(shownCount()).toBe(0); + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('hidden'); + }); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 2 — the rate limit: BOTH halves. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 2 — deduped per predicate SOURCE, not per render and not per node instance', () => { + it('three nodes with DIFFERENT ids and the SAME predicate source produce exactly ONE line', async () => { + // This is the "not the call-site instance" half. The ids differ, so a key + // that included the node instance (or the schema object) would emit three. + await inProduction((mount) => { + const warn = spyWarn(); + mount([ + { id: 'alpha', visibleWhen: FAULT_BARE }, + { id: 'beta', visibleWhen: FAULT_BARE }, + { id: 'gamma', visibleWhen: FAULT_BARE }, + ]); + expect(shownCount()).toBe(3); + expect(reports(warn)).toHaveLength(1); + }); + }); + + it('re-rendering the same faulting predicate does NOT add a line — the "not per render" half', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + cleanup(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + cleanup(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + expect(reports(warn)).toHaveLength(1); + }); + }); + + it('a SECOND distinct predicate source still warns — without this, a dedupe that suppresses everything looks identical', async () => { + // The half that a "warned at least once" test cannot see. An implementation + // that printed one line ever, for the life of the page, passes every pin + // above and fails this one. + await inProduction((mount) => { + const warn = spyWarn(); + mount([ + { id: 'n1', visibleWhen: FAULT_BARE }, + { id: 'n2', visibleWhen: FAULT_BARE_2 }, + ]); + expect(shownCount()).toBe(2); + const lines = reports(warn); + expect(lines).toHaveLength(2); + expect(lines.some((l) => l.includes(FAULT_BARE))).toBe(true); + expect(lines.some((l) => l.includes(FAULT_BARE_2))).toBe(true); + }); + }); + + it('a two-hundred-row list of ONE broken predicate is one line, not two hundred', async () => { + // The scenario the ruling's own cost table names ("一个列表里几百行会淹没 + // 控制台"). It is the same property as the case above, at the scale that + // decides whether option B is usable. + await inProduction((mount) => { + const warn = spyWarn(); + mount(Array.from({ length: 200 }, (_, i) => ({ id: `row-${i}`, visibleWhen: FAULT_BARE }))); + expect(shownCount()).toBe(200); + expect(reports(warn)).toHaveLength(1); + expect(allWarnings(warn)).toHaveLength(1); + }); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 3 — the silence that makes the noise mean something. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 3 — nothing else became loud', () => { + it('a HEALTHY predicate prints nothing, on either verdict', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: HEALTHY }]); + expect(shownCount()).toBe(1); + cleanup(); + mount([{ id: 'n2', visibleWhen: "record.status == 'closed'" }]); + expect(shownCount()).toBe(0); + expect(allWarnings(warn)).toHaveLength(0); + }); + }); + + it('a node with NO gate at all prints nothing and renders', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1' }]); + expect(shownCount()).toBe(1); + expect(allWarnings(warn)).toHaveLength(0); + }); + }); + + it('objectui#5687 stays DEV-ONLY: an adapter-only `data.*` predicate is silent in production', async () => { + // Deliberately NOT extended by this card. That leg reports a predicate that + // evaluated perfectly against the wrong object — not a fault — and its own + // ruling (2026-08-22, option A) scoped it to development. Pinned here so + // "production is louder" cannot quietly become "production reports + // everything the dev build reports". + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: "data.status == 'draft'" }]); + // Verdict: the constant-false still hides, exactly as documented. + expect(shownCount()).toBe(0); + expect(allWarnings(warn)).toHaveLength(0); + }); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 4 — development is unchanged, and says the same words. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 4 — dev and production print the IDENTICAL line', () => { + it('the same fault produces the same message in a development build', async () => { + // Two builds, one message. This is what makes "the `__DEV__` gate now + // decides HOW the fault is detected, not WHETHER it is reported" a fact + // rather than a comment: a production-only message would be a second + // diagnostic to keep in sync, and they would drift. + const { __resetVisibilityPredicateWarnings } = await import('../utils/visibilityDiagnostic'); + const { SchemaRenderer } = await import('../SchemaRenderer'); + const { SchemaRendererContext } = await import('../context/SchemaRendererContext'); + const { RecordContextProvider } = await import('../context/RecordContext'); + const { PredicateScopeProvider } = await import('../hooks/useExpression'); + const { ComponentRegistry } = await import('@object-ui/core'); + ComponentRegistry.register(NAME, Probe as never, { + namespace: 'element', + skipFallback: true, + } as never); + __resetVisibilityPredicateWarnings(); + + const warn = spyWarn(); + render( + + + + + + + , + ); + const devLines = reports(warn); + expect(devLines).toHaveLength(1); + expect(devLines[0]).toContain(TYPE); + expect(devLines[0]).toContain(FAULT_BARE); + // Dev still fails open too — the verdict is one behaviour, not two. + expect(shownCount()).toBe(1); + ComponentRegistry.unregister?.(NAME, 'element'); + + // Restore before spying again: `vi.spyOn` on an already-spied method hands + // back the SAME spy, so a second `spyWarn()` here would carry the dev call + // into the production reading and every count below would be off by the + // lines this half already made. (Measured — it is how this case first + // failed, at 2 lines instead of 1.) + vi.restoreAllMocks(); + + // …and the production graph's line for the same fault, word for word. + let prodLine = ''; + await inProduction((mount) => { + const prodWarn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + const lines = reports(prodWarn); + expect(lines).toHaveLength(1); + prodLine = lines[0]; + }); + expect(prodLine).toBe(devLines[0]); + }); +}); diff --git a/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx b/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx index af90f8531c..4e8e956a12 100644 --- a/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx +++ b/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx @@ -364,8 +364,26 @@ describe('#5454 leg 3 — an unresolvable predicate is loud, and its verdict is cleanup(); mountProd({ hidden: cel('record.nope.deeper == 1') }, undefined); expect(shown()).toBe(false); - // ...but with none of this module's diagnostic, which is dev-only. - expect(warn.mock.calls.map(c => String(c[0])).filter(m => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX))).toHaveLength(0); + // ...and, since objectui#6038, WITH this module's diagnostic — one line + // per distinct faulting predicate source, in production too. + // + // This assertion used to read `toHaveLength(0)`, and the change is the + // whole of the maintainer's 2026-08-25 ruling (option B): "A is rejected + // — the silence is no longer an accepted property." The pin is rewritten + // rather than deleted, because what it was really guarding is the half + // that did NOT move: every verdict above is untouched, and the branch is + // still the SINGLE-evaluation one (production never pays for the + // `throwOnError` probe — `EvaluationOptions.onFault` reports the fault + // the evaluator had already caught). + // + // TWO lines, not one: the two mounts above carry two DIFFERENT predicate + // sources (`record.status == 'in_review'` on `visibleWhen`, and + // `record.nope.deeper == 1` on `hidden`). One line each is the dedupe + // working; one line total would mean it had swallowed the second. + const produced = warn.mock.calls.map(c => String(c[0])).filter(m => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX)); + expect(produced).toHaveLength(2); + expect(produced.some(m => m.includes("record.status == 'in_review'"))).toBe(true); + expect(produced.some(m => m.includes('record.nope.deeper == 1'))).toBe(true); core.ComponentRegistry.unregister?.(NAME, 'element'); } finally { vi.unstubAllEnvs(); diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index be8ea1124b..559790ede3 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -20,6 +20,20 @@ export * from './element-data-source/ElementDataSourceGate.js'; // i18n utilities export { resolveKeyedI18nLabel } from './utils/i18n.js'; +// Node-gate predicate diagnostics. Exported at the package entry (objectui#6038) +// so every surface that evaluates a node `visibleWhen` reports a fault through +// ONE reporter and ONE dedupe `Set` — `page:tabs` item predicates live in +// `@object-ui/components`, which depends on this package. A second copy of the +// reporter would mean a second rate limit, and the same broken predicate would +// then be entitled to one line per package instead of one line, which is the +// property the 2026-08-25 ruling asked for. +export { + reportUnresolvableVisibilityPredicate, + formatUnresolvableVisibilityMessage, + UNRESOLVABLE_VISIBILITY_PREFIX, + __resetVisibilityPredicateWarnings, +} from './utils/visibilityDiagnostic.js'; + // Write-error surfacing utilities (shared by drag-write plugins so a failed // PATCH — e.g. an RLS 403 — is never silently swallowed). export { extractWriteErrorMessage, isPermissionError, extractFieldErrors, classifyLoadError, declaredUserMessage } from './utils/error-message.js'; diff --git a/packages/react/src/utils/visibilityDiagnostic.ts b/packages/react/src/utils/visibilityDiagnostic.ts index f8aa316fe8..673b2fe521 100644 --- a/packages/react/src/utils/visibilityDiagnostic.ts +++ b/packages/react/src/utils/visibilityDiagnostic.ts @@ -7,8 +7,9 @@ */ /** - * Dev-build diagnostic: a visibility predicate could not be evaluated - * (objectui#5454, leg 3 of the 2026-08-21 ruling). + * Diagnostic: a visibility predicate could not be evaluated (objectui#5454, + * leg 3 of the 2026-08-21 ruling; production coverage added by objectui#6038, + * maintainer ruling 2026-08-25 option B). * * ## The defect this names * @@ -93,15 +94,34 @@ export function formatUnresolvableVisibilityMessage( * repeat the line. Keyed on the predicate TEXT rather than the schema object: * the same broken predicate authored once and rendered over many rows is ONE * authoring bug, and an object key would report it once per row. + * + * This is the RATE LIMIT the 2026-08-25 ruling requires of the production leg, + * and it is why that leg can be a plain `console.warn`: the ceiling is not "one + * line per render" but "one line per distinct authored predicate", for the + * lifetime of the page. Two properties have to hold together, and a test that + * pins only the first cannot tell a working dedupe from one that suppresses + * everything — so objectui#6038 pins both: N renders of ONE faulting predicate + * emit exactly one line, and a SECOND distinct predicate source still emits. */ const _warnedVisibilityPredicates = new Set(); /** - * Dev-build only; the caller applies the gate, so this module is dead code in a - * production build. `console.warn`, not `error`: the verdict is unchanged and - * the page still renders, so this is a diagnostic about a predicate — not the - * refusal `reportUnevaluatedExpressions` emits once raw source has reached the - * DOM. + * Reports a visibility predicate that could not be evaluated — in DEVELOPMENT + * AND IN PRODUCTION since objectui#6038 (maintainer ruling 2026-08-25, option + * B: "the silence is no longer an accepted property"). + * + * `console.warn`, not `error`: the verdict is unchanged and the page still + * renders, so this is a diagnostic about a predicate — not the refusal + * `reportUnevaluatedExpressions` emits once raw source has reached the DOM. + * + * ## `err` takes a reason, not only an `Error` + * + * The dev caller catches a throw and passes the `Error`; the production caller + * is handed the evaluator's own reason STRING through `EvaluationOptions.onFault` + * (no throw is raised there, because raising one would cost a second + * evaluation). `String(err)` already covered that shape, so both callers reach + * the same `Reason:` text and the same dedupe entry — which is what makes "dev + * and production print the identical line" true rather than approximately true. */ export function reportUnresolvableVisibilityPredicate( type: unknown, @@ -139,8 +159,20 @@ export function __resetVisibilityPredicateWarnings(): void { * dev-only unresolvable-predicate report". What is load-bearing there — and * what the dispatch restated — is the reporter's POSTURE: same module, same * severity (`console.warn`), same dedupe key shape, same dedupe Set, same - * dev-only gate at the same call site, same test-only reset. All of that is - * shared below. The first LINE is not reused, because on this path it would + * gate at the same call site, same test-only reset. All of that is shared + * below. + * + * ⚠️ The two legs stopped sharing that gate's VALUE in objectui#6038, and only + * that. The unresolvable leg now reports in production as well, because the + * 2026-08-25 ruling retired its silence; THIS leg stays dev-only under its own + * 2026-08-22 ruling, and the difference is not an oversight. A fault is a + * predicate that could not be evaluated, and shipping a live gate that has + * stopped biting is the class-1 defect production has to be able to see. This + * leg reports something else: a predicate that evaluated perfectly, against the + * wrong object. Its trigger is a LEXICAL scan of the predicate source with a + * stated false-positive residue (the deliberate-absence idioms below), which is + * a cost worth paying for an author at their keyboard and not for every user of + * every production page. The first LINE is not reused, because on this path it would * state something false: the predicate did not fail to evaluate. It evaluated * perfectly, against the wrong object, and produced a constant. Telling an * author "could not be evaluated" would send them hunting for a syntax error @@ -294,7 +326,9 @@ export function formatAdapterOnlyDataMessage( * objectui#5454 existed to remove, one path further along. * * Sharing the module means sharing the LIFECYCLE, which is the part that has to - * match: one dedupe Set, one reset, one severity, one dev-only gate. The dedupe + * match: one dedupe Set, one reset, one severity. (One gate, too, until + * objectui#6038 — see the prefix constant above for why only the sibling leg + * crossed into production.) The dedupe * key is tagged with this leg's name so the two diagnostics cannot silence each * other for the same (type, key, source) triple — they are different faults, * and a node that faults one way is not evidence about the other.