Skip to content

Commit f764691

Browse files
os-zhuangclaude
andauthored
fix(spec): the page-component conversion walk reaches nested containers, not only regions and slots (#6775) (#7034)
`mapPageComponents` visited `pages[].regions[].components[]` and `pages[].slots.<slot>` and stopped. A component nested inside another component's `properties` — a card's `children` / `body` / `footer`, a `page:tabs` / `page:accordion` panel's `items[].children` — was visited by nobody, so no page-component conversion rewrote it, while `walkPageComponents` in `packages/lint` has descended into those containers from the start. Every conversion therefore reached strictly less than the lint rule that judges its result. The walk now descends into the same containers lint does, to any depth, with the same path spelling, so a conversion notice and a lint finding name one site with one string. The mapper still runs on the container first and the descent reads the MAPPED component, so `page-card-body-to-children` — which moves `properties.body` to `properties.children` — walks its sub-tree exactly once, under the canonical key. Copy-on-write is unchanged: an untouched sub-tree keeps its reference, and a stack where nothing converts is returned by identity. The load-path cost this fixes belongs to `page-header-subtitle-alias`. Every other entry leans on a tombstone for the sites a conversion cannot reach; this one has none, because `description` stays a live declared prop on other components, so `properties.description` parses green at any position. A header authored in a card or on a `kind: 'slotted'` record page got no rewrite and no diagnostic from any layer. Fixtures for all seven walker-backed conversions now pin the slotted and nested positions beside the region-level one, and a cross-walker parity test in `packages/lint` — the only place that can see both walkers — asserts the two reachable sets agree, pinning the two deliberate differences: lint skips source-authored pages (the conversion must still normalize their derived cache) and the conversion walk keeps a 32-container depth ceiling for hand-built `defineStack` objects. No conversion registry entry added or removed. Claude-Session: https://claude.ai/code/session_01Cd32yJ2omZsgUXxiAjeBcE Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3de535b commit f764691

5 files changed

Lines changed: 1021 additions & 72 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): a page-component conversion reaches components nested inside a container, not only region- and slot-level ones (#6775)
6+
7+
`mapPageComponents` — the walker every page-component conversion is built on —
8+
visited `pages[].regions[].components[]` and `pages[].slots.<slot>` and stopped
9+
there. A component nested inside another component's `properties` (a card's
10+
`children` / `body` / `footer`, a `page:tabs` or `page:accordion` panel's
11+
`items[].children`) was never visited, so **no** page-component conversion
12+
rewrote it. `walkPageComponents` in `@objectstack/lint` has descended into
13+
those containers from the start, which means every conversion reached strictly
14+
less than the lint rule that judges its result.
15+
16+
The walker now descends into the same containers lint does, to any depth, with
17+
the same path spelling — so a conversion notice and a lint finding name one
18+
site with one string. Copy-on-write is unchanged: an untouched sub-tree keeps
19+
its reference, and a stack where nothing converts is still returned by
20+
identity.
21+
22+
**Why this mattered on the load path.** The usual answer for a site a
23+
conversion cannot reach is the tombstone: the key is typed `never`, so `tsc`
24+
refuses it at the authoring site and the parse refuses it at load, wherever it
25+
sits. That answer does not hold for a key that stays live elsewhere on the
26+
surface. `page-header-subtitle-alias` retires `description` on page-header
27+
components, and `description` remains a declared prop on other components (an
28+
`element:text_input`'s helper text), so it cannot be tombstoned —
29+
`properties.description` parses green at *any* position. A header authored in
30+
a card or inside a `kind: 'slotted'` record page therefore got no rewrite and
31+
no diagnostic from any of the three layers: the conversion did not fire, the
32+
page schema was satisfied (`properties` is an open bag nothing validates by
33+
`type` on the load path), and the props check is advisory, CLI-only, and runs
34+
on already-converted metadata. Retiring the consumer-side
35+
`subtitle ?? description` fallback would have dropped those pages' second line
36+
silently.
37+
38+
Every page-component conversion rides the widened walk and its fixture now
39+
pins the nested and slotted positions alongside the region-level one:
40+
`page-header-subtitle-alias`, `record-picker-display-field-to-label-field`,
41+
`record-picker-inert-keys-removed`, `page-card-body-to-children`,
42+
`inline-action-api-params-to-body-extra`, `page-tabs-type-to-tab-style`, and
43+
`page-component-visibility-to-visibleWhen`.
44+
45+
`page-card-body-to-children` is the one interaction worth naming: it MOVES a
46+
container key (`properties.body``properties.children`). The descent reads
47+
the mapped component, so a nested sub-tree is walked exactly once — under the
48+
canonical key, not once per spelling.
49+
50+
Two differences from the lint walk remain, both deliberate and both pinned by
51+
a cross-walker parity test: source-authored pages (`kind: 'html' | 'react' |
52+
'jsx'`) are skipped by lint and still converted here (their regions are a
53+
derived cache that must be normalized, or a stored page rehydrates in a shape
54+
the runtime no longer serves), and the conversion walk keeps a depth ceiling of
55+
32 containers, which lint has no counterpart for because it never runs on
56+
hand-built `defineStack` objects.
57+
58+
No conversion was added or removed, and no already-converted metadata changes
59+
shape: this widens which authoring positions the existing rewrites reach.
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The two page-component walkers must reach the same components (#6775).
5+
*
6+
* There are two of them and there has to be: `walkPageComponents` (here) yields
7+
* nodes for the lint rules to judge, and `mapPageComponents`
8+
* (`@objectstack/spec`'s conversion layer) rewrites them copy-on-write. What
9+
* must NOT differ is which components each one arrives at — a conversion that
10+
* reaches less than the rule judging its result normalizes part of a corpus and
11+
* leaves the rest looking converted, which is exactly what #6775 measured:
12+
* `page-header-subtitle-alias` rewrote a header in a region and skipped the
13+
* identical header in a slot or inside a card, with no diagnostic from any
14+
* layer (the props bag is unvalidated on the load path, and that key has no
15+
* tombstone to fall back on).
16+
*
17+
* This file is the only place that can see both, since `@objectstack/lint`
18+
* depends on `@objectstack/spec` and not the other way round. The parity is
19+
* asserted BEHAVIOURALLY — every position this walk yields is a position a
20+
* conversion notice names — rather than by comparing implementations, so it
21+
* keeps holding if either walk is rewritten.
22+
*
23+
* One difference is deliberate and pinned below: source-authored pages
24+
* (`kind: 'html' | 'react' | 'jsx'`) are skipped here and visited there. Lint
25+
* skips them so it does not report findings about a DERIVED region cache the
26+
* author never wrote; a conversion still has to normalize that cache, or a
27+
* stored page rehydrates in a shape the runtime no longer serves.
28+
*/
29+
30+
import { applyConversions } from '@objectstack/spec';
31+
import { describe, expect, it } from 'vitest';
32+
33+
import { walkPageComponents } from './page-walk.js';
34+
35+
/**
36+
* A page-header authored with the retired `description` spelling — the probe.
37+
* `page-header-subtitle-alias` rewrites it to `subtitle` and emits a notice
38+
* whose path names the site, so "did the conversion reach here?" is answerable
39+
* for any position without exporting the walker itself.
40+
*/
41+
const probe = (title: string) => ({ type: 'page:header', properties: { title, description: 'Second line' } });
42+
43+
/**
44+
* Every authoring position in one page: both region slots, a single-component
45+
* slot and an array slot, and each container a component nests a sub-tree in
46+
* (`children`, `items[].children`, `body`, `footer`), including two levels of
47+
* nesting.
48+
*/
49+
const page = {
50+
name: 'parity',
51+
kind: 'slotted',
52+
object: 'account',
53+
regions: [
54+
{
55+
name: 'main',
56+
components: [
57+
probe('region'),
58+
{ type: 'page:section', properties: { children: [probe('children')] } },
59+
{
60+
type: 'page:tabs',
61+
properties: { tabStyle: 'line', items: [{ label: 'T', children: [probe('tab panel')] }] },
62+
},
63+
{
64+
type: 'page:card',
65+
properties: {
66+
body: [probe('card body')],
67+
footer: [{ type: 'page:section', properties: { children: [probe('two deep')] } }],
68+
},
69+
},
70+
],
71+
},
72+
],
73+
slots: {
74+
header: probe('single slot'),
75+
details: [probe('array slot 0'), probe('array slot 1')],
76+
},
77+
};
78+
79+
/** The positions the lint walk yields that carry the probe. */
80+
const walkedProbePaths = () =>
81+
walkPageComponents(page as unknown as Record<string, unknown>, 'pages[0]')
82+
.filter((w) => w.component.type === 'page:header')
83+
.map((w) => w.path);
84+
85+
/**
86+
* The positions the conversion layer actually rewrote the probe at.
87+
*
88+
* Filtered to this one entry: the fixture page also carries a `page:card` with
89+
* a `body`, which `page-card-body-to-children` rewrites — a real notice about a
90+
* different key, and not a position the probe sits at.
91+
*/
92+
const convertedProbePaths = () => {
93+
const paths: string[] = [];
94+
applyConversions(
95+
{ pages: [structuredClone(page)] },
96+
{
97+
includeRetired: true,
98+
onNotice: (n) => { if (n.conversionId === 'page-header-subtitle-alias') paths.push(n.path); },
99+
},
100+
);
101+
// The notice names the rewritten KEY; the component is its parent.
102+
return paths.map((p) => p.replace(/\.properties\.subtitle$/, ''));
103+
};
104+
105+
describe('#6775 — walkPageComponents and the conversion walk reach the same components', () => {
106+
it('the probe sits at every position the lint walk knows about', () => {
107+
// Guards the fixture itself: if a container shape is added to the lint walk
108+
// and not to this page, the parity assertion below would pass vacuously.
109+
expect(walkedProbePaths()).toEqual([
110+
'pages[0].regions[0].components[0]',
111+
'pages[0].regions[0].components[1].properties.children[0]',
112+
'pages[0].regions[0].components[2].properties.items[0].children[0]',
113+
'pages[0].regions[0].components[3].properties.body[0]',
114+
'pages[0].regions[0].components[3].properties.footer[0].properties.children[0]',
115+
'pages[0].slots.header',
116+
'pages[0].slots.details[0]',
117+
'pages[0].slots.details[1]',
118+
]);
119+
});
120+
121+
it('a conversion rewrites the probe at every one of them, spelling the same paths', () => {
122+
// Order-insensitive: the two walks are free to visit in different orders,
123+
// but neither may reach a component the other cannot.
124+
expect(new Set(convertedProbePaths())).toEqual(new Set(walkedProbePaths()));
125+
});
126+
127+
it('source-authored pages are the one deliberate difference', () => {
128+
// Lint yields nothing for them (the regions are a derived cache, not
129+
// authored metadata); the conversion still normalizes that cache.
130+
const jsxPage = { name: 'j', kind: 'jsx', source: '<div/>', regions: [{ name: 'main', components: [probe('cached')] }] };
131+
expect(walkPageComponents(jsxPage as unknown as Record<string, unknown>, 'pages[0]')).toEqual([]);
132+
133+
const notices: string[] = [];
134+
applyConversions(
135+
{ pages: [structuredClone(jsxPage)] },
136+
{
137+
includeRetired: true,
138+
// `kind: 'jsx'` itself converts (`page-kind-jsx-to-html`, protocol 11);
139+
// what this pins is the component inside the derived cache.
140+
onNotice: (n) => { if (n.conversionId === 'page-header-subtitle-alias') notices.push(n.path); },
141+
},
142+
);
143+
expect(notices).toEqual(['pages[0].regions[0].components[0].properties.subtitle']);
144+
});
145+
});

0 commit comments

Comments
 (0)