fix(layout): navigation-renderer 的 items 声明为 required —— 校验器不再放过必崩的节点 (#3987)

.changeset/navigation-items-required-3987.md

@@ -0,0 +1,30 @@
+---
+"@object-ui/layout": patch
+---
+
+`navigation-renderer` 的 `items` 声明为 `required: true` —— 校验器不再放过必崩的节点
+
+`items` 在组件侧是**非可选**的 `NavigationItem[]`(`NavigationRendererProps.items`,无 `?`),
+渲染器也不给默认值,而注册声明一直没写 `required`。`sdui-parser` 只在 `input.required` 为真时
+报 `missing-required-prop`(`validate.ts:55-64`),于是 `{ "type": "navigation-renderer" }`
+这个节点**校验零诊断、渲染直接抛**:第一处无守卫的读点是 `pinnedItems` memo 里的
+`collectPinnedItems(filteredItems)`(`NavigationRenderer.tsx:1242` → `:1410` 的
+`for (const item of items)`),实测 `TypeError: items is not iterable`。
+(`resolveActiveNavItem` memo 挡得住 —— 它的 `visit` 首行是 `if (!nodes) return`;
+`:1247` 的 `filteredItems.slice()` 同样会抛,但根本走不到。)
+
+这是 objectui#3972(键的**存在**与**类型**三面对齐)的第四面:**可选性**。#3972 与
+objectui#3900 都是删除假诊断,这一条相反——它是**收紧**。
+
+**blast radius:** 今天省略 `items` 写 `navigation-renderer` 的 schema,会新增一条
+**error** 级 `missing-required-prop`。受影响面是仓外按 `inputs` / `packages/layout/README.md`
+做 schema 驱动的消费者(仓内没有任何 JSON 元数据把它当 schema 节点写,React 调用侧的必填由
+TS 兜住;`examples/schema-catalog` 的 `not-a-container` 对照节点补了 `items: []`,使它只剩
+那一处故意植入的缺陷)。而这条诊断新拦下的形状,**恰好等于渲染必然崩溃的形状** —— 让作者
+(尤其是 AI 作者)在发布期就听到运行期注定要发生的失败,正是 `missing-required-prop` 存在
+的理由。若某个消费者确实想要"缺 items 就渲染空导航",那是给组件加 `= []` 默认值的另一条路
+(objectui#3987 里记了,与本改动不互斥),而不是让校验器继续沉默。
+
+`basePath` **保持可选**并被钉成对照:渲染器真的给了它默认值(`basePath = ''`)。`required`
+是逐个属性从组件读出的事实,不是一刀切——否则这道门会开始拒绝完全能渲染的 schema,作者就会
+学着无视 `missing-required-prop`,正如 #3972 里他们被教着无视 `type-mismatch`。

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).
  *
- * Four things are pinned here, and they are different facts:
+ * Five 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
@@ -23,6 +23,10 @@
  *  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.
+ *  5. VALIDATE, OPTIONALITY — a `navigation-renderer` WITHOUT `items` now draws
+ *     an error-level `missing-required-prop` (#3987), and the props the renderer
+ *     defaults still draw nothing. Unlike (3) and (4) this one is a tightening,
+ *     not a false-diagnostic fix; see that describe's header.
  *
  * (2) and (3) are two halves of one contradiction that used to be live. The
  * render path never consults `isContainer` — `SchemaRenderer` strips `children`
@@ -188,15 +192,20 @@ describe('the documented demo validates clean of `not-a-container` (#3900)', ()
     // 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. 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.
+    // `items: []` is written for the same single-fact reason the two earlier
+    // versions of this comment gave, arrived at from the opposite direction each
+    // time. It used to be OMITTED because writing it drew a `type-mismatch` (the
+    // registration said `type: 'object'` while `NavigationRendererProps.items` is
+    // `NavigationItem[]` — fixed by #3972, pinned in the describe below), and
+    // because its absence drew nothing at all. Both halves have since moved: the
+    // array form now validates clean, and the absence draws
+    // `missing-required-prop` (#3987 — omitting `items` crashes the renderer, so
+    // the declaration says `required: true`). Supplying the empty array keeps the
+    // planted defect — children under a childless component — the only thing
+    // wrong with this node.
     const codes = diagnose({
       type: 'navigation-renderer',
+      items: [],
       children: [{ type: 'button', label: 'Nope' }],
     }).map((d) => d.code);
 
@@ -282,3 +291,74 @@ describe('the declaration face matches what the renderers read (#3972)', () => {
     );
   });
 });
+
+/**
+ * The optionality face of the same key (#3987), through the same manifest.
+ *
+ * `navigation-renderer.items` is non-optional in TS and has no default, so a node
+ * that omits it throws `TypeError: items is not iterable` on the first thing the
+ * render does with it (`NavigationRenderer.tsx:1242` → `:1410`; measured in
+ * `packages/layout/src/__tests__/navigation-renderer-items-declaration.test.tsx`).
+ * The declaration used to leave `required` unset, and `validate.ts:55-64` reports
+ * `missing-required-prop` only when it is set — so the ONE node shape guaranteed
+ * to crash was also the one shape the validator had nothing to say about.
+ *
+ * This is a TIGHTENING, not a false-diagnostic fix like #3900/#3972: it adds an
+ * error-level diagnostic to schemas that validated clean yesterday. That is the
+ * point — the schemas it newly rejects are exactly the ones that cannot render —
+ * but it is also why the control below matters more than usual. "Required" has to
+ * stay a per-prop fact read off the component: if it ever becomes a blanket, this
+ * gate starts rejecting perfectly renderable schemas and authors learn to ignore
+ * `missing-required-prop` the way #3972's authors were being taught to ignore
+ * `type-mismatch`.
+ */
+describe('the validator now reports the node shape that is guaranteed to crash (#3987)', () => {
+  const REQUIRED = 'missing-required-prop';
+
+  it('reports `missing-required-prop` for a `navigation-renderer` without `items`', () => {
+    const diagnostics = diagnose({ type: 'navigation-renderer' });
+
+    // Reachability before the presence assertion: an `unknown-component` return
+    // never reaches the required-prop loop at all, so this would otherwise pass
+    // on a manifest that lost the tag.
+    expect(diagnostics.filter((d) => d.code === 'unknown-component')).toEqual([]);
+
+    const missing = diagnostics.find((d) => d.code === REQUIRED);
+    expect(missing, 'omitting `items` drew no `missing-required-prop`').toBeTruthy();
+    // Error, not warning — the render cannot recover, so neither should the gate.
+    expect(missing?.severity).toBe('error');
+    expect(missing?.message).toContain('"items"');
+  });
+
+  it('reports nothing once `items` is supplied', () => {
+    // The array form the renderer actually consumes: no `missing-required-prop`,
+    // and no `type-mismatch` either (that pair is #3972's, asserted above).
+    expect(diagnose({ type: 'navigation-renderer', items: [] }).map((d) => d.code)).toEqual([]);
+    expect(
+      diagnose({
+        type: 'navigation-renderer',
+        items: [{ id: 'home', type: 'object', label: 'Home', objectName: 'home' }],
+        basePath: '/apps/crm',
+      }).map((d) => d.code),
+    ).toEqual([]);
+  });
+
+  it('does not report the optional props the renderer defaults', () => {
+    // The control that keeps `required` a per-prop fact. `basePath` is omitted
+    // here (as it is in the first node of the test above, while the second one
+    // supplies it — both validate clean): the renderer defaults it
+    // (`basePath = ''`), the declaration leaves `required` unset, and the gate
+    // must stay silent about it. If this goes red, the tightening has spread
+    // from "the prop whose absence crashes" to "every declared prop", and the
+    // assertions above stop meaning what they say.
+    const codes = diagnose({ type: 'navigation-renderer', items: [] }).map((d) => d.code);
+    expect(codes).not.toContain(REQUIRED);
+
+    // …and the same for a second component in the same registration file, so the
+    // control is not one prop's accident: `page-header` renders with only a
+    // title, and every key it declares is optional.
+    expect(diagnose({ type: 'page-header', title: 'Users' }).map((d) => d.code)).not.toContain(
+      REQUIRED,
+    );
+  });
+});

packages/layout/src/__tests__/navigation-renderer-items-declaration.test.tsx

@@ -36,14 +36,23 @@
  * `examples/schema-catalog/test/pageheader-with-actions.test.tsx`, next to the
  * `diagnose()` helper that builds the manifest the app really validates against.
  *
+ * objectui#3987 added the fourth face of the same key — `required: true`, plus
+ * the render crash that makes it a crash-stopper rather than documentation — in
+ * the two describes at the bottom of this file, and its manifest-gate half sits
+ * beside #3972's in that same schema-catalog file.
+ *
  * 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 { describe, it, expect, vi } from 'vitest';
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { MemoryRouter } from 'react-router-dom';
+import { SidebarProvider } from '@object-ui/components';
 import { ComponentRegistry } from '@object-ui/core';
-import { registerLayout, type NavigationRendererProps } from '../index';
+import { registerLayout, NavigationRenderer, type NavigationRendererProps } from '../index';
 
 registerLayout();
 
@@ -71,3 +80,111 @@ describe('the `navigation-renderer` registration declares `items` as an array (o
     expect(ITEMS_ARE_AN_ARRAY).toBe(true);
   });
 });
+
+/**
+ * The FOURTH face of the same agreement: `items` is declared REQUIRED
+ * (objectui#3987).
+ *
+ * #3972 above aligned the key's existence and its type. Optionality is a
+ * separate fact and it was declared the opposite of what the component
+ * enforces: `NavigationRendererProps.items` has no `?`, the renderer supplies
+ * no default, and `sdui-parser` reports `missing-required-prop` only when
+ * `input.required` is set (`validate.ts:55-64`). So a bare
+ * `{ "type": "navigation-renderer" }` node passed validation in complete
+ * silence and then threw on render — the crash is witnessed by the describe
+ * below, not inferred.
+ *
+ * `basePath` is the control, and a real one rather than a nonsense key: the
+ * renderer genuinely defaults it (`basePath = ''`), so it must stay optional.
+ * "Everything is required" would be as wrong as "nothing is" — the flag is a
+ * per-prop fact about whether the component can proceed without it.
+ */
+type ItemsIsRequiredProp = undefined extends NavigationRendererProps['items'] ? false : true;
+const ITEMS_IS_REQUIRED_PROP: ItemsIsRequiredProp = true;
+
+describe('the `navigation-renderer` registration declares `items` required (objectui#3987)', () => {
+  it.each([undefined, 'layout'])('declares `required: true` (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?.required).toBe(true);
+  });
+
+  it.each([undefined, 'layout'])(
+    'leaves `basePath` optional — the renderer defaults it (namespace: %s)',
+    (namespace) => {
+      const config = ComponentRegistry.getConfig('navigation-renderer', namespace);
+      const basePath = (config?.inputs ?? []).find((input) => input.name === 'basePath');
+      expect(basePath, 'navigation-renderer no longer declares `basePath` at all').toBeTruthy();
+      // Not `toBe(false)`: the declaration omits the flag entirely, and
+      // `validate.ts` reads it as a truthiness test. What must never happen is
+      // this control turning truthy, which would mean the `required` assertion
+      // above is measuring a blanket rather than a per-prop decision.
+      expect(basePath?.required).toBeFalsy();
+    },
+  );
+
+  it('and the prop it describes is still non-optional in TS', () => {
+    // Compile-time half: if `items` ever becomes `items?: NavigationItem[]`,
+    // `ItemsIsRequiredProp` resolves to `false` and this file stops
+    // type-checking — the declaration cannot silently drift back.
+    expect(ITEMS_IS_REQUIRED_PROP).toBe(true);
+  });
+});
+
+/**
+ * Why the declaration above is a crash-stopper and not documentation
+ * (objectui#3987).
+ *
+ * objectui#3987 read the crash statically (no default + unguarded
+ * dereferences). This is the measured version, and it corrects the reading in
+ * one place: the FIRST unguarded read is `collectPinnedItems(filteredItems)` in
+ * the `pinnedItems` memo (`NavigationRenderer.tsx:1242` → `:1410`), which does
+ * `for (const item of items)`. The `resolveActiveNavItem` memo above it survives
+ * an absent list (its `visit` starts with `if (!nodes) return`), and
+ * `filteredItems.slice()` at `:1247` would throw too but is never reached.
+ *
+ * If the alternative route in objectui#3987 is ever taken — giving `items` a
+ * `= []` default so a node without it renders an empty navigation — REPLACE this
+ * describe with the empty-render assertion instead of deleting it, and keep
+ * `required: true`: an author who forgot `items` wants a diagnostic, and a
+ * silently empty sidebar is not one.
+ */
+describe('a `navigation-renderer` authored without `items` crashes on render (objectui#3987)', () => {
+  const renderNav = (props: Partial<NavigationRendererProps>) =>
+    render(
+      <MemoryRouter initialEntries={['/apps/crm']}>
+        <SidebarProvider defaultOpen>
+          <NavigationRenderer {...(props as NavigationRendererProps)} />
+        </SidebarProvider>
+      </MemoryRouter>,
+    );
+
+  it('throws a TypeError instead of rendering an empty navigation', () => {
+    // React logs the render error on its own; the assertion is about the throw,
+    // so keep the captured output clean.
+    const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
+    try {
+      // `basePath` is supplied so the ONLY missing prop is the required one.
+      expect(() => renderNav({ basePath: '/apps/crm' })).toThrow(TypeError);
+      // The wording is V8's for `for…of` over `undefined`; the load-bearing part
+      // is the throw, but pinning the text catches a swap to some other failure
+      // mode that would need this whole describe re-read.
+      expect(() => renderNav({ basePath: '/apps/crm' })).toThrow(/items is not iterable/);
+    } finally {
+      consoleError.mockRestore();
+    }
+  });
+
+  it('renders fine when `items` is supplied and the optional `basePath` is not', () => {
+    // The other half of the control pair: the props the declaration calls
+    // optional really are optional, so the throw above is about `items` and not
+    // about rendering this component bare in a test.
+    expect(() =>
+      renderNav({ items: [{ id: 'nav_accounts', type: 'object', label: 'Accounts', objectName: 'account' }] }),
+    ).not.toThrow();
+    expect(screen.getByText('Accounts')).toBeTruthy();
+  });
+});

packages/layout/src/index.ts

@@ -160,12 +160,31 @@ export function registerLayout() {
   // 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.
+  //
+  // `required: true` is the fourth face of the same agreement (objectui#3987).
+  // #3972 aligned the key's EXISTENCE and TYPE; optionality was still declared
+  // the opposite of what the component enforces. `NavigationRendererProps.items`
+  // has no `?` and the renderer supplies no default (`NavigationRenderer.tsx:1204`),
+  // so `{ "type": "navigation-renderer" }` — a node the validator passed in
+  // silence, because `validate.ts:55-64` only reports `missing-required-prop`
+  // when `input.required` is set — crashes on the first thing the render does
+  // with the prop: `collectPinnedItems(filteredItems)` at `:1242` does
+  // `for (const item of items)` (`:1410`) and throws
+  // `TypeError: items is not iterable`. (The `resolveActiveNavItem` memo above
+  // it survives, its `visit` guards `if (!nodes) return`; `filteredItems.slice()`
+  // at `:1247` would throw too but is never reached.)
+  //
+  // So this is not a stylistic "document it as required" — it is the one
+  // diagnostic that exists precisely to stop a node whose render is a
+  // guaranteed crash from shipping. `basePath` below stays optional because the
+  // renderer really does default it (`basePath = ''`); the two are declared
+  // differently because the component treats them differently.
   ComponentRegistry.register('navigation-renderer', NavigationRenderer, {
     namespace: 'layout',
     label: 'Navigation Renderer',
     category: 'Layout',
     inputs: [
-      { name: 'items', type: 'array' },
+      { name: 'items', type: 'array', required: true },
       { name: 'basePath', type: 'string' },
     ],
   });