Skip to content

Commit c0ae356

Browse files
fix(lint)!: refuse a page component whose properties is present but not an object, nested ones included (#25067)
Closes #24991 Clause-②: no (narrowing: `os lint --strict` refuses a nested page component whose `properties` is `null` or an array) ## What `validateComponentProps` (`packages/lint/src/validate-component-props.ts`) now refuses a walked page component, top-level or nested, whose `properties` is present but not a record (`null`, an array, a scalar). The refusal is one `component-props-invalid` error at the node's own `properties` path, for example `pages[0].regions[0].components[0].properties.children[0].properties`. The message names what was received. - It applies whatever the node's `type`, registered or not: the bag's shape is the node's contract, not a props schema's. An unregistered type with an object bag is still skipped, as before. - No per-prop "missing required" verdicts are added beside it. A malformed bag yields one finding, not one per required key. - An absent bag keeps its earlier verdict (judged as `{}`), unchanged from the absent-bag fix in #22212. - The binding verdict for data-source-bound elements is independent of the bag and still fires alongside. ## Why `PageComponentSchema` declares `properties` a record, but that parse only reaches top-level components (`regions[].components[]`, `slots.*`). A nested component (in `children`, `items[].children` or `footer`) sits in a `z.array(z.unknown())` slot that no parse judges. The rule set `props` to `undefined` for a non-record bag and skipped the node, so a nested `record:related_list` with `properties: null` or `[]`, or a nested `object-metric` with `properties: []`, passed `os lint --strict`, `os validate` and `tsc`. The node published with none of its props. ## No duplicate with the schema's own top-level refusal At both doors (CLI and the runtime page save door), the page parse runs before any authoring rule and stops on its own refusal. So a top-level non-record bag is reported once, by the parse ("expected record"), and this rule's finding never appears beside it. On a top-level node the new branch only answers a caller that hands the rule an unparsed stack, and it gives the same verdict. A test pins both halves: `PageSchema.safeParse` refuses the top-level bag at `regions.0.components.0.properties` and accepts the nested one, which is the gap. ## Tests On head `88d3dd0572` (the lint source is byte-identical to `c444510a55`, where the package suite ran; the later merge brought no `packages/lint` or `packages/spec` change): - `pnpm --filter @objectstack/lint test`: 153 files passed, 7080 tests passed, 5 skipped. - `pnpm --filter @objectstack/lint typecheck` (with `check:test-typecheck`): exit 0. - New cases cover five walks (container `children`, tab `items[].children`, accordion `items[].children`, card `footer`, a slot component's `children`) times three bags (`null`, `[]`, `'x'`). Each draws exactly one `component-props-invalid` at the node's `properties` path. Each walk also has two controls: a valid bag stays silent, and an absent bag keeps the earlier verdict. Further cases: the `object-metric` `[]` probe, an unregistered type (refused, plus a silent object-bag control), a data-source-bound element (bag plus binding verdict, nothing more), the top-level parse/no-duplicate pin, and pipeline gating at `error` for `validate`, `lint` and `build`. - The old pin "a present bag that is not an object is still left to the parse that owns its shape" is removed. It pinned exactly the skip this PR closes. Gates: `node scripts/pm/dispatch-gates.mjs --commands` derived 65 families on `c444510a55`. All 65 ran, and `--ran` reconciled them with exit codes recorded: 65 run, 0 NOT-MEASURED (a derived zero). The `--commands` derivation on `88d3dd0572` is byte-identical. On `88d3dd0572` I reran `check-adr-0087-registration --base origin/main`, `check-changeset-no-major --base origin/main`, `check-empty-changeset --base origin/main` and `check:nul-bytes`, all exit 0. `check:dts-closure` and `check:dual-build-cjs-loads` exited non-zero in the first sweep. That sweep ran while I was rebuilding shared dists with `OS_SKIP_DTS=1` for the producer measurement, so the dists were mid-rebuild. Both reran clean (exit 0) once that build finished, with no source change between the two runs. ## Producers measured I ran the rule over every in-repo example stack (normalized, through `validateComponentProps`) on `c444510a55`: | stack | pages | component-props findings | non-record bag findings | |---|---|---|---| | `examples/app-crm` | 1 | 0 | 0 | | `examples/app-multi-package` | 0 | 0 | 0 | | `examples/app-showcase` | 29 | 0 | 0 | | `examples/app-todo` | 0 | 0 | 0 | Positive control: I planted a nested `record:related_list` with `properties: null` in memory into the showcase's `showcase_component_gallery` page. It drew exactly 1 finding at `pages[2].regions[0].components[1].properties.children[0].properties`. A static grep of `examples/**` and the page sources for a literal `properties: null` or `properties: []` also returns zero hits. There are no in-repo producers to fix. ## Changeset `.changeset/24991-nested-props-non-record.md`: `@objectstack/lint` minor, with a BREAKING section (what moves at `os build` / `os validate` / `os lint` and the page save door, the remedy, what is unchanged). It carries the claim's `Clause-②` line. The ADR-0087 disposition is `not-required (no-migration-prescription)`: no authorable key, spelling, export or stored shape moves. Stored pages are not converted and keep loading until their next save, and the repair (restoring the lost props) is not derivable by a ledger entry. `check-adr-0087-registration` accepts it. ## Acceptance notes - No governed surface is touched: the diff is `packages/lint/src/validate-component-props.ts`, its test and one changeset. - No open PR touches `validate-component-props.ts`. I checked the files of all 11 open PRs before opening this one. - CI owns the repo-wide `pnpm lint` and the full battery. This PR did not run them locally. --- _Generated by [Claude Code](https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent baedcec commit c0ae356

3 files changed

Lines changed: 186 additions & 27 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
fix(lint)!: `os build`, `os validate`, `os lint` and the page save door refuse a page component whose `properties` is present but not an object, nested components included (#24991)
6+
7+
Clause-②: no (narrowing: `os lint --strict` refuses a nested page component whose `properties` is `null` or an array)
8+
9+
A page component's `properties` is a record on every node. `PageComponentSchema` refuses anything else, but that parse only reaches the top-level components (`regions[].components[]` and `slots.*`). A nested component, in a container's `children`, a tab or accordion panel's `items[].children` or a card's `footer`, sits in an untyped slot that no parse judges. The component-props rule (`validateComponentProps`) then skipped a nested node whose bag was `null`, an array or a scalar. So a nested `record:related_list` with `properties: null`, or a nested `object-metric` with `properties: []`, passed `os lint --strict`, `os validate` and `tsc`, and the page published a component that had lost every prop it was authored with. An absent bag was already judged as `{}`; this was the present-but-not-an-object residual of the same skip.
10+
11+
The rule now refuses such a bag on every walked node, whatever its `type`, with one `component-props-invalid` error located at the node's `properties` path (for example `pages[0].regions[0].components[0].properties.children[0].properties`). The message names what was received (`null`, an array, a string). No per-prop verdicts are added beside it.
12+
13+
**BREAKING — what moves for consumers.**
14+
15+
- `os build`, `os validate` and `os lint` fail on a page whose nested component carries `properties: null`, `properties: []` or a scalar `properties`. Before, they passed.
16+
- A page write in publish mode that carries such a component answered 200. It now answers `422 INVALID_METADATA` with a `component-props-invalid` issue at that path, because the page save door runs this same rule.
17+
18+
**Remedy.** Write the bag as an object of the props the component reads (`properties: { … }`), or omit the key when the component needs none. An omitted bag is judged as `{}`, so a component with required props still has to name them.
19+
20+
**Unchanged.**
21+
22+
- A top-level component's non-object bag is refused by the page parse, as before, and that parse runs before any authoring rule at both doors, so no second finding appears beside it.
23+
- An absent bag keeps its earlier verdict: it is judged as `{}`.
24+
- A valid bag is judged exactly as before. An unregistered component `type` with an object bag is still skipped.
25+
- The finding takes the rule's existing id and severity (`component-props-invalid`, `error`). No public export or signature moves.
26+
- Stored pages are not migrated and are not refused on read. A stored page with such a component keeps loading until it is next saved, and that save is judged.
27+
- Measured before crossing: no page this repository ships authors a non-object `properties` bag.
28+
29+
<!-- adr-0087: not-required (no-migration-prescription) a refusal at the CLI doors and the page save door of a page component whose properties bag is present but not an object: no authorable key, spelling, export or stored shape moves, and no stored row is read, rewritten or converted. A stored page with such a component keeps loading until it is next saved, and the repair is the author restoring the props the component lost, which no ledger entry can derive. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this verdict (not already-registered); and the change is a lint verdict, not a declaration (not runtime-interface-only or type-surface-only). -->

‎packages/lint/src/validate-component-props.test.ts‎

Lines changed: 99 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// something.
1313
import { describe, expect, it } from 'vitest';
1414
import { normalizeStackInput } from '@objectstack/spec';
15-
import { ComponentPropsMap } from '@objectstack/spec/ui';
15+
import { ComponentPropsMap, PageSchema } from '@objectstack/spec/ui';
1616
import {
1717
validateComponentProps as validateComponentPropsUnrecorded,
1818
COMPONENT_PROPS_UNKNOWN_KEY,
@@ -325,19 +325,111 @@ describe('validateComponentProps — value verdicts', () => {
325325
expect(required(validateComponentProps(stackWith([node])))).toContain(path);
326326
});
327327

328-
it('a present bag that is not an object is still left to the parse that owns its shape', () => {
329-
expect(validateComponentProps(
330-
stackWith([{ type: 'page:section', properties: { children: [{ type: 'record:related_list', properties: 'x' }] } }]),
331-
)).toEqual([]);
332-
});
333-
334328
it('a nested node whose required props are all optional stays silent with no bag', () => {
335329
expect(validateComponentProps(
336330
stackWith([{ type: 'page:section', properties: { children: [{ type: 'page:header' }] } }]),
337331
)).toEqual([]);
338332
});
339333
});
340334

335+
/**
336+
* [#24991] A NESTED component whose `properties` is PRESENT but not a record.
337+
* `PageComponentSchema` refuses that bag on a top-level node ("expected
338+
* record"), but a nested node sits in a `z.array(z.unknown())` slot that no
339+
* parse reaches, and this rule used to skip it — `null` and `[]` were measured
340+
* passing `lint --strict` on nested `record:related_list` and `object-metric`
341+
* in the reporting app. The bag is refused at the node's `properties` path,
342+
* once, with no per-prop verdicts beside it.
343+
*/
344+
describe('a nested component whose `properties` is present but not a record (#24991)', () => {
345+
const shape = (f: ReturnType<typeof validateComponentProps>) => f.map((x) => [x.rule, x.severity, x.path]);
346+
const RELATED = (properties: unknown) => ({ type: 'record:related_list', properties });
347+
348+
/** The walks #22212 covers, plus a `page:card` footer and a slot's sub-tree. */
349+
const WALKS: Array<[string, (child: unknown) => AnyRec, string]> = [
350+
['a container\'s `children`', (child) => stackWith([{ type: 'page:section', properties: { children: [child] } }]),
351+
'pages[0].regions[0].components[0].properties.children[0].properties'],
352+
['a tab panel\'s `items[].children`',
353+
(child) => stackWith([{ type: 'page:tabs', properties: { items: [{ label: 'T', value: 't', children: [child] }] } }]),
354+
'pages[0].regions[0].components[0].properties.items[0].children[0].properties'],
355+
['an accordion panel\'s `items[].children`',
356+
(child) => stackWith([{ type: 'page:accordion', properties: { items: [{ label: 'A', children: [child] }] } }]),
357+
'pages[0].regions[0].components[0].properties.items[0].children[0].properties'],
358+
['a card\'s `footer`', (child) => stackWith([{ type: 'page:card', properties: { footer: [child] } }]),
359+
'pages[0].regions[0].components[0].properties.footer[0].properties'],
360+
['a slot component\'s `children`',
361+
(child) => ({ pages: [{ name: 'probe_page', label: 'Probe', type: 'record', object: 'task', kind: 'slotted',
362+
slots: { details: [{ type: 'page:section', properties: { children: [child] } }] } }] }),
363+
'pages[0].slots.details[0].properties.children[0].properties'],
364+
];
365+
const BAGS: Array<[string, unknown, string]> = [
366+
['null', null, '`null`'],
367+
['an array', [], 'an array'],
368+
['a scalar', 'x', 'a string'],
369+
];
370+
371+
describe.each(WALKS)('in %s', (_walk, wrap, path) => {
372+
it.each(BAGS)('a %s bag draws one `component-props-invalid` error at the node\'s `properties`', (_l, bag, received) => {
373+
const findings = validateComponentProps(wrap(RELATED(bag)));
374+
expect(shape(findings)).toEqual([[COMPONENT_PROPS_INVALID, 'error', path]]);
375+
expect(findings[0]!.message).toContain(`received ${received}`);
376+
});
377+
378+
it('control: the same node with a valid bag stays silent', () => {
379+
expect(validateComponentProps(wrap(RELATED({ objectName: 'task', relationshipField: 'project' })))).toEqual([]);
380+
});
381+
382+
it('control: the same node with NO bag keeps its #22212 verdict (judged as `{}`)', () => {
383+
const paths = invalid(validateComponentProps(wrap({ type: 'record:related_list' }))).map((f) => f.path);
384+
expect(paths).toContain(`${path}.relationshipField`);
385+
expect(paths).not.toContain(path);
386+
});
387+
});
388+
389+
it('`object-metric` nested with `properties: []` — the third measured probe', () => {
390+
const findings = validateComponentProps(stackWith([{ type: 'page:section', properties: { children: [{ type: 'object-metric', properties: [] }] } }]));
391+
expect(shape(findings)).toEqual([[COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties.children[0].properties']]);
392+
});
393+
394+
it('an unregistered type is refused too: the bag\'s shape is the node\'s contract, not a props schema\'s', () => {
395+
const findings = validateComponentProps(stackWith([{ type: 'page:section', properties: { children: [{ type: 'flex', properties: null }] } }]));
396+
expect(shape(findings)).toEqual([[COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties.children[0].properties']]);
397+
// Control: the same unregistered node with an object bag is still skipped.
398+
expect(validateComponentProps(stackWith([{ type: 'page:section', properties: { children: [{ type: 'flex', properties: { anything: 1 } }] } }]))).toEqual([]);
399+
});
400+
401+
it('a data-source-bound element with a malformed bag and no binding draws both node verdicts, nothing more', () => {
402+
const findings = validateComponentProps(stackWith([{ type: 'page:section', properties: { children: [{ type: 'element:repeater', properties: null }] } }]));
403+
expect(shape(findings).sort()).toEqual([
404+
[COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties.children[0].dataSource.object'],
405+
[COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties.children[0].properties'],
406+
]);
407+
});
408+
409+
it('top level: the page parse refuses the bag first, and the rule given the raw node answers the same verdict', () => {
410+
// The page parse owns the top-level shape — and only that one: the
411+
// nested node's bag passes it untouched, which is the whole gap.
412+
const top = PageSchema.safeParse((stackWith([RELATED(null)]).pages as AnyRec[])[0]);
413+
expect(top.success).toBe(false);
414+
expect(top.error!.issues.map((i) => i.path.join('.'))).toContain('regions.0.components.0.properties');
415+
const nested = PageSchema.safeParse((stackWith([{ type: 'page:section', properties: { children: [RELATED(null)] } }]).pages as AnyRec[])[0]);
416+
expect(nested.success).toBe(true);
417+
// An unparsed caller gets one finding, not one per required prop.
418+
expect(shape(validateComponentProps(stackWith([RELATED(null)])))).toEqual([
419+
[COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties'],
420+
]);
421+
});
422+
423+
it('gates through the shared pipeline at `error`, the rule\'s existing class (#23147)', () => {
424+
for (const command of ['validate', 'lint', 'build'] as const) {
425+
const mine = runAuthoringRules(command, {
426+
normalized: stackWith([{ type: 'page:section', properties: { children: [RELATED(null)] } }]) as never,
427+
}).filter((f) => f.rule.startsWith('component-props'));
428+
expect(mine.map((f) => [f.rule, f.severity]), command).toEqual([[COMPONENT_PROPS_INVALID, 'error']]);
429+
}
430+
});
431+
});
432+
341433
/**
342434
* #7702 — `PageHeaderProps.title` used to be required while the platform's
343435
* own synthesizer (objectui `buildDefaultHeader`) emits every seeded

‎packages/lint/src/validate-component-props.ts‎

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@
3939
* is loud on its own, passthrough stays silent because the key survives) and
4040
* the rename suggestions are single-source, never re-derived here.
4141
* - **`component-props-invalid`** — a value the props schema rejects: a wrong
42-
* type, a missing required key, a value outside a declared enum.
42+
* type, a missing required key, a value outside a declared enum; and a
43+
* `properties` bag that is present but not an object (`null`, an array, a
44+
* scalar), on any walked node (#24991 — a nested node's bag is parsed by
45+
* nothing else).
4346
*
4447
* A schema that is STRIP today reports its unknown keys through the walker; one
4548
* that a later `strictObject` batch closes reports them through `safeParse` as
@@ -180,6 +183,13 @@ function strName(v: unknown): string | undefined {
180183
return typeof v === 'string' && v.length > 0 ? v : undefined;
181184
}
182185

186+
/** What a non-record `properties` bag is, for the refusal's message. */
187+
function describeBag(v: unknown): string {
188+
if (v === null) return '`null`';
189+
if (Array.isArray(v)) return 'an array';
190+
return `a ${typeof v}`;
191+
}
192+
183193
/** A zod schema, as much of one as this file reads. */
184194
interface PropsSchema {
185195
safeParse(value: unknown): { success: boolean; error?: { issues: ReadonlyArray<LintZodIssue> } };
@@ -312,26 +322,44 @@ export function validateComponentProps(stack: AnyRec): ComponentPropsFinding[] {
312322
for (const { component, path } of walkPageComponents(page, `pages[${pi}]`)) {
313323
const type = strName(component.type);
314324
if (!type) continue;
315-
// Unregistered type — skipped silently. See the module header: `type` is
316-
// an open union and the majority of authored nodes are SDUI blocks this
317-
// map does not carry.
325+
const where = `page "${pageName}" · ${type}`;
326+
327+
// ── The bag's own shape ──────────────────────────────────────────
328+
// [#24991] A PRESENT bag that is not an object (`null`, an array, a
329+
// scalar) is refused at the node's `properties` path, on every walked
330+
// node and whatever its `type`. `PageComponentSchema` declares
331+
// `properties` a record for every component, but that parse only ever
332+
// reaches the top-level ones (`regions[].components[]`, `slots.*`): a
333+
// NESTED component sits in a `z.array(z.unknown())` slot that no parse
334+
// judges, so until this check its `null` or `[]` bag was skipped here
335+
// and published a component stripped of every prop it was authored
336+
// with. A top-level non-record bag is the parse's refusal first: both
337+
// doors run that parse before any authoring rule and stop on it, so this
338+
// finding never sits beside it — on a top-level node it only answers a
339+
// caller that hands this rule an unparsed stack, with the same verdict.
340+
const bag = component.properties;
341+
const malformedBag = bag !== undefined && !isRec(bag);
342+
if (malformedBag) {
343+
findings.push({
344+
severity: 'error',
345+
rule: COMPONENT_PROPS_INVALID,
346+
where,
347+
path: `${path}.properties`,
348+
message:
349+
`properties: expected an object of props, received ${describeBag(bag)} — a component's ` +
350+
'`properties` is a record on every node, nested ones included, and this node carries none ' +
351+
'of its props, so it is refused and this stack does not pass until the bag is an object',
352+
hint:
353+
'Write `properties` as an object of the props this component reads (`properties: { … }`), ' +
354+
'or omit the key when it needs none.',
355+
});
356+
}
357+
358+
// Unregistered type — no props schema to judge the bag against, so the
359+
// rest is skipped. See the module header: `type` is an open union and
360+
// the majority of authored nodes are SDUI blocks this map does not carry.
318361
const schema = PROPS_SCHEMAS[type];
319362
if (!schema) continue;
320-
// [#22212] An ABSENT bag is judged as `{}` — what `PageComponentSchema`'s
321-
// `properties` default makes it. That default only ever reaches the
322-
// components the schema parses, which are the top-level ones: a NESTED
323-
// component (in another's `children` / `items[].children` / `footer`) sits
324-
// in a `z.array(z.unknown())` slot and keeps its `undefined`. Skipping
325-
// that hid every required prop such a component omits, while the same
326-
// node one level up drew `component-props-invalid`. A PRESENT bag that is
327-
// not an object is still left to the parse that owns its shape.
328-
const props =
329-
component.properties === undefined
330-
? {}
331-
: isRec(component.properties)
332-
? component.properties
333-
: undefined;
334-
const where = `page "${pageName}" · ${type}`;
335363

336364
// ── The binding an element reads ─────────────────────────────────
337365
// Judged before the props bag, and whatever the bag holds: the binding
@@ -354,7 +382,17 @@ export function validateComponentProps(stack: AnyRec): ComponentPropsFinding[] {
354382
});
355383
}
356384

357-
if (!props) continue;
385+
// A malformed bag drew its one finding above; judging its props would
386+
// only repeat it as a missing-prop verdict per required key.
387+
if (malformedBag) continue;
388+
// [#22212] An ABSENT bag is judged as `{}` — what `PageComponentSchema`'s
389+
// `properties` default makes it. That default only ever reaches the
390+
// components the schema parses, which are the top-level ones: a NESTED
391+
// component (in another's `children` / `items[].children` / `footer`) sits
392+
// in a `z.array(z.unknown())` slot and keeps its `undefined`. Skipping
393+
// that hid every required prop such a component omits, while the same
394+
// node one level up drew `component-props-invalid`.
395+
const props: AnyRec = isRec(bag) ? bag : {};
358396

359397
const base = `${path}.properties`;
360398

0 commit comments

Comments
 (0)