From 9b15ccf54161cde41bf811e8c778dbb4dd9e122a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 05:39:29 +0000 Subject: [PATCH] =?UTF-8?q?fix(console):=20preview=20=E7=94=BB=E5=BB=8A?= =?UTF-8?q?=E7=9A=84=20page=20=E6=A0=B7=E4=BE=8B=E6=94=B9=E7=94=A8?= =?UTF-8?q?=E6=B3=A8=E5=86=8C=E8=BF=87=E7=9A=84=E5=9D=97=E7=B1=BB=E5=9E=8B?= =?UTF-8?q?,=E5=B9=B6=E5=8A=A0=E6=B3=A8=E5=86=8C=E8=A1=A8=E5=8F=AF?= =?UTF-8?q?=E8=A7=A3=E6=9E=90=E6=96=AD=E8=A8=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `preview-samples.ts` 的 page 样例用 `{ type: 'heading' }` 开头,而 `heading` 在本仓从未注册过——全仓 `register('heading'` / `registerLazy('heading'` 零命中。 spec 并不会拦下它:`PageComponentSchema.type` 是 `z.union([PageComponentType, z.string()])`,枚举**或**任意字符串(好让插件贡献块),所以拼错的类型是完全合法的 元数据。两个节点因此一路解析干净、渲染成 ComponentRegistry fallback:画廊里 「一个组合页面长什么样」的样板,标题与小节标题的位置各摆着一个红色错误框。 同一处还有第二个毛病:配置放在 `props` 下。`PageComponentSchema` 是 `.strict()`, 按名字拒收 `props`(ADR-0089 D3a)——这正是 `page` 一直躺在 `preview-samples-spec-valid.test.ts` 的 KNOWN_STALE 账本里的唯一原因。两件事其实 是一个缺陷的两张脸:`page:header` 从 `schema` / `schema.properties` 上读 `title`, 只改类型不改 bag 的话,标题栏照样是空的。 改法(类型全部取自 `PageComponentType`): • 页面标题 → `page:header`,由它承担 h1、副标题、面包屑/操作槽与下边框; `recordChrome: false` 选择非记录页的裸标题布局。 • 正文小节标题 → `element:text` + `variant: 'subheading'`(渲染 h3,正是原来 `level: 3` 想要的)。`heading` 是 element:text 的**变体**,从来不是类型—— 原样例就错在这个擦肩而过。 • 分隔线 → `element:divider`。原来的裸 `separator` 确实能解析(`ui:separator` 占着裸名),但它不是页面块类型。 新增 `preview-samples-registry-resolvable.test.ts`:递归走遍所有样例的 `regions[].components[]`(含嵌套 children/body/items 与 properties.* 下的同名键), 断言每个 type 都能 `has()` 或 `hasLazy()`。注册表从 console 真正启动的那两个模块图 读取,不手抄已知类型清单——手抄的清单只会自己跟自己吻合。另配两道防空转断言: 遍历确实走到了 page 样例;以及塞进 page:card 里的未注册类型确实会被报出来。 `page` 随之从 KNOWN_STALE 提升进 SPEC_CLEAN——这不是可选项,账本的反向断言 (「修好了就提升」)在样例变得合法的那一刻就会红。 浏览器复核(preview-gallery.html?only=page,无后端):改前两个 `heading` 是 「Unknown component type: heading (OBJUI-001)」错误框,两个 `text` 是空白卡片 (`ui:text` 读 `schema.content`,根本不看 `props.text`);改后 h1「Welcome to the CRM」、h3「Quick links」、正文与分隔线齐全,页面上再无 fallback 标记。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- ...review-samples-registry-resolvable.test.ts | 193 ++++++++++++++++++ .../preview-samples-spec-valid.test.ts | 14 +- apps/console/src/preview-samples.ts | 37 +++- 3 files changed, 236 insertions(+), 8 deletions(-) create mode 100644 apps/console/src/__tests__/preview-samples-registry-resolvable.test.ts diff --git a/apps/console/src/__tests__/preview-samples-registry-resolvable.test.ts b/apps/console/src/__tests__/preview-samples-registry-resolvable.test.ts new file mode 100644 index 0000000000..7e9eb7c149 --- /dev/null +++ b/apps/console/src/__tests__/preview-samples-registry-resolvable.test.ts @@ -0,0 +1,193 @@ +/** + * 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. + * + * `preview-samples.ts` page blocks ↔ `ComponentRegistry` resolvability + * (objectui#3446). + * + * ## Why the spec guard is not enough + * + * `preview-samples-spec-valid.test.ts` already asks whether a sample would + * survive being published. It cannot ask this question, and the gap is by + * design rather than by oversight: `PageComponentSchema.type` is + * + * z.union([ PageComponentType, z.string() ]) + * + * — the enum OR any string, so that a page may host a block a plugin + * registered outside the spec's own catalog. A misspelt or invented block type + * is therefore perfectly valid metadata. `{ type: 'heading' }` parsed clean + * through every spec release while resolving to nothing: `heading` has never + * been registered in this repo (the nearest real things are `element:text` + * with `variant: 'heading'`, which renders an h2, and the `ui/typography` + * primitives, which never go through the registry at all). Both `heading` + * nodes in the `page` sample fell through to the ComponentRegistry fallback, + * so the gallery's page card — the worked example of a composed page — showed + * two fallback boxes where its title and its section heading should have been. + * + * The samples are not fixtures. They are what an author (increasingly, a model + * generating metadata) reads as "a page looks like this", so a block type that + * resolves to nothing is a broken example that propagates. This test is the + * missing half of that feedback loop: the spec guard pins the SHAPE, this one + * pins that every type in the shape names something that can actually render. + * + * ## What resolvable means here + * + * `has(type) || hasLazy(type)` — a loaded registration or a pending + * `registerLazy` stub. Both are legitimate: a lazily registered tag is a full + * member of the contract and only differs by when its chunk is imported + * (objectui#2953). `getConfig` would be the wrong probe for the same reason — + * it is deliberately loaded-only. + * + * Registrations are read from the two module graphs the console actually boots + * from (`@object-ui/components` + `../register-plugins`), the same posture as + * `public-contract.test.ts` and `public-block-binding-reach.test.tsx`: a + * hand-copied list of known types would agree with itself and prove nothing. + * + * ## Scope, stated so a pass is not over-read + * + * Resolvable is not the same as CORRECT. This asserts that a type names a real + * renderer, not that the renderer reads the props the sample hands it — a + * node can resolve and still render blank if its config sits under the wrong + * key. That is a separate question, and the spec guard is what covers it from + * the other side (the canonical bag is `properties`; `props` is rejected by + * name under ADR-0089 D3a). + * + * Dashboard `widgets[]` are deliberately NOT walked. Those types are component + * types too, but half of them (`metric`, `dashboard-grid`, `pivot`, …) are + * registered only by `preview-gallery.tsx`'s own `registerLazy` block, and + * that module mounts React at import time so a test cannot import it. Walking + * them against this bootstrap would report unresolved types that resolve + * perfectly well in the gallery, and re-declaring the gallery's list here + * would be exactly the self-agreeing copy described above. The walk covers + * page-component trees, which is where the objectui#3446 defect lived; the + * dashboard sample is separately ledgered as KNOWN_STALE in the spec guard. + */ + +import { describe, it, expect } from 'vitest'; +import { ComponentRegistry } from '@object-ui/core'; +// The two graphs whose registrations this reads — the layout/content +// primitives and the console's own plugin layer, from the module main.tsx +// boots from. +import '@object-ui/components'; +import '../register-plugins'; +import { SAMPLES } from '../preview-samples'; + +/** One component node found by the walk, with the path that located it. */ +interface FoundNode { + /** e.g. `page.regions[0].components[1]` */ + path: string; + type: string; +} + +/** + * Child keys a component node may carry its subtree under. Mirrors the + * candidate list the runtime containers themselves walk + * (`packages/components/src/renderers/layout/containers.tsx` — + * `children` / `properties.children` / `properties.items` / `body` / `items`), + * plus `components`, the key the page spec uses inside a region. Kept in sync + * with the runtime deliberately: a subtree the renderer descends into but this + * walk does not is precisely where an unresolvable type would hide. + */ +const CHILD_KEYS = ['components', 'children', 'body', 'items'] as const; + +function isRecord(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value); +} + +/** Push every component node under `value`, depth-first, into `out`. */ +function walkNodes(value: unknown, path: string, out: FoundNode[]): void { + if (Array.isArray(value)) { + value.forEach((item, i) => walkNodes(item, `${path}[${i}]`, out)); + return; + } + if (!isRecord(value)) return; + + if (typeof value.type === 'string') out.push({ path, type: value.type }); + + const properties = isRecord(value.properties) ? value.properties : undefined; + for (const key of CHILD_KEYS) { + if (value[key] !== undefined) walkNodes(value[key], `${path}.${key}`, out); + if (properties?.[key] !== undefined) { + walkNodes(properties[key], `${path}.properties.${key}`, out); + } + } +} + +/** + * Every page-component node across every sample, found via `regions[]`. Generic + * over samples rather than reaching for `SAMPLES.page`, so a sample that grows + * regions tomorrow is covered without anyone remembering to add it here. + */ +function collectPageComponents(samples: Record): FoundNode[] { + const out: FoundNode[] = []; + for (const [sampleType, sample] of Object.entries(samples)) { + if (!isRecord(sample) || !Array.isArray(sample.regions)) continue; + sample.regions.forEach((region: unknown, i: number) => { + if (!isRecord(region)) return; + walkNodes(region.components, `${sampleType}.regions[${i}].components`, out); + }); + } + return out; +} + +const resolvable = (type: string): boolean => + ComponentRegistry.has(type) || ComponentRegistry.hasLazy(type); + +describe('preview-samples page blocks resolve in the ComponentRegistry', () => { + const found = collectPageComponents(SAMPLES); + + it('every page-component type names a registered renderer', () => { + const unresolved = found + .filter((node) => !resolvable(node.type)) + .map((node) => `${node.path}: "${node.type}"`); + expect(unresolved).toEqual([]); + }); + + /** + * Anti-vacuity guard #1 — coverage. An empty walk would pass the assertion + * above without checking anything, which is the failure mode this whole file + * exists to end. Asserting the sample keys the walk reached (rather than a + * count) also makes a sample that silently loses its regions visible. + */ + it('the walk actually reaches the samples that compose regions', () => { + const reached = [...new Set(found.map((n) => n.path.split('.')[0]))].sort(); + expect(reached).toContain('page'); + expect(found.length).toBeGreaterThan(0); + }); + + /** + * Anti-vacuity guard #2 — teeth. The assertion is only worth its green if the + * walk descends into nested subtrees; a walk that stopped at the first level + * would report `[]` for a page whose unresolvable block sits inside a card. + * Probed with a synthetic draft rather than by mutating SAMPLES, so this + * stays true regardless of what the real samples happen to contain. + */ + it('reports an unresolvable type nested inside a container block', () => { + const synthetic = { + probe: { + regions: [ + { + name: 'main', + components: [ + { + type: 'page:card', + properties: { + children: [{ type: 'no_such_block_type_3446' }], + }, + }, + ], + }, + ], + }, + }; + const unresolved = collectPageComponents(synthetic) + .filter((node) => !resolvable(node.type)) + .map((node) => `${node.path}: "${node.type}"`); + expect(unresolved).toEqual([ + 'probe.regions[0].components[0].properties.children[0]: "no_such_block_type_3446"', + ]); + }); +}); diff --git a/apps/console/src/__tests__/preview-samples-spec-valid.test.ts b/apps/console/src/__tests__/preview-samples-spec-valid.test.ts index e634b10d2b..1a9925aec7 100644 --- a/apps/console/src/__tests__/preview-samples-spec-valid.test.ts +++ b/apps/console/src/__tests__/preview-samples-spec-valid.test.ts @@ -107,6 +107,14 @@ const SPEC_CLEAN = [ 'validation', 'datasource', 'app', + // Promoted from KNOWN_STALE by objectui#3446. The page's components carried + // their config under `props`, which `PageComponentSchema.strict()` rejects by + // name (ADR-0089 D3a) — the sole reason this row was quarantined. Fixed as + // part of making the sample's block types resolvable: the mis-layered bag and + // the unregistered `heading` type were one defect wearing two faces, since + // `page:header` reads its title off `schema`/`schema.properties` and a `props` + // bag would have rendered an empty header bar even with the type corrected. + 'page', ] as const; /** @@ -116,9 +124,10 @@ const SPEC_CLEAN = [ * This list is a ledger, not an excuse: the reverse assertion below fails if an * entry starts passing, so it can only ever shrink. objectui#3266 emptied out * the mechanically-fixable half of it (`action`, `agent`, `skill`, `flow`, - * `report`, `validation`, `datasource`, `app` — all now in SPEC_CLEAN above). + * `report`, `validation`, `datasource`, `app` — all now in SPEC_CLEAN above), + * and objectui#3446 took `page` the same way. * - * What remains is NOT "not got to yet" — each of these four needs a decision + * What remains is NOT "not got to yet" — each of these three needs a decision * that does not belong in this file, and the reason is recorded with it. A * sample only leaves this ledger by being FIXED; relaxing the guard, exempting * a schema, or moving a row into NO_AUTHORING_SCHEMA to buy a green run would @@ -133,7 +142,6 @@ const KNOWN_STALE: Record = { // which is the actual question (AGENTS.md #0.1) — and it is an app-shell // question, not a preview-samples one. object: '`fields` is an array; ObjectSchema wants a record keyed by field name', - page: 'page components carry `props`, which PageComponentSchema rejects (ADR-0089 D3a)', dashboard: 'widgets miss `dataset`/`values` and use retired `value`/`format`; `chart` is not a widget type', translation: 'the `translations` collection is Array< Record< locale, TranslationData > >, but this sample is the metadata-RECORD form (name/label/locale/data) the console edits — so this row is a mapping mismatch, not necessarily a stale sample. Settle which contract the sample targets before guarding it.', diff --git a/apps/console/src/preview-samples.ts b/apps/console/src/preview-samples.ts index a756f688cf..83355a309b 100644 --- a/apps/console/src/preview-samples.ts +++ b/apps/console/src/preview-samples.ts @@ -19,6 +19,33 @@ export const SAMPLES: Record> = { ], }, + // Every `type` here must resolve in the ComponentRegistry, and every config + // bag must be `properties` — both are guarded, and both were broken + // (objectui#3446). The page previously opened with `{ type: 'heading' }`, + // which has never been registered in this repo: the spec accepts it + // (`PageComponentSchema.type` is the enum OR any string, so plugins can + // contribute blocks), so it parsed clean while rendering as a + // ComponentRegistry fallback box — the gallery's example of a composed page + // was showing two placeholders where its title and section heading belong. + // + // The canonical spellings, all from `PageComponentType`: + // • the page title is a `page:header` (not a level-1 text block) — it owns + // the h1, the subtitle, the breadcrumb/action slots and the bottom rule; + // `recordChrome: false` selects the bare non-record layout, since this is + // a welcome page with no bound record. + // • in-body headings are `element:text` with a variant — `subheading` + // renders an h3, which is what the old `level: 3` asked for. (`heading` + // is a VARIANT of element:text, never a type; that near-miss is how the + // original went wrong.) + // • the rule is `element:divider`. The old bare `separator` did resolve — + // `ui:separator` claims the bare name — but it is not a page block type, + // and the `element:*` family is what the designer palette offers. + // + // Config lives under `properties`, NOT `props`: PageComponentSchema is + // `.strict()` and rejects `props` by name (ADR-0089 D3a). That mis-layering + // is why `page` sat in this file's spec-conformance ledger; the two are one + // fix, because `page:header` reads `title` off `schema`/`schema.properties` + // and would have rendered an empty header bar from a `props` bag. page: { name: 'crm_welcome', label: 'CRM Welcome', @@ -26,11 +53,11 @@ export const SAMPLES: Record> = { { name: 'main', components: [ - { type: 'heading', props: { level: 1, text: 'Welcome to the CRM' } }, - { type: 'text', props: { text: 'Track accounts, contacts, and deals in one place.' } }, - { type: 'separator' }, - { type: 'heading', props: { level: 3, text: 'Quick links' } }, - { type: 'text', props: { text: 'Open the pipeline, review tasks, or create a new lead.' } }, + { type: 'page:header', properties: { title: 'Welcome to the CRM', recordChrome: false } }, + { type: 'element:text', properties: { content: 'Track accounts, contacts, and deals in one place.' } }, + { type: 'element:divider' }, + { type: 'element:text', properties: { content: 'Quick links', variant: 'subheading' } }, + { type: 'element:text', properties: { content: 'Open the pipeline, review tasks, or create a new lead.' } }, ], }, ],