From 9d204edf3fdba67b79ecd4e3c2b39c7ab358e6bc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 02:48:13 +0000 Subject: [PATCH 1/2] feat(core,react): declare `record.*` the row-predicate canon and warn on the two deprecated spellings (#5330) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the maintainer ruling of 2026-08-20 (option B): the canon is `record.*`; the bare shorthand and `data.*` enter a deprecation window, warned about now and removable only after a stored-metadata survey. The canon states the SERVER's accept set — the ruling's stated first measurement. Measured on `@objectstack/formula@17.1.0`: `buildScope({ record })` mounts exactly `['record']`, so a bare field faults `Unknown variable: status` and `data.*` faults `Unknown variable: data`. The renderer's three-way binding has no server counterpart. `data.*` is lint-silent and runtime-fatal: `data` IS in SCOPE_ROOTS (a generous "never faults" lint baseline, not the runtime accept set), so it passes every authoring gate and then binds nothing — a constant false, which for `visible` is a button that silently never appears. No spelling is removed; no predicate changes verdict. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01E7snar5mwF7qoXJazqKhys --- .changeset/row-predicate-record-canon-5330.md | 59 ++++ .../__tests__/rowPredicateCanon.test.ts | 150 +++++++++++ packages/core/src/evaluator/index.ts | 1 + .../core/src/evaluator/listConditional.ts | 34 ++- .../core/src/evaluator/rowPredicateCanon.ts | 251 ++++++++++++++++++ packages/react/src/hooks/useExpression.ts | 68 ++++- 6 files changed, 557 insertions(+), 6 deletions(-) create mode 100644 .changeset/row-predicate-record-canon-5330.md create mode 100644 packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts create mode 100644 packages/core/src/evaluator/rowPredicateCanon.ts diff --git a/.changeset/row-predicate-record-canon-5330.md b/.changeset/row-predicate-record-canon-5330.md new file mode 100644 index 0000000000..cfd76abde8 --- /dev/null +++ b/.changeset/row-predicate-record-canon-5330.md @@ -0,0 +1,59 @@ +--- +'@object-ui/core': minor +'@object-ui/react': minor +--- + +Row predicates declare a canon: `record.*`. The bare shorthand and `data.*` now +warn once, and are unchanged otherwise. + +A row predicate (`visible` / `disabled` / `enabled` on an action renderer, a row +scope, a `record:alert`) has bound the row three ways since objectui#4075 — +`record.status`, bare `status`, and `data.status` — without any of them being +declared the contract. The maintainer ruled that question on 2026-08-20 +(objectui#5330, option B), mirroring the objectstack#7917 option-② precedent for +the identical renderer-tolerance shape: **the canon is `record.*`**, and the +other two enter a deprecation window. + +The canon states the **server's** accept set, which was this card's first +measurement and turns out to be strictly narrower than the renderer's. Measured +against `@objectstack/formula@17.1.0`, the engine the server evaluates with: + +| spelling | server runtime | server authoring oracle | +|---|---|---| +| `record.status` | `{ ok: true, value: true }` | accepted | +| bare `status` | `Unknown variable: status` | refused | +| `data.status` | `Unknown variable: data` | **silently accepted** | + +`buildScope({ record })` mounts exactly `['record']` — `data` is never bound and +the row's fields are never flattened to top level. The three-way binding is a +client tolerance with no server counterpart, which is why the warning belongs on +this side. + +`data.*` is the dangerous one, and the reason the warning exists. `data` is in +`@objectstack/formula`'s `SCOPE_ROOTS`, so the server's bare-identifier oracle +waves it through — that list is a deliberately generous "never faults" lint +baseline, not the runtime accept set. A `data.*` row predicate therefore passes +every authoring gate the platform has and then binds nothing at runtime: not an +error, a constant `false`. A `visible` that is constantly false is a button that +silently never appears — the objectui#4075 fail-closed signature. + +What ships: + +- `@object-ui/core` exports `detectNonCanonicalRowSpelling`, + `warnNonCanonicalRowSpelling`, `resetRowPredicateCanonWarnings` and + `ROW_PREDICATE_CANONICAL_ROOT` from a new `evaluator/rowPredicateCanon.ts`, + which carries the canon statement and the measurement. +- Both evaluation tiers report once, in dev: `evalRowPredicate` (core) and + `useCondition` (react, for bags bound by `usePredicateRecordContext`). +- Detection reuses the server's own oracles (`collectCelRootIdentifiers`, + `firstUndeclaredReference`) rather than a regex, so no second dialect + judgement is invented client-side. + +**No spelling is removed and no behaviour changes.** Every predicate that +resolved before resolves now — the ruling defers removal behind a stored-metadata +survey, and the warning is what makes that survey possible (ADR-0078: a +tolerance nothing ever reports can never be retired). + +The deprecation is scoped to the **runtime record layer**. `data` remains the +canonical root one layer over, in a metadata-editing form (ADR-0089 D3 +`CANONICAL_ROOT_BY_LAYER`), and the detector stands down there. diff --git a/packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts b/packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts new file mode 100644 index 0000000000..3ff42bb991 --- /dev/null +++ b/packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts @@ -0,0 +1,150 @@ +/** + * 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#5330 — the row-predicate spelling CANON (`record.*`) and its Phase-1 + * deprecation warning. + * + * These pins are deliberately split in two, because the card's two halves fail + * in opposite directions: + * + * - the CANON pins assert the binding is UNCHANGED. The ruling defers every + * removal behind a stored-metadata survey, so a test that stopped resolving + * the shorthand would be the regression, not the feature. + * - the WARNING pins assert the tolerance is no longer silent — the ADR-0078 + * reason a tolerance nothing reports can never be retired. + * + * The `record-alert` renderer's own three-spelling pins landed separately with + * PR #5688 (`plugin-detail/.../record-alert.rowBinding.test.tsx`) and are NOT + * duplicated here; this file pins the shared evaluator tier those renderers sit + * on, plus the detector itself. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { + detectNonCanonicalRowSpelling, + resetRowPredicateCanonWarnings, + ROW_PREDICATE_CANONICAL_ROOT, + evalRowPredicate, +} from '../index.js'; + +const row = { status: 'in_review', amount: 10 }; + +beforeEach(() => resetRowPredicateCanonWarnings()); + +describe('[#5330] the canon is `record.*`', () => { + it('names `record` as the one canonical root', () => { + expect(ROW_PREDICATE_CANONICAL_ROOT).toBe('record'); + }); + + it('reports nothing for the canonical spelling', () => { + expect(detectNonCanonicalRowSpelling("record.status == 'in_review'", row, true)).toBeNull(); + }); +}); + +describe('[#5330] non-canonical spellings are DETECTED', () => { + it('reports the bare shorthand, and names the canonical rewrite', () => { + expect(detectNonCanonicalRowSpelling("status == 'in_review'", row, true)).toEqual({ + kind: 'bare-shorthand', + identifier: 'status', + canonical: 'record.status', + }); + }); + + it('reports a `data.`-rooted predicate on a record surface', () => { + expect(detectNonCanonicalRowSpelling("data.status == 'in_review'", row, true)).toEqual({ + kind: 'metadata-layer-root', + identifier: 'data', + canonical: 'record', + }); + }); +}); + +/** + * Every case here is a spelling the detector must NOT report. They are the + * whole reason it consults the row and the caller's binding rather than + * pattern-matching the source: a false deprecation warning sends an author to + * rewrite a predicate that was correct. + */ +describe('[#5330] the detector stands down rather than guessing', () => { + it('leaves `data.*` alone when `data` is NOT this row (rowless / metadata-editing layer)', () => { + // ADR-0089 D3: `data` is the CANONICAL root of a metadata-editing form. + // Reporting it there would contradict that layer's own contract. + expect(detectNonCanonicalRowSpelling("data.status == 'in_review'", row, false)).toBeNull(); + }); + + it('leaves a host-scope root alone', () => { + expect(detectNonCanonicalRowSpelling('features.beta == true', row, true)).toBeNull(); + }); + + it('leaves an undeclared identifier alone when it is not a field of THIS row', () => { + // A deployment global this module cannot see is not the #4075 shorthand. + expect(detectNonCanonicalRowSpelling('unknownGlobal == 1', row, true)).toBeNull(); + }); + + it('leaves an unparseable source alone — syntax is another gate’s verdict', () => { + expect(detectNonCanonicalRowSpelling("record.status === 'x'", row, true)).toBeNull(); + }); +}); + +describe('[#5330] `evalRowPredicate` — the binding is UNCHANGED (no removal before the survey)', () => { + it('still resolves all three spellings against the row', () => { + expect(evalRowPredicate("record.status == 'in_review'", row)).toBe(true); + expect(evalRowPredicate("status == 'in_review'", row)).toBe(true); + expect(evalRowPredicate("data.status == 'in_review'", row)).toBe(true); + }); + + it('still tells the three spellings apart on a NON-matching row (not vacuously true)', () => { + const other = { status: 'draft', amount: 1 }; + expect(evalRowPredicate("record.status == 'in_review'", other)).toBe(false); + expect(evalRowPredicate("status == 'in_review'", other)).toBe(false); + expect(evalRowPredicate("data.status == 'in_review'", other)).toBe(false); + }); +}); + +describe('[#5330] `evalRowPredicate` — the tolerance is no longer silent', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => warn.mockRestore()); + + const deprecationWarnings = (): string[] => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('DEPRECATED spelling')); + + it('warns on the bare shorthand and prescribes `record.status`', () => { + evalRowPredicate("status == 'in_review'", row, { label: 'row action "approve"' }); + const msgs = deprecationWarnings(); + expect(msgs).toHaveLength(1); + expect(msgs[0]).toContain('record.status'); + expect(msgs[0]).toContain('objectui#5330'); + expect(msgs[0]).toContain('row action "approve"'); + }); + + it('warns on `data.*` and says the server binds no `data` at all', () => { + evalRowPredicate("data.status == 'in_review'", row); + const msgs = deprecationWarnings(); + expect(msgs).toHaveLength(1); + expect(msgs[0]).toContain('metadata-editing-form root'); + }); + + it('stays silent for the canonical spelling', () => { + evalRowPredicate("record.status == 'in_review'", row); + expect(deprecationWarnings()).toHaveLength(0); + }); + + it('warns ONCE per (label, predicate) — these run on every row of every frame', () => { + for (let i = 0; i < 5; i++) evalRowPredicate("status == 'in_review'", row, { label: 'grid' }); + expect(deprecationWarnings()).toHaveLength(1); + }); + + it('does NOT report a legacy `${…}`-dialect predicate, where `data.*` is the normal spelling', () => { + evalRowPredicate('${data.status === "in_review"}', row); + expect(deprecationWarnings()).toHaveLength(0); + }); +}); diff --git a/packages/core/src/evaluator/index.ts b/packages/core/src/evaluator/index.ts index 0a6d8dea1d..6308ba33a5 100644 --- a/packages/core/src/evaluator/index.ts +++ b/packages/core/src/evaluator/index.ts @@ -11,6 +11,7 @@ export * from './ExpressionEvaluator.js'; export * from './predicateInput.js'; export * from './declaredPredicate.js'; export * from './fieldRules.js'; +export * from './rowPredicateCanon.js'; export * from './listConditional.js'; export * from './optionRules.js'; export * from './optionLint.js'; diff --git a/packages/core/src/evaluator/listConditional.ts b/packages/core/src/evaluator/listConditional.ts index 800b76a44c..23c4b2168d 100644 --- a/packages/core/src/evaluator/listConditional.ts +++ b/packages/core/src/evaluator/listConditional.ts @@ -37,6 +37,7 @@ import { evalFieldPredicate, type FieldRulePredicate } from './fieldRules.js'; import { ExpressionEvaluator } from './ExpressionEvaluator.js'; import { toPredicateRecord, type FieldContainerLike } from '../utils/predicate-record.js'; +import { warnNonCanonicalRowSpelling } from './rowPredicateCanon.js'; /** * Syntax that only the legacy JS-dialect evaluator understands and that is NOT @@ -206,8 +207,27 @@ export interface RowPredicateOptions { * Evaluate a single boolean predicate against a row record on the canonical CEL * engine (with a legacy-dialect fallback — see the module note). The row's * fields are bound three ways so every authoring convention resolves: - * `record.status` (spec/canonical), bare `status` (row-action shorthand), and - * `data.status` (legacy). The optional `scope` (host predicate scope) is bound + * `record.status`, bare `status` (row-action shorthand), and `data.status`. + * + * ⚠️ Those three are NOT peers, and this doc comment used to read as though + * they were. **The canon is `record.*`** (maintainer ruling 2026-08-20 on + * objectui#5330, option B); the other two are client tolerances in a + * deprecation window, kept because stored metadata carries them and warned + * about — from the CEL path below — by `warnNonCanonicalRowSpelling`. The + * server accepts `record.*` and NOTHING else: measured on + * `@objectstack/formula@17.1.0`, `buildScope({ record })` mounts exactly + * `['record']`, so a bare field faults `Unknown variable: status` there and a + * `data.*` predicate faults `Unknown variable: data`. See + * {@link ./rowPredicateCanon.ts} for the full measurement, for why `data.*` is + * the dangerous one (it is silently ACCEPTED by the server's authoring oracle + * and still binds nothing at runtime), and for why the deprecation is scoped to + * this runtime layer rather than declared platform-wide (`data` is the + * canonical root of a metadata-editing form — ADR-0089 D3). + * + * ⛔ No spelling is removed from the binding, and none may be before a + * stored-metadata survey sizes the window — that is part of the same ruling. + * + * The optional `scope` (host predicate scope) is bound * alongside so `features.*` / `user.*` predicates keep working — but the row is * the subject: `record` and `data` always name THIS row, on both dialect paths, * even when the host scope carries keys of those names (objectui#3796). @@ -287,6 +307,16 @@ export function evalRowPredicate( } } + // CEL path — everything reaching here is CEL, which is what makes this the + // right place for the objectui#5330 spelling warning: in the legacy `${…}` + // dialect above, `data.*` is the NORMAL spelling, so reporting it there would + // be a false positive on every legacy predicate. `data` names THIS row unless + // the caller is `rowless` (then the host scope keeps its own — see the + // option), which is exactly the condition the detector needs. + if (predicateText !== '(expression)') { + warnNonCanonicalRowSpelling(predicateText, rowObj, !opts.rowless, opts.label); + } + // CEL path. The fault-aware `evalCel` costs two evaluations (to tell a fault // from a genuine `false`), so only pay it when a caller wants the labelled // fail-closed warning — the formatting hot path takes the single-eval fast diff --git a/packages/core/src/evaluator/rowPredicateCanon.ts b/packages/core/src/evaluator/rowPredicateCanon.ts new file mode 100644 index 0000000000..748af51ee1 --- /dev/null +++ b/packages/core/src/evaluator/rowPredicateCanon.ts @@ -0,0 +1,251 @@ +/** + * 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. + */ + +/** + * The row-predicate spelling CANON, and the Phase-1 deprecation warning for the + * two spellings that are not it (objectui#5330). + * + * ## The canon (maintainer ruling, 2026-08-20 — option B) + * + * A row predicate — `visible` / `disabled` / `enabled` on an action renderer, a + * row scope, a `record:alert` — binds the row THREE ways today: canonical + * `record.status`, bare shorthand `status`, and legacy `data.status` + * (objectui#4075 / PR #4079 bound all four action renderers all three ways to + * restore consistency, deliberately without deciding which of them is + * CONTRACT). This module is where that decision now lives: + * + * > **The canon is `record.*`.** The bare shorthand and `data.*` are + * > client-side tolerances in a deprecation window, kept because stored + * > metadata carries them, warned about here, and removable only after a + * > stored-metadata survey sizes the window (⛔ no removal before the survey). + * + * It mirrors the objectstack#7917 option-② precedent for the identical shape (a + * renderer tolerance quietly becoming a second de-facto contract — AGENTS.md + * #0.1), whose objectui half is `utils/dashboard-filters.ts`' bare-string + * `options` shorthand: same three phases, same reason the warning is not + * decoration (ADR-0078 — nothing silently inert; a tolerance nothing ever + * reports can never be retired, because nothing would ever show that the last + * document carrying it is gone). + * + * ## The canon states the SERVER's accept set, not this client's + * + * The ruling made that the dev's first measurement, because a canon that only + * describes the renderer would be the very thing it exists to end. Measured + * against `@objectstack/formula@17.1.0` — the engine the server evaluates with, + * and the one `fieldRules.ts` already delegates to: + * + * | spelling | server runtime (`buildScope` + `celEngine`) | server authoring oracle | + * |---|---|---| + * | `record.status` | ✅ `{ ok: true, value: true }` | ✅ accepted | + * | bare `status` | ❌ `Unknown variable: status` | ❌ refused (`'status'`) | + * | `data.status` | ❌ `Unknown variable: data` | ⚠️ **silently accepted** | + * + * `buildScope({ record })` mounts exactly `['record']` — `data` is never bound + * and the row's fields are never flattened to top level. So **the server + * accepts `record.*` and nothing else**; both other spellings fault there. The + * three-way binding is a client tolerance with no server counterpart, which is + * precisely why it is the client's job to warn. + * + * ⚠️ The `data.*` row is the dangerous one and the reason this warning exists + * at all. `data` IS in `@objectstack/formula`'s `SCOPE_ROOTS`, so the server's + * bare-identifier oracle waves `data.status` through — that list is a + * deliberately generous "never faults" LINT BASELINE, not the runtime accept + * set. A `data.*` row predicate therefore passes every authoring gate the + * platform has and then binds nothing at runtime: it is not an error, it is a + * constant `false`, and a `visible` that is constantly false is a button that + * silently never appears. That is the #4075 fail-closed family's exact + * signature, and this client is the only layer positioned to catch it. + * + * ## `data.*` is DEPRECATED HERE, not everywhere — the canon is layer-scoped + * + * `data` is the CANONICAL root one layer over, in a metadata-editing form (the + * row under edit): objectstack's `CANONICAL_ROOT_BY_LAYER` reads + * `{ runtime: 'record', metadata: 'data' }` (ADR-0089 D3), and objectui's own + * `app-shell` metadata-admin `SchemaForm` binds `{ data: row }` on purpose. So + * `data.*` is not a legacy alias to be deprecated platform-wide — it is a + * WRONG-LAYER paste on a runtime record surface, and only that is what this + * module reports. Stating the deprecation unqualified would contradict + * ADR-0089 D3 and break the metadata-editing layer's own contract. + * + * ## Why the detection reuses the server's oracle instead of a regex + * + * `collectCelRootIdentifiers` and `firstUndeclaredReference` are + * `@objectstack/formula` exports — the same two oracles objectstack's + * `visibility-bare-identifier` gate uses. Reading roots off the canonical AST + * rather than pattern-matching the source is what keeps this from inventing a + * second dialect judgement client-side: a string this module cannot parse is + * NOT this module's verdict to give (syntax belongs to the gates that own it), + * and it stands down rather than guessing. + */ + +import { collectCelRootIdentifiers, firstUndeclaredReference } from '@objectstack/formula'; + +/** The one canonical row-predicate root. */ +export const ROW_PREDICATE_CANONICAL_ROOT = 'record'; + +/** + * The metadata-editing-form root (ADR-0089 D3). Canonical on THAT layer, and a + * wrong-layer paste on a runtime record surface — see the module note. + */ +const METADATA_LAYER_ROOT = 'data'; + +/** A non-canonical row-predicate spelling, and the edit that fixes it. */ +export type NonCanonicalRowSpelling = + | { + /** Bare shorthand: `status == 'active'` — a field of THIS row, unrooted. */ + kind: 'bare-shorthand'; + /** The offending identifier (`status`). */ + identifier: string; + /** What the author should write instead (`record.status`). */ + canonical: string; + } + | { + /** Wrong-layer root: `data.status == 'active'` on a runtime record surface. */ + kind: 'metadata-layer-root'; + identifier: string; + canonical: string; + }; + +/** + * Detect a non-canonical spelling in ONE row predicate, or `null` when the + * predicate is already canonical (or is not this module's verdict to give). + * + * Deliberately conservative in both arms — every condition below can only + * REMOVE a finding, never invent one, because a false deprecation warning sends + * an author to rewrite a predicate that was correct: + * + * - **Unparseable source** → `null`. Syntax is another gate's verdict. + * - **Bare shorthand** is reported only when the undeclared identifier is an own + * key of THIS row. That is what makes it unambiguously the #4075 shorthand + * rather than a host-scope global this module cannot see (a deployment key + * that is simply not in `SCOPE_ROOTS` would otherwise read as a bare field). + * - **`data.*`** is reported only when the caller confirms `data` names the row + * (`dataNamesRow`). A surface whose `data` is the host scope's own — a + * `rowless` dialog, a metadata-editing form — is a legitimate `data.*` site + * and is left alone. + * + * Only the FIRST finding is returned, and the bare-shorthand arm is checked + * first: it is the arm the server refuses outright (`visibility-bare-identifier` + * is an `error` there, while a mis-layered root is a `warning`), so when a + * predicate manages both it is the one the author must fix to have anything + * evaluate at all. + * + * @param source The predicate's CEL text. Callers must have already + * routed legacy `${…}`-dialect strings elsewhere — in that + * dialect `data.*` is the NORMAL spelling, and reporting it + * here would be a false positive on every legacy predicate. + * @param row The row the predicate is bound against. + * @param dataNamesRow Whether `data` is bound to that same row on this surface. + */ +export function detectNonCanonicalRowSpelling( + source: string, + row: Record | null | undefined, + dataNamesRow: boolean, +): NonCanonicalRowSpelling | null { + if (typeof source !== 'string' || !source.trim()) return null; + + const roots = collectCelRootIdentifiers(source); + // Not parseable through the canonical front end — not our verdict. + if (!roots || roots.ok !== true) return null; + + // (1) Bare shorthand — the arm the server refuses outright. + const bare = firstUndeclaredReference(source); + if ( + typeof bare === 'string' && + row != null && + typeof row === 'object' && + Object.prototype.hasOwnProperty.call(row, bare) + ) { + return { + kind: 'bare-shorthand', + identifier: bare, + canonical: `${ROW_PREDICATE_CANONICAL_ROOT}.${bare}`, + }; + } + + // (2) Wrong-layer `data.*` root — silent at runtime on the server. + if (dataNamesRow && Array.isArray(roots.roots) && roots.roots.includes(METADATA_LAYER_ROOT)) { + return { + kind: 'metadata-layer-root', + identifier: METADATA_LAYER_ROOT, + canonical: ROW_PREDICATE_CANONICAL_ROOT, + }; + } + + return null; +} + +/** + * Dev-mode gate, matching `utils/dashboard-filters.ts` and `actions/actionKeys.ts` + * — a deprecation warning that floods a production console is a warning that + * gets muted. + */ +const isDev = (): boolean => + (globalThis as { process?: { env?: Record } }).process?.env + ?.NODE_ENV !== 'production'; + +/** + * Warn-once memo, keyed by the `(label, predicate source)` pair — the same + * identity `warnEvalError` uses, and JSON-encoded for the same reason: the + * separator that boundary once used was a raw U+0000, which made the file + * carrying it binary to grep (objectstack#5450). Keying on the source alone + * would report the first surface carrying a shorthand `status` predicate and + * stay silent about every other one; the label is what sends the author to the + * right screen. + * + * Module scope, not per-call: these predicates are re-evaluated on every row of + * every render, so per-call state would warn once per frame — the flood the + * dedupe exists to prevent. + */ +const warnedSpellings = new Set(); + +/** Reset the row-predicate spelling warn-once memo. Exported for tests. */ +export function resetRowPredicateCanonWarnings(): void { + warnedSpellings.clear(); +} + +/** + * Report a non-canonical row-predicate spelling once, in dev. + * + * Phase 1 of the objectui#5330 window: the binding is UNCHANGED and every + * spelling still resolves — this only says so out loud, so the stored + * population stops growing and a later survey has something to count. It is a + * warning and deliberately not a refusal: turning it into one would move the + * accept/reject set, which this card is explicitly not entitled to do. + */ +export function warnNonCanonicalRowSpelling( + source: string, + row: Record | null | undefined, + dataNamesRow: boolean, + label?: string, +): void { + if (!isDev()) return; + const finding = detectNonCanonicalRowSpelling(source, row, dataNamesRow); + if (!finding) return; + + const key = JSON.stringify([label ?? '', source, finding.kind]); + if (warnedSpellings.has(key)) return; + warnedSpellings.add(key); + + const where = label ? ` (${label})` : ''; + const detail = + finding.kind === 'bare-shorthand' + ? `it references the bare field ${JSON.stringify(finding.identifier)}; ` + + `the server refuses that spelling outright ("Unknown variable: ${finding.identifier}")` + : 'it is rooted at `data.`, which is the metadata-editing-form root — on a ' + + 'record surface the server binds no `data` at all, so the predicate is a ' + + 'constant false there rather than an error'; + + console.warn( + `[object-ui] A row predicate${where} uses a DEPRECATED spelling: ` + + `${JSON.stringify(source)} — ${detail}. The canon is \`record.*\` ` + + `(objectui#5330, ruled 2026-08-20): write \`${finding.canonical}\`. ` + + 'This still evaluates here for now; the tolerance retires after a ' + + 'stored-metadata survey.', + ); +} diff --git a/packages/react/src/hooks/useExpression.ts b/packages/react/src/hooks/useExpression.ts index 2d7346e1f3..3f7b919862 100644 --- a/packages/react/src/hooks/useExpression.ts +++ b/packages/react/src/hooks/useExpression.ts @@ -7,7 +7,12 @@ */ import { createContext, createElement, useContext, useMemo, type ReactNode } from 'react'; -import { ExpressionEvaluator, evalRowPredicate } from '@object-ui/core'; +import { + ExpressionEvaluator, + evalRowPredicate, + isLegacyDialectSource, + warnNonCanonicalRowSpelling, +} from '@object-ui/core'; /** * Global predicate scope — populated by host shells (e.g. app-shell's @@ -113,6 +118,25 @@ export { toPredicateInput } from '@object-ui/core'; * and the four generic action renderers carried the same root-only binding * until objectui#4075. * + * ## The canon is `record.*` (objectui#5330, ruled 2026-08-20) + * + * All three spellings are bound here, and that is unchanged — but they are NOT + * peers. `record.*` is the CONTRACT; the bare shorthand and `data.*` are + * tolerances in a deprecation window, kept because stored metadata carries + * them, reported once by `warnNonCanonicalRowSpelling` from `useCondition` + * below, and removable only after a stored-metadata survey sizes the window + * (⛔ no removal before the survey — part of the same ruling). + * + * The canon states the SERVER's accept set, which is narrower than this bag: + * measured on `@objectstack/formula@17.1.0`, `buildScope({ record })` mounts + * exactly `['record']`, so of the three spellings this helper binds, only + * `record.*` evaluates server-side — a bare field faults `Unknown variable: + * status` and `data.*` faults `Unknown variable: data`. This three-way bag has + * no server counterpart at all, which is exactly why the warning belongs on + * this side. `@object-ui/core`'s `evaluator/rowPredicateCanon.ts` carries the + * full measurement and the layer scoping (`data` stays canonical one layer + * over, in a metadata-editing form — ADR-0089 D3). + * * `record` and `data` are written AFTER the spread deliberately: a row that * happens to carry a field literally named `record` or `data` must not shadow * the root every predicate is written against. Same precedence as @@ -194,6 +218,40 @@ export function useCondition( // We evaluate directly without caching the evaluator to avoid issues with context changes return useMemo( () => { + // objectui#5330 Phase 1 — report a deprecated row-predicate spelling on + // the `useCondition` tier. This is the BINDING half's evaluation entry: + // `usePredicateRecordContext` sees the row but never the predicate text, + // so it structurally cannot detect a spelling, and this is the first + // point where the two meet. + // + // Two guards, both of which can only remove a report: + // - the bag must carry the `usePredicateRecordContext` signature, `data` + // and `record` being the SAME object by identity. A host scope that + // merely happens to carry a `data` key cannot satisfy that, so a + // non-row `useCondition` call is never judged as a row predicate. + // - the source must be CEL. In this tier's legacy `${…}` dialect + // (`'${data.status === "active"}'` — this hook's own doc example) + // `data.*` is the NORMAL spelling, and reporting it would be a false + // positive on every legacy predicate. + const bag = context as { record?: unknown; data?: unknown }; + const boundRow = + bag.record != null && typeof bag.record === 'object' && bag.record === bag.data + ? (bag.record as Record) + : undefined; + if (boundRow !== undefined) { + const celSource = + typeof condition === 'string' + ? isLegacyDialectSource(condition) + ? undefined + : condition + : condition && typeof condition === 'object' && condition.dialect === 'cel' + ? condition.source + : undefined; + if (typeof celSource === 'string') { + warnNonCanonicalRowSpelling(celSource, boundRow, true, options?.label); + } + } + const evaluator = new ExpressionEvaluator({ ...scope, ...context }); if (options?.throwOnError) { // Fail-closed: a predicate that can't be evaluated hides/disables @@ -235,9 +293,11 @@ export function useCondition( * routes to `@object-ui/core`'s `evalRowPredicate`: a bare string is CEL (the * spec contract for `ActionSchema.visible`), a `{ dialect: 'cel', source }` * envelope is always CEL, and only a legacy-dialect string falls back to the - * old engine (with a deprecation warning). The row is bound as `record.*` and - * bare fields; the ambient predicate scope (`features` / `user` / …) is merged - * alongside so deployment-level gates keep resolving. + * old engine (with a deprecation warning). The row is bound as `record.*` — the + * CANON (objectui#5330) — and, for stored metadata only, as bare fields and + * `data.*`, both of which now warn once; the ambient predicate scope + * (`features` / `user` / …) is merged alongside so deployment-level gates keep + * resolving. * * @param pred The raw predicate: `boolean` (returned as-is), a CEL string, * an `{ dialect, source }` envelope, or `null`/`undefined`/`''`. From a0ba08e7c31dab95843b38d9d802e91977727cd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 03:09:02 +0000 Subject: [PATCH 2/2] test(core,react): pin the row-predicate canon on BOTH evaluation tiers (#5330) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two tiers are separate evaluation entries and a warning wired into only one misses the surfaces this card is about: the generic action renderers and `record:alert` go through `usePredicateRecordContext` + `useCondition`, not through `evalRowPredicate`. Each tier's pins assert both directions — the binding still resolves all three spellings (no removal before the survey) AND the two deprecated ones now report. Ablation-checked on both tiers: removing the core call turns 3 of 15 red, removing the react call turns 2 of 7 red, and in both cases the "stays silent" pins correctly stay green. `record:alert`'s own three-spelling pins landed with PR #5688 and are not duplicated here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01E7snar5mwF7qoXJazqKhys --- .../__tests__/rowPredicateCanon.test.ts | 2 +- .../useCondition.canonSpelling.test.tsx | 94 +++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx diff --git a/packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts b/packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts index 3ff42bb991..07947ee079 100644 --- a/packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts +++ b/packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts @@ -115,7 +115,7 @@ describe('[#5330] `evalRowPredicate` — the tolerance is no longer silent', () afterEach(() => warn.mockRestore()); const deprecationWarnings = (): string[] => - warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('DEPRECATED spelling')); + warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('DEPRECATED spelling')); it('warns on the bare shorthand and prescribes `record.status`', () => { evalRowPredicate("status == 'in_review'", row, { label: 'row action "approve"' }); diff --git a/packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx b/packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx new file mode 100644 index 0000000000..27e4a6d3d9 --- /dev/null +++ b/packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx @@ -0,0 +1,94 @@ +/** + * 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#5330 — the row-predicate canon on the `useCondition` TIER. + * + * `packages/core`'s `rowPredicateCanon.test.ts` pins the detector and the + * `evalRowPredicate` tier. This file exists because the two tiers are genuinely + * separate evaluation entries and a warning wired into only one of them misses + * the surfaces this card is actually about: the four generic action renderers + * and `record:alert` go through `usePredicateRecordContext` + `useCondition`, + * NOT through `evalRowPredicate`. Pinning only the core tier would have left + * them silent while reading as covered. + * + * The legacy-dialect case is the one that would make this warning unshippable + * if it were wrong: `useCondition`'s own documented example is + * `'${data.status === "active"}'`, where `data.*` is the CORRECT spelling. A + * detector that reported it would fire on essentially every legacy predicate in + * the wild. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { resetRowPredicateCanonWarnings } from '@object-ui/core'; +import { useCondition, usePredicateRecordContext } from '../useExpression'; + +const row = { status: 'in_review', amount: 10 }; + +/** Drive the real pairing the action renderers use: bind the row, then evaluate. */ +const evaluate = (pred: unknown, record: unknown = row, label?: string) => + renderHook(() => { + const ctx = usePredicateRecordContext(record); + return useCondition(pred as never, ctx, label ? { label } : undefined); + }).result.current; + +describe('[#5330] useCondition tier — the canon and its deprecation warning', () => { + let warn: ReturnType; + beforeEach(() => { + resetRowPredicateCanonWarnings(); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => warn.mockRestore()); + + const deprecationWarnings = (): string[] => + warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('DEPRECATED spelling')); + + it('still resolves ALL THREE spellings — no removal before the survey', () => { + expect(evaluate("record.status == 'in_review'")).toBe(true); + expect(evaluate("status == 'in_review'")).toBe(true); + expect(evaluate("data.status == 'in_review'")).toBe(true); + }); + + it('is not vacuous — the same three spellings are false on a non-matching row', () => { + const other = { status: 'draft', amount: 1 }; + expect(evaluate("record.status == 'in_review'", other)).toBe(false); + expect(evaluate("status == 'in_review'", other)).toBe(false); + expect(evaluate("data.status == 'in_review'", other)).toBe(false); + }); + + it('warns once on the bare shorthand, naming the canonical rewrite', () => { + evaluate("status == 'in_review'", row, 'action:button "approve"'); + const msgs = deprecationWarnings(); + expect(msgs).toHaveLength(1); + expect(msgs[0]).toContain('record.status'); + expect(msgs[0]).toContain('action:button "approve"'); + }); + + it('warns on a `data.`-rooted CEL predicate', () => { + evaluate("data.status == 'in_review'"); + expect(deprecationWarnings()).toHaveLength(1); + }); + + it('stays silent on the canonical spelling', () => { + evaluate("record.status == 'in_review'"); + expect(deprecationWarnings()).toHaveLength(0); + }); + + it('stays silent on a legacy `${…}` predicate, where `data.*` is CORRECT', () => { + evaluate('${data.status === "in_review"}'); + expect(deprecationWarnings()).toHaveLength(0); + }); + + it('stays silent when there is no row bound (a non-row `useCondition` call)', () => { + // `usePredicateRecordContext(null)` binds NOTHING, so `record`/`data` are + // absent and this is not a row predicate at all. + renderHook(() => useCondition("status == 'in_review'" as never, usePredicateRecordContext(null))); + expect(deprecationWarnings()).toHaveLength(0); + }); +});