Skip to content

Commit 3eb1b2b

Browse files
os-zhuangclaude
andauthored
feat(lint): every field-bearing prop on a React page block resolves against the object it names (#4340) (#4353)
* feat(lint): every field-bearing prop on a React page block resolves against the object it names (#4340) #4329 closed ONE of them — <ListView searchableFields> — by running the metadata rule's core from the gate that owns React block props. That prop was an instance, not the class: every other prop a kind:'react' page binds BY FIELD NAME shipped exactly as typed, the same silent drift page-field-unknown already closes for the page-component `properties` bag one surface over. validate-react-page-props now resolves all of them: ListView fields/columns/sort/grouping/userFilters/hiddenFields/fieldOrder/filterableFields, ObjectForm fields + initialValues KEYS + sections[].fields[] + subforms (each entry against its own childObject, totalField against the form's), and the record:* blocks via the SAME COMPONENT_FIELD_SPECS table the metadata surface uses, keyed by the block's schemaType — so the two surfaces agree by construction rather than by two lists that happen to match. <Block type="…"> reaches that table by the type the author writes, so the escape hatch is checked instead of being a hole. Findings carry page-field-unknown at its advisory severity, because the consumer behaves the same way. A FILTER position gates instead. <ListView filters> / <ObjectChart filter> name fields in a QUERY: an unknown column there is not a skipped column, the predicate can never match, SqlDriver swallows "no such column" and returns [], and the surface renders an empty list indistinguishable from "there is no data" — the silent zero filter-token-unknown and validate-flow-template-paths' own filter-position call both gate on. Filter positions are also resolved INDEPENDENTLY of one another, unlike every other value this gate reads: filters={['status','=',stage]} is the shape a react page actually writes, and the all-or-nothing static reader skipped the whole array including the one position that was knowable. <RecordRelatedList objectName> is settled as the RELATED (child) object, which is what the spec schema always said, what record:related_list means on every metadata surface, and what validate-page-field-bindings already resolves its columns against. The React overlay had declared objectName a second time and glossed it "The parent object"; the generated contract publishes the overlay's description in place of the schema's, so the react surface both contradicted the spec and lost any way to name the object it renders — and the one live page authored against the gloss listed the wrong object. The class is closed too: REACT_OVERLAY_SHADOWS ledgers every overlay prop that restates a spec-schema prop, and a test asserts the ledger equals the real collision set. Also: the shared sort reader judges the LEGACY bare-string form ("amount desc") by its head instead of reporting the whole string as an unknown field — a finding no author could act on, removed from both surfaces at once. Verified against the live corpus: `os validate` is clean on app-showcase and app-crm, and re-introducing the old objectName spelling makes the new rule report both stale positions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rK2McPogrTJBKdUsThrGs * docs: validating-metadata documents the react block field-prop class; fix a related-list example that named keys nothing reads Two hand-written docs the docs-drift check flagged, both about the surface this PR changes. `deployment/validating-metadata.mdx` enumerates what `os validate` catches, and after this PR it missed the largest new class. §10 covers it: which block props resolve, that the record:* blocks ride the SAME descriptor table as §5 (so a component is described once and checked wherever it is authored), that <RecordRelatedList objectName> is the related child object, and why a filter position gates when everything else advises. §5 gains one line pointing at it. `ui/pages.mdx` authored its record:related_list example as `properties: { object, relationship }`. Neither key exists on RecordRelatedListProps and no conversion-layer alias maps them, so `properties` being unvalidated is the only reason the example parses — an author copying it gets a related list bound to nothing. Corrected to `objectName` / `relationshipField`, with a comment naming which object each one is: the same confusion this PR settles in the contract, one doc over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rK2McPogrTJBKdUsThrGs --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7fec5d6 commit 3eb1b2b

13 files changed

Lines changed: 1298 additions & 137 deletions

File tree

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
---
2+
"@objectstack/lint": minor
3+
"@objectstack/spec": minor
4+
---
5+
6+
feat(lint): every field-bearing prop on a React page block resolves against the
7+
object it names
8+
9+
#4329 closed ONE of them — `<ListView searchableFields>` — by running the
10+
metadata rule's core from the gate that owns React block props. That prop was an
11+
instance, not the class: every other prop a `kind:'react'` page binds BY FIELD
12+
NAME shipped exactly as typed, the same silent drift `page-field-unknown`
13+
already closes for the page-component `properties` bag one surface over.
14+
15+
`validate-react-page-props` now resolves all of them:
16+
17+
- `<ListView>` `fields` / `columns` / `sort` / `grouping` / `userFilters` /
18+
`hiddenFields` / `fieldOrder` / `filterableFields`
19+
- `<ObjectForm>` `fields`, `initialValues` KEYS, `sections[].fields[]`
20+
- `<RecordHighlights>` / `<RecordDetails>` / `<RecordPath>` /
21+
`<RecordRelatedList>` — via the SAME `COMPONENT_FIELD_SPECS` table the
22+
metadata surface uses, keyed by the block's `schemaType`, so the two surfaces
23+
agree by construction rather than by two lists that happen to match
24+
- `<Block type="…">` — the escape hatch reaches the same table by the type the
25+
author writes, so it is checked instead of being a hole
26+
27+
Findings carry the metadata rule's id (`page-field-unknown`) at its advisory
28+
severity, because the consumer behaves the same way: an unknown name is skipped
29+
and the rest renders.
30+
31+
**A FILTER position gates instead.** `<ListView filters>` / `<ObjectChart
32+
filter>` name fields in a QUERY, and an unknown column there is not a skipped
33+
column: the predicate can never match, `SqlDriver` swallows the driver's
34+
"no such column" and returns `[]`, and the surface renders an empty list that
35+
looks exactly like "there is no data" — the silent zero `filter-token-unknown`
36+
and `validate-flow-template-paths`' filter-position call both gate on. Those
37+
are reported as `error`.
38+
39+
Filter positions are also resolved INDEPENDENTLY of each other, unlike every
40+
other value this gate reads. `filters={['status', '=', stage]}` — a static field
41+
beside a React-state value — is the shape a react page actually writes, and the
42+
all-or-nothing static reader skipped the whole array, including the one position
43+
that was knowable.
44+
45+
Everything else is unchanged: a value from a variable, a call, or behind a
46+
spread is unresolvable rather than wrong and is skipped silently (ADR-0072 D1),
47+
as are cross-package objects, objects with no authored field map, dotted
48+
relationship paths, and registry-injected system columns.
49+
50+
### Breaking: `<RecordRelatedList objectName>` is the RELATED object, as the spec always said
51+
52+
`RecordRelatedListProps.objectName` is the related (child) object — that is what
53+
`record:related_list` means on every metadata surface, what
54+
`validate-page-field-bindings` resolves its `columns` against, and what the one
55+
registry component behind both surfaces consumes. The React overlay declared
56+
`objectName` a SECOND time and glossed it "The parent object", and the generated
57+
contract publishes the overlay's description in place of the schema's — so the
58+
react surface both contradicted the spec and lost any way to name the object it
59+
renders.
60+
61+
FROM → TO for a page authored against the old gloss:
62+
63+
```diff
64+
- <RecordRelatedList objectName="account" recordId={id} relationshipField="account_id" columns={['name','total']} />
65+
+ <RecordRelatedList objectName="invoice" recordId={id} relationshipField="account_id" columns={['name','total']} />
66+
```
67+
68+
`objectName` names the CHILD object being listed; the parent record stays bound
69+
by `recordId`, and `relationshipField` is the child's field pointing back at it.
70+
The lint above reports the old spelling (the child's columns and its FK do not
71+
resolve against the parent). `objectName` is now also published as required, as
72+
the schema declares it.
73+
74+
The class is closed as well as the instance: `REACT_OVERLAY_SHADOWS` in
75+
`@objectstack/spec/ui` ledgers every overlay prop that restates a spec-schema
76+
prop, and a test asserts the ledger equals the real collision set — so the next
77+
overlay entry that silently redefines a schema prop fails a test instead of
78+
shipping a second dialect.

content/docs/deployment/validating-metadata.mdx

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ Which object a component binds follows `dataSource.object` → `properties.objec
133133
against one page-wide guess. A `record:related_list`'s `columns`/`sort`/`filter`
134134
resolve against its **related** object (`objectName`), and its add-picker against
135135
its own. Advisory, like form-layout field references — every consumer degrades
136-
rather than failing.
136+
rather than failing. The same descriptor table drives the react page surface
137+
(§10), so a component is described once and checked wherever it is authored.
137138

138139
Skipped, to keep false positives at zero: relationship paths (`account.name`,
139140
resolved by the query engine), registry-injected system fields (`created_at`,
@@ -274,6 +275,49 @@ most-authored surfaces, so the tightening is scheduled on what this check finds
274275
rather than assumed. `defineStack` reports the same findings at config-load
275276
time, so an author sees them without running the CLI at all.
276277

278+
### 10. React block props naming fields the object doesn't have
279+
280+
§5 is about a metadata page's untyped `properties` bag. A `kind:'react'` page
281+
authors the **same** components as JSX props, and every prop that binds by field
282+
name has the same failure: the block skips the name and renders one column, one
283+
filter chip, or one form field short.
284+
285+
```jsx
286+
<ListView objectName="crm_account" columns={['name', 'revenue']} />
287+
// ↑ not a crm_account field → warning
288+
```
289+
290+
Checked on every injected block: `<ListView>`'s
291+
`fields`/`columns`/`sort`/`grouping`/`userFilters`, `<ObjectForm>`'s `fields`,
292+
`initialValues` keys, `sections[].fields[]` and `subforms` (each against its own
293+
`childObject`), and `<RecordHighlights>` / `<RecordDetails>` / `<RecordPath>` /
294+
`<RecordRelatedList>` — those last four through the **same** descriptor table
295+
§5 uses, so a component's field-bearing props are described once and checked on
296+
both surfaces. `<Block type="…">` reaches that table by the type the author
297+
writes, so the escape hatch is covered rather than left as a hole.
298+
299+
`<RecordRelatedList objectName>` is the **related (child)** object whose records
300+
are listed — the parent record is bound by `recordId`, and `relationshipField`
301+
is the child's field pointing back at it. Passing the parent there is the
302+
mistake this check was extended to catch.
303+
304+
A **filter position** is the exception that gates:
305+
306+
```jsx
307+
<ListView objectName="crm_account" filters={['revenu', '=', stage]} />
308+
// ↑ error, not a warning
309+
```
310+
311+
An unknown column in a predicate is not a skipped column — the predicate can
312+
never match, the driver's "no such column" is swallowed, and the list comes back
313+
empty and indistinguishable from *"there is no data"*. Each position of a filter
314+
is judged on its own, so the field above is still checked even though `stage`
315+
comes from React state.
316+
317+
Skipped, to keep false positives at zero: the same set as §8 — non-static
318+
values, `{...spread}` usages, relationship paths, system fields, and objects
319+
another package defines.
320+
277321
## The one gate, two entry points
278322

279323
`os validate` and `os build` (alias of `os compile`) run the **same** validator:

content/docs/ui/pages.mdx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,15 @@ const accountRecordPage = {
228228
type: 'record:related_list',
229229
id: 'contacts',
230230
label: 'Contacts',
231-
properties: { object: 'contact', relationship: 'account' },
231+
// `objectName` is the RELATED (child) object being listed;
232+
// `relationshipField` is the child's field pointing back at this record.
233+
properties: { objectName: 'contact', relationshipField: 'account' },
232234
},
233235
{
234236
type: 'record:related_list',
235237
id: 'opportunities',
236238
label: 'Opportunities',
237-
properties: { object: 'opportunity', relationship: 'account' },
239+
properties: { objectName: 'opportunity', relationshipField: 'account' },
238240
},
239241
],
240242
},

examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ import { definePage } from '@objectstack/spec/ui';
2424
* `groupBy`) and `total` (its `field`) — not by a dataset-style measure name.
2525
* `os validate` checks both halves.
2626
*
27+
* `<RecordRelatedList>` binds the CHILD object it lists (`showcase_invoice`),
28+
* not the parent — the parent is `recordId`, and `relationshipField="account"`
29+
* is the invoice's lookup back to it. This page used to pass the parent, which
30+
* is what #4340 found: the react contract had glossed `objectName` as "the
31+
* parent object" while the schema (and the renderer behind both surfaces) read
32+
* it as the related one, so the list resolved `total` against an account and
33+
* came back empty. Every field-bearing prop on the page is now checked against
34+
* the object it actually names.
35+
*
2736
* Styling (ADR-0065): no Tailwind — inline `style={{}}` with `hsl(var(--token))`;
2837
* data blocks and the drawer bring their own compiled styling. The drawer sets
2938
* NO pixel width: per #2578 pixel widths are deprecated (the author can't know
@@ -122,7 +131,7 @@ function Page() {
122131
123132
<ObjectChart objectName="showcase_invoice" type="bar" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxis={{ field: 'status' }} yAxis={[{ field: 'total', format: '$0,0' }]} series={[{ name: 'total', label: 'Invoice value' }]} title="Invoice value by status" showLegend={true} />
124133
125-
<RecordRelatedList objectName="showcase_account" recordId={sel} relationshipField="account" columns={['name', 'status', 'total']} limit={5} showViewAll={true} title="Invoices" />
134+
<RecordRelatedList objectName="showcase_invoice" recordId={sel} relationshipField="account" columns={['name', 'status', 'total']} limit={5} showViewAll={true} title="Invoices" />
126135
127136
{editing ? (
128137
<ObjectForm objectName="showcase_account" mode="edit" recordId={sel}

packages/lint/src/validate-page-field-bindings.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,30 @@ describe('validatePageFieldBindings — false-positive floor', () => {
317317
expect(findings).toEqual([]);
318318
});
319319
});
320+
321+
describe('validatePageFieldBindings — legacy bare-string sort (#4340)', () => {
322+
/**
323+
* `ListViewSchema.sort` still accepts `"created_at desc"`. Reading the whole
324+
* string as a field name reported `"amount desc"` as unknown — a finding
325+
* whose "field" the author never wrote, and one no fix could satisfy. The
326+
* shared `sortFieldRefs` reads its head instead, the way the renderer does.
327+
*/
328+
it('judges the head segment, not the whole "field desc" string', () => {
329+
const listPage = (sort: unknown) => ({
330+
...baseStack(),
331+
pages: [
332+
{
333+
name: 'list_page',
334+
type: 'list',
335+
object: 'crm_lead',
336+
interfaceConfig: { source: 'crm_lead', columns: ['name'], sort },
337+
},
338+
],
339+
});
340+
expect(validatePageFieldBindings(listPage('amount desc'))).toEqual([]);
341+
const bad = validatePageFieldBindings(listPage('ghost_col desc'));
342+
expect(bad).toHaveLength(1);
343+
expect(bad[0].message).toContain('"ghost_col"');
344+
expect(bad[0].message).not.toContain('ghost_col desc');
345+
});
346+
});

0 commit comments

Comments
 (0)