From b9f8e9c713a7ef21e28b38ca91de920c2667a9a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 20:36:53 +0000 Subject: [PATCH] =?UTF-8?q?fix(layout):=20registerLayout=20=E7=9A=84=20inp?= =?UTF-8?q?uts=20=E5=A3=B0=E6=98=8E=E9=9D=A2=E5=AF=B9=E9=BD=90=E6=B8=B2?= =?UTF-8?q?=E6=9F=93=E5=99=A8=E8=AF=BB=E7=82=B9=20(#3972)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit page-header 补声明 icon 与 actions(渲染器真读 + spec 声明 + ManifestInputType 表达得了),navigation-renderer 的 items 由 type:'object' 改为 'array' (NavigationRendererProps.items 是 NavigationItem[])。 声明面即作者面:sdui-parser 拿节点顶层属性比对 comp.inputs,漏声明的键报 unknown-prop、类型写错的报 type-mismatch —— 两处都是对着正确写法报假诊断, 其中 icon 那条今天就落在 content/docs/layout/page-header.mdx 唯一的 live demo 上。 渲染输出不变(渲染路径不读 inputs)。 两方向都钉住:文档 demo 过 manifest 门无 unknown-prop、数组 items 无 type-mismatch;对照侧 description(#3226 刻意不声明)仍报 unknown-prop、 items 写成对象仍报 type-mismatch 且消息已变为 expected an array。 breadcrumb / showBack / action / aria 的刻意不声明也各自钉了理由。 Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt Co-authored-by: Claude --- .changeset/layout-inputs-declaration-3972.md | 38 +++++++ .../test/pageheader-with-actions.test.tsx | 106 ++++++++++++++++-- ...gation-renderer-items-declaration.test.tsx | 73 ++++++++++++ .../page-header-authorable-keys.test.tsx | 52 ++++++++- packages/layout/src/index.ts | 55 ++++++++- 5 files changed, 309 insertions(+), 15 deletions(-) create mode 100644 .changeset/layout-inputs-declaration-3972.md create mode 100644 packages/layout/src/__tests__/navigation-renderer-items-declaration.test.tsx diff --git a/.changeset/layout-inputs-declaration-3972.md b/.changeset/layout-inputs-declaration-3972.md new file mode 100644 index 0000000000..bb81db34e0 --- /dev/null +++ b/.changeset/layout-inputs-declaration-3972.md @@ -0,0 +1,38 @@ +--- +"@object-ui/layout": patch +--- + +`registerLayout()` 的 `inputs` 声明面与渲染器实现对齐 —— 校验器不再对正确写法报假诊断 + +`inputs` 是**作者面**:`sdui-parser/src/validate.ts` 拿一个节点的顶层属性逐个比对 +`comp.inputs`,不在其中的报 `unknown-prop`,类型不符的报 `type-mismatch`;设计器面板、 +`sdui.manifest.json`、生成的 JSX 类型也都由它派生。所以声明面写错的代价不是"文档不全", +而是**校验器对着能正常渲染的 schema 说谎**(objectui#3972,与 objectui#3900 同族、换到属性面)。 + +两处修正,都是按渲染器**实际读点**审计出来的: + +- **`page-header` 补声明 `icon` 与 `actions`。** 两个键三面齐全:渲染器真读 + (`PageHeader.tsx:117` 取参 `:224-226` 渲染 icon;`:119` 取参 `:192-196` 把 actions + 委派给 `record:quick_actions`)、`@objectstack/spec/ui` 的 `PageHeaderProps` 声明、 + `ManifestInputType` 表达得了(`string` / `array`)。`content/docs/layout/page-header.mdx` + 的 Component Props 段落也把两者写成公开契约,而该文档页**唯一**的 live demo + (`layout-page-header/pageheader-with-actions`)就写着 `"icon": "users"` —— 于是仓内 + 自己的文档示例每次过 manifest 门都收到一条 `unknown-prop: page-header has no prop "icon"`。 + `actions` 的类型与 canonical 的 `page:header` 逐字一致(`type: 'array'`),一个概念一个键 + 一种类型。 +- **`navigation-renderer` 的 `items` 由 `type: 'object'` 改为 `'array'`。** + `NavigationRendererProps.items` 是 `NavigationItem[]`(`NavigationRenderer.tsx:108`), + 而 `checkType` 对 `'object'` 判 `typeof value === 'object' && !Array.isArray(value)`、 + 对 `'array'` 判 `Array.isArray`(`validate.ts:124-129`)—— 两者互斥。所以旧声明对这个 + 渲染器**唯一能渲染的形状**报 `type-mismatch ... expected an object`,而对真会让它崩的 + 对象形状一言不发。这不是 objectui#3832 的表达力问题:`ManifestInputType` 本来就有 + `'array'`,只是声明写错了一个表达得了的类型。 + +**刻意不声明**的键同样被钉住,因为"照抄 spec 的 shape"是这条修复最容易滑进去的反向缺陷: +`breadcrumb`(spec 有、这个渲染器零读点 —— 声明它就是 objectui#3829 的缺陷方向)、 +`showBack` / `action` / `description`(渲染器读、spec 无此键 —— 声明任一个就是在开第二套 +方言,正是 objectui#3226 收窄要防的事)、`aria`(每个 block 都因同一理由省略的可访问性逃逸口)。 + +渲染输出逐字节不变:渲染路径从不读 `inputs`。变化只在校验/设计器/清单这一侧,且两个方向 +都钉了 —— 正确写法放行的同时,`description` 这类刻意不声明的键仍报 `unknown-prop`、 +`items` 写成对象仍报 `type-mismatch`(诊断没有被弄哑)。 diff --git a/examples/schema-catalog/test/pageheader-with-actions.test.tsx b/examples/schema-catalog/test/pageheader-with-actions.test.tsx index c911607e99..2bebc81b2e 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). * - * Three things are pinned here, and they are different facts: + * Four 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 @@ -20,6 +20,9 @@ * 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). + * 4. VALIDATE, PROPS — the same JSON draws no `unknown-prop` either, and an + * array-valued `navigation-renderer.items` draws no `type-mismatch` (#3972). + * Same lie as (3) on the prop face instead of the containment face. * * (2) and (3) are two halves of one contradiction that used to be live. The * render path never consults `isContainer` — `SchemaRenderer` strips `children` @@ -170,12 +173,11 @@ describe('the documented demo validates clean of `not-a-container` (#3900)', () 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. + // Filtered by code, not asserted against an empty diagnostic list. When this + // was written the demo's `icon` drew a live `unknown-prop` — the same lie as + // #3900 on the prop face — which #3972 has since fixed and pins in the + // describe below. The filter stays: this test owns the CONTAINMENT fact only, + // so a future diagnostic on another key belongs to that key's pin, not here. expect(diagnostics.filter((d) => d.code === CONTAINMENT)).toEqual([]); }); @@ -187,11 +189,12 @@ describe('the documented demo validates clean of `not-a-container` (#3900)', () // 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. + // draws nothing): this control is about containment only. It used to have a + // second reason — writing `items: []` ALSO drew a `type-mismatch`, because the + // registration declared that prop `type: 'object'` while + // `NavigationRendererProps.items` is `NavigationItem[]`. That defect is fixed + // (#3972) and pinned in the describe below; the omission here is now about + // keeping this control single-fact, nothing more. const codes = diagnose({ type: 'navigation-renderer', children: [{ type: 'button', label: 'Nope' }], @@ -200,3 +203,82 @@ describe('the documented demo validates clean of `not-a-container` (#3900)', () expect(codes).toContain(CONTAINMENT); }); }); + +/** + * The prop face of the same agreement (#3972). + * + * `registerLayout()`'s `inputs` lists are what `sdui-parser` validates a node's + * top-level props against, so a key the renderer reads and `inputs` omits comes + * back as `unknown-prop`, and a key declared with the wrong `type` comes back as + * `type-mismatch` — on CORRECT authoring, both times. Two keys were in that + * state: `page-header.icon` (rendered at `PageHeader.tsx:224-226`, declared by the + * spec, written by the demo below) and `navigation-renderer.items` (declared + * `object`, actually `NavigationItem[]`). + * + * Every assertion here comes in pairs, positive then control, because "no + * diagnostic" is exactly what a silenced check also looks like: if `unknown-prop` + * or `type-mismatch` were removed from `validate.ts`, or `inputs` were widened to + * accept anything, the positive halves would all still pass. The controls are + * chosen so they can only stay green while the check is still working. + */ +describe('the declaration face matches what the renderers read (#3972)', () => { + const codesFor = (schema: unknown): string[] => diagnose(schema).map((d) => d.code); + + it('draws no `unknown-prop` on the demo the docs page ships', () => { + const schema = getExample(EXAMPLE_ID).schema as Record; + + // Reachability first: the demo must still WRITE `icon`, otherwise the absence + // below is satisfied by a fixture that stopped exercising the key. (The docs + // page documents `icon` in its Component Props block, so a demo without one + // is its own defect.) + expect(schema.icon).toBe('users'); + expect(diagnose(schema).filter((d) => d.code === 'unknown-component')).toEqual([]); + + expect(diagnose(schema).filter((d) => d.code === 'unknown-prop')).toEqual([]); + }); + + it('still draws `unknown-prop` for a key the declaration deliberately withholds', () => { + // The control, and it is a real key rather than a nonsense one: `description` + // is the legacy alias `PageHeader.tsx:143` still READS, which objectui#3226 + // removed from `inputs` on purpose so the registry stops teaching a second + // dialect for `subtitle`. It must keep drawing `unknown-prop` — that warning + // is the narrowing doing its job. If this goes green, either the alias was + // re-declared or the check stopped running, and the assertion above is then + // measuring nothing. + const schema = { ...(getExample(EXAMPLE_ID).schema as Record), description: 'x' }; + expect(codesFor(schema)).toContain('unknown-prop'); + }); + + it('accepts the `actions` array the docs page documents', () => { + // `actions` is read at `PageHeader.tsx:119` / `:192-196` and declared by the + // spec, so it was `unknown-prop` for the same reason `icon` was. Type matters + // as much as presence here: it is declared `array`, matching the canonical + // `page:header`, so the array form authors write validates clean… + expect(codesFor({ type: 'page-header', title: 'Users', actions: ['export'] })).toEqual([]); + // …and a non-array is now reported instead of silently accepted. + expect(codesFor({ type: 'page-header', title: 'Users', actions: { export: true } })).toContain( + 'type-mismatch', + ); + }); + + it('accepts an array-valued `navigation-renderer.items`, and reports an object', () => { + expect(codesFor({ type: 'navigation-renderer', items: [] })).toEqual([]); + expect( + codesFor({ + type: 'navigation-renderer', + items: [{ id: 'home', type: 'object', label: 'Home', objectName: 'home' }], + }), + ).toEqual([]); + + // The control, and the direction is inverted rather than absent: this exact + // object shape was the ONLY one that validated clean before #3972, so the pin + // is not "a diagnostic disappeared" but "the two shapes swapped verdicts". + // The message is asserted too — a `type-mismatch` still saying "expected an + // object" would mean the declaration never moved. + const objectValued = diagnose({ type: 'navigation-renderer', items: { home: {} } }); + expect(objectValued.map((d) => d.code)).toContain('type-mismatch'); + expect(objectValued.find((d) => d.code === 'type-mismatch')?.message).toContain( + 'expected an array', + ); + }); +}); diff --git a/packages/layout/src/__tests__/navigation-renderer-items-declaration.test.tsx b/packages/layout/src/__tests__/navigation-renderer-items-declaration.test.tsx new file mode 100644 index 0000000000..e426a06755 --- /dev/null +++ b/packages/layout/src/__tests__/navigation-renderer-items-declaration.test.tsx @@ -0,0 +1,73 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `navigation-renderer` declares `items` as the ARRAY it is (objectui#3972). + * + * The registration used to say `{ name: 'items', type: 'object' }` while + * `NavigationRendererProps.items` is `NavigationItem[]`. In `sdui-parser` those + * two are mutually exclusive checks, not loose synonyms + * (`src/validate.ts:124-129`): + * + * 'array' -> Array.isArray(value) + * 'object' -> typeof value === 'object' && value !== null && !Array.isArray(value) + * + * So the manifest gate reported `type-mismatch: prop + * "items" expected an object` on the only value this renderer can render, and + * said nothing about the object that would crash it. A gate that is wrong about + * the correct shape is worse than no gate: it teaches authors — AI authors above + * all, which is who reads a manifest — to write the shape the component cannot + * consume, and to discount `type-mismatch` in general. + * + * Two halves are pinned here, and they fail for different reasons: + * + * 1. the DECLARATION says `array` (runtime, from the live registry); + * 2. the PROP really is an array (compile-time, from the component's own type), + * so if `items` is ever reshaped, `pnpm type-check` fails here instead of the + * declaration silently drifting back out of alignment. + * + * The manifest-gate half — an array-valued `items` node drawing no + * `type-mismatch`, with an object-valued one still drawing it — lives in + * `examples/schema-catalog/test/pageheader-with-actions.test.tsx`, next to the + * `diagnose()` helper that builds the manifest the app really validates against. + * + * Module-scope import of the barrel, not `beforeAll` (AGENTS.md §测试纪律): the + * registration is a load-time side effect of `../index`, and its transform cost + * belongs to the import phase rather than a hook's 10s budget. + */ + +import { describe, it, expect } from 'vitest'; +import { ComponentRegistry } from '@object-ui/core'; +import { registerLayout, type NavigationRendererProps } from '../index'; + +registerLayout(); + +/** + * Compile-time half of the pin. `NavigationRendererProps['items']` must remain + * assignable to an array; when it is not, this constant stops type-checking and + * the message points at the declaration that has to move with it. + */ +type ItemsAreAnArray = NavigationRendererProps['items'] extends readonly unknown[] ? true : false; +const ITEMS_ARE_AN_ARRAY: ItemsAreAnArray = true; + +describe('the `navigation-renderer` registration declares `items` as an array (objectui#3972)', () => { + it.each([undefined, 'layout'])('declares `type: array` (namespace: %s)', (namespace) => { + const config = ComponentRegistry.getConfig('navigation-renderer', namespace); + expect(config, 'navigation-renderer is not registered').toBeTruthy(); + + const items = (config?.inputs ?? []).find((input) => input.name === 'items'); + expect(items, 'navigation-renderer no longer declares `items` at all').toBeTruthy(); + expect(items?.type).toBe('array'); + }); + + it('and the prop it describes is still an array', () => { + // Asserted rather than merely declared above so the compile-time half is + // visible in the run output too — a `const` nobody reads is easy to delete. + expect(ITEMS_ARE_AN_ARRAY).toBe(true); + }); +}); 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 0b94b21352..8462c2a2fa 100644 --- a/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx +++ b/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx @@ -52,12 +52,15 @@ import { registerLayout, PageHeader } from '../index'; /** Authorable keys of the spec node this renderer serves. */ const specKeys = new Set(Object.keys(SpecPageHeaderProps.shape)); -const declaredInputNames = (type: string, namespace?: string): string[] => { +const declaredInputs = (type: string, namespace?: string) => { const config = ComponentRegistry.getConfig(type, namespace); if (!config) throw new Error(`"${namespace ? `${namespace}:${type}` : type}" is not registered`); - return (config.inputs ?? []).map((input) => input.name); + return config.inputs ?? []; }; +const declaredInputNames = (type: string, namespace?: string): string[] => + declaredInputs(type, namespace).map((input) => input.name); + beforeAll(() => { registerLayout(); }); @@ -121,6 +124,51 @@ describe('the `page-header` registration declares the child slot it renders (obj }); }); +describe('the `page-header` registration declares the keys the renderer reads (objectui#3972)', () => { + // The `declares nothing @objectstack/spec does not` test above is ONE-WAY: it + // catches a declared key the spec rejects, and can never catch a spec key the + // renderer honours while `inputs` omits it. That omission is not a documentation + // gap — `sdui-parser/src/validate.ts:70-77` reports `unknown-prop` for any + // top-level key not in `inputs`, so the manifest gate warned authors off keys + // this component renders. `icon` was live on the repo's own documented demo. + // + // Both keys pass the same three-face test (renderer read point × spec key × + // a `ManifestInputType` that can spell the value); the omissions below pin the + // audit's negative results, which is what keeps this from becoming "copy the + // spec's shape into `inputs`". + it.each([ + ['icon', 'string'], + ['actions', 'array'], + ])('declares `%s` with type `%s`, on both registration keys', (name, type) => { + for (const namespace of [undefined, 'layout']) { + const input = declaredInputs('page-header', namespace).find((i) => i.name === name); + expect(input, `page-header (namespace: ${namespace}) does not declare \`${name}\``).toBeTruthy(); + expect(input?.type).toBe(type); + } + // …and it is legal to declare only because the spec owns the key. If the spec + // ever drops it, this line fails here rather than as a mystery parity red. + expect(specKeys.has(name)).toBe(true); + }); + + it('does not declare `breadcrumb` — a spec key this renderer never reads', () => { + // The other direction of the same audit, and the reason it is not "declare + // every spec key": `PageHeader.tsx` has no `breadcrumb` read point at all + // (the word occurs only in a comment and an `aria-label`). Declaring it would + // publish configuration the component silently drops — objectui#3829's defect, + // which is the exact mistake this file's positive assertions could invite. + expect(specKeys.has('breadcrumb')).toBe(true); + expect(declaredInputNames('page-header')).not.toContain('breadcrumb'); + }); + + it.each(['showBack', 'action', 'description'])( + 'does not declare `%s` — read by the renderer, but no spec key exists', + (name) => { + expect(specKeys.has(name)).toBe(false); + expect(declaredInputNames('page-header')).not.toContain(name); + }, + ); +}); + 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 203489e40c..e7353a67e3 100644 --- a/packages/layout/src/index.ts +++ b/packages/layout/src/index.ts @@ -66,6 +66,40 @@ export function registerLayout() { // 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. + // + // `icon` and `actions` are the same lie on two more keys, found by auditing + // this list against the renderer's actual read points (objectui#3972). Each + // declared key below is aligned on the three faces that have to agree, and the + // audit's negative results are as load-bearing as its positive ones: + // + // RENDERER READS IT × SPEC DECLARES IT × `ManifestInputType` CAN SPELL IT + // - `title` / `subtitle` — read at `PageHeader.tsx:113/115`, spec keys. + // - `icon` — read at `:117`, rendered at `:224-226` (a string goes + // through `LazyIcon`, a node renders as-is); `PageHeaderProps.icon` is the + // spec's icon NAME, hence `type: 'string'`. `content/docs/layout/ + // page-header.mdx` documents it AND its only live demo + // (`layout-page-header/pageheader-with-actions`) writes `"icon": "users"` + // — so omitting it made the manifest gate report `unknown-prop` on the + // repo's own documented example. + // - `actions` — read at `:119`, resolved at `:192-196` and delegated to + // `record:quick_actions`; `PageHeaderProps.actions` is an array of action + // ids, and the canonical `page:header` already publishes it as + // `type: 'array'` (`components/.../containers.tsx:1585`). Spelled + // identically here on purpose — one concept, one key, one type. + // + // NOT declared, deliberately, and each for its own reason: + // - `breadcrumb` — spec declares it, this renderer has NO read point (the + // word appears only in a comment and an `aria-label`). Declaring it would + // be objectui#3829's defect in reverse: an authoring surface the platform + // silently drops. (`page:header.icon` is that same case on the CANONICAL + // renderer, which is why it sits in `UNPUBLISHED_EXEMPTIONS` in + // `apps/console/src/__tests__/registry-inputs-spec-parity.test.ts` while + // `icon` gets declared HERE — different renderers, opposite read facts.) + // - `showBack` / `action` / `description` — this renderer reads them, the + // spec has no such keys. Declaring one would publish a second dialect, + // which is the whole point of the objectui#3226 narrowing above. + // - `aria` — spec declares it; omitted for the reason every block omits it + // (accessibility escape hatch, not a layout choice). ComponentRegistry.register('page-header', PageHeader, { namespace: 'layout', label: 'Page Header', @@ -74,6 +108,13 @@ export function registerLayout() { inputs: [ { name: 'title', type: 'string', label: 'Title' }, { name: 'subtitle', type: 'string', label: 'Subtitle' }, + { name: 'icon', type: 'string', label: 'Icon', description: 'Lucide icon name' }, + { + name: 'actions', + type: 'array', + label: 'Actions', + description: 'ActionDef list (or action ids) rendered through record:quick_actions', + }, ], }); @@ -107,12 +148,24 @@ export function registerLayout() { ], }); + // `items` is `NavigationItem[]` (`NavigationRenderer.tsx:108`) — an ARRAY, and + // it used to be declared `type: 'object'` (objectui#3972). Those are not two + // spellings of one check: `sdui-parser`'s `checkType` accepts `'object'` only + // for `typeof value === 'object' && !Array.isArray(value)` and `'array'` only + // for `Array.isArray(value)` (`validate.ts:124-129`), so the declaration made + // the manifest gate report `type-mismatch: prop "items" + // expected an object` on the ONLY value this renderer can render — and stay + // silent on the object that would crash it. + // + // This is not objectui#3832 (`ComponentInput.type` cannot spell a spec union): + // `ManifestInputType` has `'array'`, so the declaration was simply wrong about + // a type it could express exactly. ComponentRegistry.register('navigation-renderer', NavigationRenderer, { namespace: 'layout', label: 'Navigation Renderer', category: 'Layout', inputs: [ - { name: 'items', type: 'object' }, + { name: 'items', type: 'array' }, { name: 'basePath', type: 'string' }, ], });