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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .changeset/row-predicate-phase2-record-only-5741.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
---
'@object-ui/core': minor
'@object-ui/react': minor
---

row predicates on runtime record surfaces resolve `record.*` only; the bare-field and `data.*` spellings are no longer bound

Phase 2 of the row-predicate canon (objectui#5330, ruled 2026-08-20, option B;
Phase 2 ruled 2026-09-02 and amended 2026-09-05 on objectui#5741). Until now a
row predicate — `visible` / `disabled` / `enabled` on an action renderer, a row
action, a `record:alert`, a `page:header` action, a conditional-formatting
`condition` — bound the row three ways: canonical `record.status`, bare
`status`, and `data.status`. The two non-canonical spellings are retired on
every runtime record surface, in both evaluation tiers (`evalRowPredicate` /
`partitionRowsByPredicate` in `@object-ui/core`; `usePredicateRecordContext` +
`useCondition` in `@object-ui/react`) and for both dialects: a legacy
`${data.x}` / `${x}` string on a row surface retires with the CEL spellings.

**What a retired spelling does now: it faults, exactly as it already did on the
server** (`buildScope({ record })` mounts exactly `['record']`, so `status` and
`data` are unknown variables there), and each surface applies its EXISTING
fault policy — no runtime detector, no "treat as absent" special case, no
uniform override:

- `evalRowPredicate` / `partitionRowsByPredicate` (row kebab, selection bar,
`page:header` actions, conditional formatting): the caller's `fallback` —
hidden / every row excluded / no style — reported once by the existing fault
warning, which names the unknown variable (`Unknown variable: status`) and,
on the fast route, carries the `record.` hint.
- `useCondition` legs that opt into `throwOnError` (`action:button` and
`action:menu` `visible`, `DeclaredActionsBar` `visible`): fail-closed —
hidden on every row, reported once as `was hidden/disabled: its predicate
threw — status is not defined`.
- the non-throwing `useCondition` legs (`action:icon` / `action:group`
`visible`, every `disabled` / `enabled`, `record:alert`): fail-soft — shown /
greyed / enabled on every row, with the evaluator's own console line.
- a host scope that carries its OWN `data` (app-shell's ambient `data: {}`) is
left standing: `data.*` on a record surface then reads the host's object — a
constant, silent `false` — which is what "no longer bound to the row" means.

The Phase-1 deprecation warning is removed with the bindings:
`warnNonCanonicalRowSpelling` and `resetRowPredicateCanonWarnings` are no
longer exported from `@object-ui/core`. `detectNonCanonicalRowSpelling`,
`ROW_PREDICATE_CANONICAL_ROOT` and the `NonCanonicalRowSpelling` type stay
exported — the offline instrument for sweeping authored metadata.

The layer rule is unchanged: `data` remains the canonical root on
metadata-editing surfaces (ADR-0089 D3, `CANONICAL_ROOT_BY_LAYER`), and
app-shell's metadata-admin `SchemaForm` / `predicate.ts` keep binding
`{ data: row }` through their own evaluator.

No stored-metadata survey, export or migration rewrite was run (the maintainer
ruled the stored population out of scope, 「不考虑存量」); the Phase-1 warning
period was the notice.

Release note: Phase 1 (PR #5737 — the canon statement plus the warning) shipped
in `@object-ui/core@17.6.0` (npm, 2026-08-24) although its changeset
`.changeset/row-predicate-record-canon-5330.md` is still pending on `main`, so
the next CHANGELOG section lists Phase 1 and this Phase 2 together: the warning
it describes was live from 17.6.0 and is gone from this release on.
121 changes: 90 additions & 31 deletions packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ describe('DeclaredActionsBar — declared `visible` on a server-declared action
});

it('an expression-valued `visible` keeps its verdict — false hides, true shows', () => {
const gated = { ...APPROVE, visible: 'status == "pending"' };
const gated = { ...APPROVE, visible: 'record.status == "pending"' };
const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'approved' });
expect(screen.queryByTestId('declared-action-approval_approve')).toBeNull();
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
Expand All @@ -520,6 +520,33 @@ describe('DeclaredActionsBar — declared `visible` on a server-declared action
expect(screen.getByTestId('declared-action-approval_approve')).toBeInTheDocument();
});

it('a bare-field `visible` no longer discriminates (objectui#5741) — hidden on BOTH rows, reported once', () => {
// Phase 2 of the objectui#5330 canon: the row is bound as `record.*` only,
// so `status` is an unknown variable. This leg opts into `throwOnError`, so
// its EXISTING policy is fail-closed: the same verdict on the holding row
// and the failing one, and one console line naming the variable. The
// ungated companion proves the bar rendered.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const gated = { ...APPROVE, visible: 'status == "pending"' };
const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'pending' });
expect(screen.queryByTestId('declared-action-approval_approve')).toBeNull();
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
unmount();
renderWithGate(gated, { ...REQUEST, status: 'approved' });
expect(screen.queryByTestId('declared-action-approval_approve')).toBeNull();
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
const reports = warn.mock.calls
.map((c) => String(c[0]))
.filter((m) => m.includes('was hidden/disabled: its predicate threw'));
expect(reports).toHaveLength(1);
expect(reports[0]).toContain('status is not defined');
expect(reports[0]).toContain('declared action "approval_approve" (visible)');
} finally {
warn.mockRestore();
}
});

it('a `${…}`-spelled `visible` keeps its verdict — true shows, false hides (objectui#3871)', () => {
// This leg opts into `throwOnError`, so the double wrap did not read as
// truthy here: `'${${…}}'` THREW inside the evaluator and `useCondition`'s
Expand All @@ -533,7 +560,7 @@ describe('DeclaredActionsBar — declared `visible` on a server-declared action
// Reverse verification: the `pending` half is the detector; the `approved`
// half stays green either way (hidden is hidden), and the companion
// assertion is what keeps that half from meaning "the bar vanished".
const gated = { ...APPROVE, visible: '${status === "pending"}' };
const gated = { ...APPROVE, visible: '${record.status === "pending"}' };
const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'approved' });
expect(screen.queryByTestId('declared-action-approval_approve')).toBeNull();
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
Expand Down Expand Up @@ -623,14 +650,27 @@ describe('DeclaredActionsBar — declared `disabled` on a server-declared action
});

it('an expression-valued `disabled` keeps its verdict — true disables, false does not', () => {
const gated = { ...APPROVE, disabled: 'status == "approved"' };
const gated = { ...APPROVE, disabled: 'record.status == "approved"' };
const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'approved' });
expect(approve()).toBeDisabled();
unmount();
renderWithGate(gated, { ...REQUEST, status: 'pending' });
expect(approve()).not.toBeDisabled();
});

it('a bare-field `disabled` no longer discriminates (objectui#5741) — disabled on BOTH rows (fail-soft leg)', () => {
// The `disabled` leg does not opt into `throwOnError`, so its EXISTING
// policy is fail-soft `true`: an unbound `status` faults and greys Approve
// on the holding row and the failing one alike. Pinned as the fail-soft
// half of the ruling, next to the fail-closed `visible` half above.
const gated = { ...APPROVE, disabled: 'status == "approved"' };
const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'approved' });
expect(approve()).toBeDisabled();
unmount();
renderWithGate(gated, { ...REQUEST, status: 'pending' });
expect(approve()).toBeDisabled();
});

it('a `${…}`-spelled `disabled` keeps its verdict — false leaves Approve clickable (objectui#3871)', () => {
// `toPredicateInput` used to wrap EVERY string, so `'${…}'` — a spelling
// AGENTS.md §4 documents and this bar's own `visible` sibling accepts —
Expand All @@ -643,7 +683,7 @@ describe('DeclaredActionsBar — declared `disabled` on a server-declared action
// disabled before the fix); the `approved` half was green already, since
// "disabled because the predicate holds" and "disabled because it could not
// be parsed" look identical from here.
const gated = { ...APPROVE, disabled: '${status === "approved"}' };
const gated = { ...APPROVE, disabled: '${record.status === "approved"}' };
const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'approved' });
expect(approve()).toBeDisabled();
unmount();
Expand All @@ -670,16 +710,17 @@ describe('DeclaredActionsBar — declared `disabled` on a server-declared action
* objectui#4077 fixed the root-only binding here with an inline
* `{ ...row, record: row, data: row }`; objectui#4079 fixed the same fault on
* the four generic action renderers and gave the rule one name instead of a
* fifth copy. The two copies agreed on every row the bar has ever been mounted
* over — which is why this is a convergence card and not a defect report, and
* why the two cases below are deliberately different in kind:
* fifth copy. objectui#5741 (Phase 2 of the objectui#5330 canon) then narrowed
* that shared rule to `{ record: row }`: the bare-field and `data.*` spellings
* are no longer bound anywhere, and this bar follows the helper. The two cases
* below are deliberately different in kind:
*
* • the ROW-PRESENT case is an EQUIVALENCE pin. It was green before the
* migration and is green after it, because the copies agree wherever a row
* exists. It is not a mutation detector for this change and must not be
* read as one; it is here so the reachable path — the only path any host
* drives today — is pinned against a future edit to the helper, which now
* owns the verdict for this bar too.
* • the ROW-PRESENT case pins the helper's CURRENT rule as this bar sees it:
* `record.*` discriminates, the two retired spellings reach the SAME
* verdict on the holding row and the failing one (they fault, and this
* leg's existing policy is fail-closed). It is here so the reachable path
* — the only path any host drives today — is pinned against a future edit
* to the helper, which owns the verdict for this bar too.
* • the NO-ROW case is the CONVERGENCE detector, and the one difference the
* two copies ever had. `usePredicateRecordContext` binds NOTHING when there
* is no row; the inline copy bound `{ record: {}, data: {} }`. Since
Expand All @@ -700,24 +741,42 @@ describe('DeclaredActionsBar — the row binds through the shared helper (object
locations: ['record_section'],
};

it('with a row present, all three spellings reach the same verdict', () => {
// Both halves per spelling: "renders" alone is satisfied by a bar that
// ignores `visible` entirely, which is the mutation objectui#3835 was.
for (const visible of [
'record.status == "pending"',
'status == "pending"',
'data.status == "pending"',
]) {
const shown = renderWithGate({ ...APPROVE, visible }, { ...REQUEST, status: 'pending' });
expect(screen.getByTestId('declared-action-approval_approve'), visible).toBeInTheDocument();
shown.unmount();

const hidden = renderWithGate({ ...APPROVE, visible }, { ...REQUEST, status: 'approved' });
expect(screen.queryByTestId('declared-action-approval_approve'), visible).toBeNull();
// The ungated companion proves the bar itself rendered — "not found" here
// must mean the gate said no, not that the located set was empty.
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
hidden.unmount();
it('with a row present, `record.*` reaches both verdicts through the shared helper', () => {
// Both halves: "renders" alone is satisfied by a bar that ignores `visible`
// entirely, which is the mutation objectui#3835 was.
const visible = 'record.status == "pending"';
const shown = renderWithGate({ ...APPROVE, visible }, { ...REQUEST, status: 'pending' });
expect(screen.getByTestId('declared-action-approval_approve'), visible).toBeInTheDocument();
shown.unmount();

const hidden = renderWithGate({ ...APPROVE, visible }, { ...REQUEST, status: 'approved' });
expect(screen.queryByTestId('declared-action-approval_approve'), visible).toBeNull();
// The ungated companion proves the bar itself rendered — "not found" here
// must mean the gate said no, not that the located set was empty.
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
hidden.unmount();
});

it('with a row present, the two retired spellings reach the SAME verdict on both rows (objectui#5741)', () => {
// The helper binds `{ record: row }` only, so these fault; the bar's
// `visible` leg is fail-closed, so "the same verdict" is hidden twice —
// with the companion present both times, so hidden means the gate faulted,
// not that the bar vanished.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
for (const visible of ['status == "pending"', 'data.status == "pending"']) {
const holding = renderWithGate({ ...APPROVE, visible }, { ...REQUEST, status: 'pending' });
expect(screen.queryByTestId('declared-action-approval_approve'), visible).toBeNull();
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
holding.unmount();

const failing = renderWithGate({ ...APPROVE, visible }, { ...REQUEST, status: 'approved' });
expect(screen.queryByTestId('declared-action-approval_approve'), visible).toBeNull();
expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
failing.unmount();
}
} finally {
warn.mockRestore();
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { render, screen } from '@testing-library/react';
import { render, screen, cleanup } from '@testing-library/react';
import { ComponentRegistry } from '@object-ui/core';
import { ActionProvider, RecordContextProvider, PredicateScopeProvider } from '@object-ui/react';

Expand Down Expand Up @@ -204,14 +204,24 @@ describe('page:header — CEL-only constructs in action predicates (#3521)', ()
});

describe('bindings the row surfaces already offered', () => {
it('binds bare field names', () => {
// objectui#5741 (Phase 2 of the objectui#5330 canon): the bare-field and
// `data.*` spellings are no longer bound on a record surface. They fault,
// and this surface's existing policy is fail-closed — the SAME verdict on a
// matching and a non-matching row is what "unbound" looks like from here.
it('no longer binds bare field names (objectui#5741) — hidden on both rows', () => {
renderHeader({ name: 'zoo_bare', visible: 'f_status == "open"' });
expect(shown()).toBe(true);
expect(shown()).toBe(false);
cleanup();
renderHeader({ name: 'zoo_bare_other', visible: 'f_status == "open"' }, { ...RECORD, f_status: 'closed' });
expect(shown()).toBe(false);
});

it('binds `data.*`', () => {
it('no longer binds `data.*` (objectui#5741) — hidden on both rows', () => {
renderHeader({ name: 'zoo_data', visible: 'data.f_status == "open"' });
expect(shown()).toBe(true);
expect(shown()).toBe(false);
cleanup();
renderHeader({ name: 'zoo_data_other', visible: 'data.f_status == "open"' }, { ...RECORD, f_status: 'closed' });
expect(shown()).toBe(false);
});

it('binds the host scope (`os.user.*`) alongside the record', () => {
Expand Down
Loading
Loading