From a0c35dd7d2dd2795ba2cdc0c91584de79c63a0ed Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 18:44:46 +0000 Subject: [PATCH] =?UTF-8?q?fix(layout):=20page-header=20=E6=B3=A8=E5=86=8C?= =?UTF-8?q?=E8=A1=A5=20isContainer:=20true,=E6=A0=A1=E9=AA=8C=E5=99=A8?= =?UTF-8?q?=E4=B8=8D=E5=86=8D=E5=AF=B9=E6=96=87=E6=A1=A3=E6=89=BF=E8=AF=BA?= =?UTF-8?q?=E7=9A=84=20children=20=E5=86=99=E6=B3=95=E6=8A=A5=20not-a-cont?= =?UTF-8?q?ainer=20(#3900)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PageHeader 一直有意把 schema.children 渲染进右侧动作槽(PageHeader.tsx:182), docs 把该槽的优先级写成公开契约,文档页唯一的 live demo 正是这个形状且实测正常 渲染;但 packages/layout/src/index.ts 的注册漏了 isContainer: true。 漏这个 flag 从来没挡住任何渲染 —— 渲染路径根本不读它,消费者是 sdui-parser 的 not-a-container 诊断、Studio 调色板、react-page 标签表。所以后果是校验器在说谎: 作者照文档写出能正常渲染的 schema 却拿到一条 warning,而会说谎的 warning 会训练 作者(尤其 AI 作者)连真实的 not-a-container 一起无视。 children 是 objectui JSON 协议里每个节点的基础属性(validate.ts 的 BASE_PROPS), 不是 PageHeaderProps 的键,所以补这个 flag 不在 spec 之外新开作者面 —— 维护者 2026-08-09 就 #3900 的 A/B 分叉按此理由裁定 A 案。 两个方向的钉子:文档 demo 走应用真实构建的 manifest 后不再产生 not-a-container (先断言无 unknown-component 且 demo 确实带 children,避免缺席假绿);阳性对照 navigation-renderer 带 children 时诊断照旧触发。 Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- .changeset/page-header-is-container-3900.md | 36 ++++++ examples/schema-catalog/package.json | 1 + .../test/pageheader-with-actions.test.tsx | 110 ++++++++++++++++-- .../page-header-authorable-keys.test.tsx | 30 +++++ packages/layout/src/index.ts | 34 ++++-- pnpm-lock.yaml | 3 + 6 files changed, 195 insertions(+), 19 deletions(-) create mode 100644 .changeset/page-header-is-container-3900.md diff --git a/.changeset/page-header-is-container-3900.md b/.changeset/page-header-is-container-3900.md new file mode 100644 index 0000000000..948a689968 --- /dev/null +++ b/.changeset/page-header-is-container-3900.md @@ -0,0 +1,36 @@ +--- +"@object-ui/layout": patch +--- + +`page-header` 注册补 `isContainer: true` —— 校验器不再对文档承诺的 children 写法报 `not-a-container` + +`PageHeader` 一直**有意**把 `schema.children` 渲染进右侧动作槽(`PageHeader.tsx:182`, +`record:quick_actions` 嵌在 `page:header.children` 下就是靠它), +`content/docs/layout/page-header.mdx` 把该槽的优先级(`action` → React `children` → +`actions` → schema children)写成公开契约,该文档页唯一的 live demo +(`layout-page-header/pageheader-with-actions`)正是这个形状且实测正常渲染。但 +`packages/layout/src/index.ts` 的注册漏了 `isContainer: true`。 + +漏这个 flag 从来没有挡住任何渲染 —— 渲染路径根本不读它(`SchemaRenderer` 把 +`children` 从 React props 里剥掉当元数据,而始终把整个节点作为 `schema` 传下去, +`PageHeader` 自己再把 `schema.children` 放回槽里)。它的消费者在别处:`sdui-parser` +的 `not-a-container` 诊断、Studio 调色板元数据、react-page 标签表。所以真正的后果是 +**校验器在说谎**:作者照文档写出能正常渲染的 schema,却拿到一条 +"`page-header` does not accept children" 的 warning;信了这条 warning 去掉 children, +右槽就空掉。而会说谎的 warning 比缺一条 warning 更贵 —— 它训练作者(尤其 AI 作者) +连真实的 `not-a-container`(那些确实不收子节点的组件)一起无视。 + +这不是在 spec 之外新开作者面:`children` 是 objectui JSON 协议里**每个节点**的基础属性 +(`sdui-parser/src/validate.ts` 的 `BASE_PROPS` 把它和 `type`/`id`/`className` 并列), +不是 `PageHeaderProps` 的键。所以这个 flag 回答的是协议层面的"该节点是否接受子节点列表", +而对这个组件,答案一直是"是"。维护者 2026-08-09 就 objectui#3900 的 A/B 分叉裁定 A 案, +理由同上。 + +行为面变化极窄:注册元数据一个布尔位。渲染输出逐字节不变(渲染路径不读该 flag); +`sdui-parser` 对带 children 的 `page-header` 少报一条 warning;设计器把它当容器对待 +(即它本来的样子)。canonical 的 `page:header`(`@object-ui/components`)不在此列且刻意不动 +—— 那个渲染器完全不读 `schema.children`,所以它没有 `isContainer` 是正确的。 + +两个方向都已钉住:文档 demo 走应用真实构建的 manifest 后不再产生 `not-a-container`, +而一个真正不收子节点的组件(`navigation-renderer`)带 children 时诊断照旧触发 —— +后者是前者的对照,保证这条修复不是把诊断弄哑了。 diff --git a/examples/schema-catalog/package.json b/examples/schema-catalog/package.json index 9533fde03e..6aba441290 100644 --- a/examples/schema-catalog/package.json +++ b/examples/schema-catalog/package.json @@ -33,6 +33,7 @@ "@object-ui/core": "workspace:*", "@object-ui/layout": "workspace:*", "@object-ui/react": "workspace:*", + "@object-ui/sdui-parser": "workspace:*", "typescript": "^6.0.3" } } diff --git a/examples/schema-catalog/test/pageheader-with-actions.test.tsx b/examples/schema-catalog/test/pageheader-with-actions.test.tsx index c0b5ac3d27..c911607e99 100644 --- a/examples/schema-catalog/test/pageheader-with-actions.test.tsx +++ b/examples/schema-catalog/test/pageheader-with-actions.test.tsx @@ -10,7 +10,7 @@ * rendering, and it held a third, already-drifted copy of the component's * spacing numbers (objectui#3786 fixed the second copy, in the prose). * - * Two things are pinned here, and they are different facts: + * Three things are pinned here, and they are different facts: * * 1. SHAPE — the example's root node is a `page-header`, and it contains none * of the class strings that only exist inside `PageHeader.tsx`. This is the @@ -18,16 +18,23 @@ * 2. RENDER — driven through the real `SchemaRenderer`, the node produces the * header: an `

` title, the subtitle, and BOTH schema children in the * right-hand slot. + * 3. VALIDATE — the same JSON, put through the manifest the app really builds + * from the live registry, draws no `not-a-container` diagnostic (#3900). * - * (2) is worth a test rather than an eyeball because the registration for - * `page-header` (`packages/layout/src/index.ts`) does NOT declare - * `isContainer: true`, which reads like children could not reach the slot. - * They do: `isContainer` is registry metadata that the render path never - * consults (its consumers are `sdui-parser`'s `not-a-container` diagnostic, the - * Studio palette, and the react-page tag map). `SchemaRenderer` strips - * `children` from the React props but always passes the whole node as `schema`, - * and `PageHeader` re-introduces `schema.children` itself. That is a load- - * bearing coincidence of two files, so it gets a pin. + * (2) and (3) are two halves of one contradiction that used to be live. The + * render path never consults `isContainer` — `SchemaRenderer` strips `children` + * from the React props but always passes the whole node as `schema`, and + * `PageHeader` re-introduces `schema.children` itself — so the header rendered + * its children correctly while `packages/layout/src/index.ts` omitted + * `isContainer: true` from the registration. The flag's consumers are elsewhere: + * `sdui-parser`'s `not-a-container` diagnostic, the Studio palette, and the + * react-page tag map. So the omission never blocked anything; it made the + * VALIDATOR contradict the docs, the demo and the render — an author following + * this very page got a warning telling them their working schema was invalid. + * #3900 added the flag (maintainer ruling, route A: `children` is a base + * property of every protocol node, not a `PageHeaderProps` key, so declaring it + * mints no authoring surface outside the spec). (3) is what keeps the two faces + * from drifting apart again in either direction. * * Module-scope imports, not `beforeAll` (AGENTS.md §测试纪律): the child * `button` node resolves through `@object-ui/components`' registration @@ -37,8 +44,11 @@ import { describe, it, expect, beforeAll } from 'vitest'; import { render, screen } from '@testing-library/react'; import '@object-ui/components'; +import { ComponentRegistry } from '@object-ui/core'; import { SchemaRenderer } from '@object-ui/react'; import { registerLayout } from '@object-ui/layout'; +import { manifestFromConfigs, validateTree } from '@object-ui/sdui-parser'; +import type { Diagnostic, SchemaElement } from '@object-ui/sdui-parser'; import { getExample } from '../src/index.js'; const EXAMPLE_ID = 'layout-page-header/pageheader-with-actions'; @@ -59,6 +69,25 @@ beforeAll(() => { registerLayout(); }); +/** + * The manifest the running app validates against, built the way the app builds + * it — keyed by every KNOWN registry tag (bare and namespaced) rather than by + * `getAllConfigs()`, whose `.type` is always the namespaced form. Mirrors + * `getJsxManifest()` in `packages/components/src/renderers/layout/page.tsx` + * (module-private, hence the four lines here): key it off `getAllConfigs()` + * instead and the bare `page-header` tag authors write is absent from the + * manifest, so every assertion below would pass on `unknown-component` and + * never reach the containment check at all. + */ +const diagnose = (schema: unknown): Diagnostic[] => { + const configs = ComponentRegistry.getKnownTypes().map((t) => { + const meta = ComponentRegistry.getMeta(t); + return { type: t, namespace: meta?.namespace, isContainer: meta?.isContainer, inputs: meta?.inputs }; + }); + const manifest = manifestFromConfigs(configs as unknown as Parameters[0]); + return validateTree(schema as SchemaElement, manifest).diagnostics; +}; + describe('the page-header docs demo uses the page-header component (#3787)', () => { it('is rooted at a `page-header` node', () => { const schema = getExample(EXAMPLE_ID).schema as { type?: string }; @@ -88,8 +117,10 @@ describe('the page-header docs demo uses the page-header component (#3787)', () expect(h1?.textContent).toBe('Users'); expect(screen.getByText('Manage your team members and permissions')).toBeTruthy(); - // Both children reach the right-hand slot despite the registration not - // declaring `isContainer` — see the module header. + // Both children reach the right-hand slot. This held even while the + // registration omitted `isContainer` — the render path never reads the flag + // (see the module header), which is why the omission was invisible here and + // only the validator complained. expect(screen.getByRole('button', { name: 'Export' })).toBeTruthy(); expect(screen.getByRole('button', { name: 'Add User' })).toBeTruthy(); }); @@ -114,3 +145,58 @@ describe('the page-header docs demo uses the page-header component (#3787)', () expect(cls).not.toMatch(/\b(sm|md|lg|xl):pb-/); }); }); + +/** + * The validator agrees with the docs, the demo and the render (#3900). + * + * Two directions, and BOTH are load-bearing. Asserting only the first would + * leave the suite green if someone silenced the containment check outright, or + * flipped `isContainer` on by default — the test would then be measuring + * nothing. The second case is the control that keeps the first one meaningful. + */ +describe('the documented demo validates clean of `not-a-container` (#3900)', () => { + const CONTAINMENT = 'not-a-container'; + + it('reports no `not-a-container` for the demo the docs page ships', () => { + const schema = getExample(EXAMPLE_ID).schema as { children?: unknown[] }; + const diagnostics = diagnose(schema); + + // Reachability BEFORE the absence — an empty result proves nothing if the + // containment branch never ran. Two ways it silently would not: + // `validateTree` reports `unknown-component` and returns before the + // containment check when the tag is missing from the manifest, and the + // branch is guarded by `node.children?.length`, so a demo that lost its + // children would satisfy the assertion below for the wrong reason. + expect(diagnostics.filter((d) => d.code === 'unknown-component')).toEqual([]); + expect(schema.children?.length ?? 0).toBeGreaterThan(0); + + // Filtered by code, not asserted against an empty diagnostic list: the demo + // also writes `icon`, which the registration's `inputs` still omits even + // though `PageHeader.tsx:224` renders it, so a live `unknown-prop` sits in + // this list. That is the same family as #3900 on a different key, filed + // separately as objectui#3972 and deliberately NOT pinned here — this test + // must not go red when that one is fixed. + expect(diagnostics.filter((d) => d.code === CONTAINMENT)).toEqual([]); + }); + + it('still reports `not-a-container` for a component that genuinely takes none', () => { + // The control. `navigation-renderer` (registered in the SAME file as the + // #3900 change) is driven entirely by its `items` prop and never reads + // `schema.children`, so children under it ARE an authoring mistake and the + // author must still hear about it. If this component ever legitimately + // becomes a container, this assertion goes red — move the control to + // another childless registration rather than deleting it. + // `items` is deliberately omitted (it is not `required`, so its absence + // draws nothing): this control is about containment only, and writing + // `items: []` would also draw a `type-mismatch` — the registration declares + // that prop `type: 'object'` while `NavigationRendererProps.items` is + // `NavigationItem[]`, a separate declaration-face defect filed as + // objectui#3972. + const codes = diagnose({ + type: 'navigation-renderer', + children: [{ type: 'button', label: 'Nope' }], + }).map((d) => d.code); + + expect(codes).toContain(CONTAINMENT); + }); +}); diff --git a/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx b/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx index 8402d0d3e5..0b94b21352 100644 --- a/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx +++ b/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx @@ -91,6 +91,36 @@ describe('the `page-header` registration declares the spec key, not a dialect', }); }); +describe('the `page-header` registration declares the child slot it renders (objectui#3900)', () => { + // Same principle as the `inputs` narrowing above, other direction: the + // declaration face must not DENY a surface the component serves either. + // `PageHeader.tsx:182` deliberately renders `schema.children` into the + // right-hand slot, `content/docs/layout/page-header.mdx` publishes that slot's + // precedence, and the docs page's only live demo is exactly that shape — while + // the registration omitted `isContainer`, so `sdui-parser`'s `not-a-container` + // diagnostic fired on it. Nothing on the render path reads the flag, so the + // omission broke no rendering; it made the validator tell authors (AI authors + // especially) that a documented, demo-verified schema was invalid, which is + // how the true `not-a-container` reports lose their credibility. + // + // Not an extension of the spec's authoring surface: `children` is a base + // property of every node in objectui's JSON protocol (`BASE_PROPS` in + // `sdui-parser/src/validate.ts`), not a key of `PageHeaderProps` — hence the + // `specKeys` cross-check above neither covers nor contradicts this. + // + // The end-to-end half of this pin — the real demo JSON through the manifest + // the app actually builds, plus the control proving the diagnostic still fires + // for a genuinely childless component — lives in + // `examples/schema-catalog/test/pageheader-with-actions.test.tsx`, next to the + // fixture it validates. + it.each([ + ['page-header', undefined], + ['page-header', 'layout'], + ])('marks %s (namespace: %s) as a container', (type, namespace) => { + expect(ComponentRegistry.getConfig(type, namespace)?.isContainer).toBe(true); + }); +}); + describe('the runtime `description` fallback stays until the conversion entry lands', () => { // NOT an endorsement of the alias — a guard on the ORDER. Removing this read // before `page-header-subtitle-alias` exists is the deletion route that was diff --git a/packages/layout/src/index.ts b/packages/layout/src/index.ts index 09b9e19dd1..203489e40c 100644 --- a/packages/layout/src/index.ts +++ b/packages/layout/src/index.ts @@ -47,14 +47,34 @@ export function registerLayout() { // which lives in the framework repo. Narrowing the DECLARATION is // unconditional and independent of that: it breaks no consumer, and it stops // the registry from teaching the wrong key in the meantime. + // + // `isContainer: true` is the other half of the same principle, and it is NOT + // an extension of the spec's authoring surface (objectui#3900). `children` is + // a base property of EVERY node in objectui's JSON protocol — `BASE_PROPS` in + // `sdui-parser/src/validate.ts` lists it beside `type`/`id`/`className` — not + // a key of the spec's `PageHeaderProps`. So this flag answers the protocol + // question "does this node accept a child list?", and for this component the + // answer has always been yes: `PageHeader.tsx` deliberately re-introduces + // `schema.children` into the right-hand slot (that is how `record:quick_actions` + // nests under `page:header.children`), `content/docs/layout/page-header.mdx` + // publishes the slot's precedence as contract, and the docs page's only live + // demo is exactly that shape. + // + // Leaving the flag off did not make children illegal — nothing on the render + // path reads `isContainer` — it made the VALIDATOR lie: `sdui-parser`'s + // `not-a-container` diagnostic fired on the documented, demo-verified, + // correctly-rendering write-up. A warning that lies is worse than a missing + // one, because it trains authors (AI authors especially) to discount the true + // `not-a-container` reports on components that really are childless. ComponentRegistry.register('page-header', PageHeader, { - namespace: 'layout', - label: 'Page Header', - category: 'Layout', - inputs: [ - { name: 'title', type: 'string', label: 'Title' }, - { name: 'subtitle', type: 'string', label: 'Subtitle' } - ] + namespace: 'layout', + label: 'Page Header', + category: 'Layout', + isContainer: true, + inputs: [ + { name: 'title', type: 'string', label: 'Title' }, + { name: 'subtitle', type: 'string', label: 'Subtitle' }, + ], }); // Page Card — register ONLY as `layout:page:card`. `skipFallback` keeps this diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f902fc198f..460ca528e1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -642,6 +642,9 @@ importers: '@object-ui/react': specifier: workspace:* version: link:../../packages/react + '@object-ui/sdui-parser': + specifier: workspace:* + version: link:../../packages/sdui-parser typescript: specifier: ^6.0.3 version: 6.0.3