diff --git a/.changeset/8626-detail-section-authored-node.md b/.changeset/8626-detail-section-authored-node.md new file mode 100644 index 0000000000..753cf33138 --- /dev/null +++ b/.changeset/8626-detail-section-authored-node.md @@ -0,0 +1,40 @@ +--- +'@object-ui/plugin-detail': patch +--- + +An authored `detail-section` node now renders. It used to draw an error banner. + +The registration declares eight FLAT inputs — `title`, `description`, `fields`, +`collapsible`, `defaultCollapsed`, `columns`, `showBorder`, `headerColor` — +while `DetailSection` declares a single `section` OBJECT prop and reads +`section.*` only. `SchemaRenderer` spreads a node's non-metadata keys as React +props, so an authored node arrived as `title` / `fields` / … and `section` +arrived `undefined`. + +Not merely inert. `DetailSection`'s first statement is +`React.useState(section.defaultCollapsed ?? false)`, so the render THREW and +`SchemaErrorBoundary` put its orange banner on the page in place of the block — +measured end to end through the real `SchemaRenderer` and the real registry: + +``` +Component "detail-section" failed to render +Cannot read properties of undefined (reading 'defaultCollapsed') +``` + +The tag is now registered against a seam adapter that folds those eight +declared inputs into the `section` object the component reads — the same shape +of repair this package already uses for `field:permission-facet-link` +(`withFieldCarrier`, objectui#3307). + +**The authoring surface did not move, deliberately.** The other available +repair — re-declaring the eight as a nested `section` input — would have +changed what authors may write, and the flat shape is both published and +authored: a manifest built the way `PageRenderer` builds the JSX-page +compiler's gives the flat eight ZERO diagnostics and REFUSES `section` +(`unknown-prop`, plus `missing-required-prop "fields"`), this package's README +documents a flat `detail-section` node inside `tabs[].content`, and +objectui#6955's landed pin asserts that same flat surface. Folding at the seam +invalidates none of them. + +`DetailSection` itself is byte-identical: every in-repo caller passes +`section={…}` as a direct JSX child and is untouched. diff --git a/packages/plugin-detail/src/DetailSectionNode.tsx b/packages/plugin-detail/src/DetailSectionNode.tsx new file mode 100644 index 0000000000..c6d59c835a --- /dev/null +++ b/packages/plugin-detail/src/DetailSectionNode.tsx @@ -0,0 +1,148 @@ +/** + * 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. + */ + +/** + * The component the `detail-section` TAG is registered against — the seam that + * makes the block's eight declared `inputs` true (objectui#8626). + * + * ## The defect this closes + * + * `ComponentRegistry.register('detail-section', …)` declares eight FLAT inputs + * — `title`, `description`, `fields`, `collapsible`, `defaultCollapsed`, + * `columns`, `showBorder`, `headerColor`. `DetailSection` declares no such + * props: it takes a single `section` object and reads `section.title`, + * `section.fields`, `section.defaultCollapsed` and the rest off it. The two + * are joined by `SchemaRenderer`, which spreads a node's non-metadata keys as + * React props — so an authored node arrived as `title` / `fields` / … and + * `section` arrived as `undefined`. + * + * That is not "every input inert". MEASURED on `b775500af` by rendering an + * authored node through the real `SchemaRenderer` and the real registry: + * `DetailSection`'s first statement is `React.useState(section.defaultCollapsed + * ?? false)`, so the render THREW, `SchemaErrorBoundary` caught it, and the + * author's page showed an orange banner reading + * `Component "detail-section" failed to render — Cannot read properties of + * undefined (reading 'defaultCollapsed')`. Every authored key was lost AND the + * block became a visible hole. + * + * ## Why the fold, and not a re-declaration + * + * The two repairs objectui#8626 names are not symmetric, and the measurement + * decides between them rather than taste: + * + * - The flat eight are the PUBLISHED surface, and the platform enforces them. + * `packages/components/src/renderers/layout/page.tsx` builds the JSX-page + * compiler's manifest from `getKnownTypes()` plus these `inputs`, and + * `sdui-parser`'s `validateTree` judges an authored page against it. + * Measured against that live manifest: the flat eight draw ZERO + * diagnostics, while `section` draws `unknown-prop` AND + * `missing-required-prop "fields"`. The validator does not merely permit + * the flat shape — it REFUSES the nested one. + * - The flat shape is authored: `packages/plugin-detail/README.md` documents + * a `detail-section` node inside `tabs[].content` (which `DetailTabs` + * renders through `SchemaRenderer`), and + * `__tests__/detailSectionHeaderColorEnum-6955.test.ts` pins the flat + * `fields` + `headerColor` surface against that same validator. + * + * Re-declaring the registration as a nested `section` object would therefore + * invalidate both, and would delete the surface objectui#6955 had just + * narrowed. Folding at the seam invalidates nothing: every authored node keeps + * validating and starts rendering. + * + * ## Why HERE and not inside `DetailSection` + * + * `DetailSection` is a published export with five in-repo callers + * (`DetailView`, `SectionGroup` and their tests), every one of which passes + * `section={…}` as a direct JSX child — none goes through the registry. Teaching + * the component two prop shapes would put a second dialect in front of all of + * them (AGENTS.md #0.1). The registration seam is where the translation + * belongs, and this package already does exactly that for + * `field:permission-facet-link` via `withFieldCarrier` (objectui#3307). + * + * ⇒ this adapter accepts the DECLARED surface and nothing else. It does not + * take a `section` prop: a node carrying one is refused by the platform + * validator, so honouring it here would create the second de-facto contract + * #0.1 exists to prevent. + */ + +import * as React from 'react'; +import type { DetailViewSection } from '@object-ui/types'; +import { DetailSection, type DetailSectionProps } from './DetailSection'; + +/** + * The section members an authored `detail-section` node carries as FLAT props, + * i.e. exactly the names the registration declares as `inputs`. + * + * THE single source for the fold, and load-bearing rather than descriptive: + * the component below folds by iterating THIS list, so a name removed from it + * is a name that stops reaching `DetailSection` — which is how the ablation + * for objectui#8626 reddens a per-input row. + * `detailSectionAuthoredNode-8626.test.tsx` additionally pins the list against + * the registration's own declared input names in BOTH directions, so a ninth + * input declared without a fold — the shape of the original defect — reds + * rather than arriving silently inert. + */ +export const DETAIL_SECTION_NODE_INPUTS = [ + 'title', + 'description', + 'fields', + 'collapsible', + 'defaultCollapsed', + 'columns', + 'showBorder', + 'headerColor', +] as const; + +type DetailSectionNodeInput = (typeof DETAIL_SECTION_NODE_INPUTS)[number]; + +/** + * What an author may write on a `detail-section` node: the eight declared + * inputs, flat, plus the render-context props a host supplies. + */ +export type DetailSectionNodeProps = Omit & + Partial>; + +const FOLDED = new Set(DETAIL_SECTION_NODE_INPUTS); + +export const DetailSectionNode: React.FC = (props) => { + /** + * The fold, driven by `DETAIL_SECTION_NODE_INPUTS` rather than by a second + * hand-written destructure — a copy of the list is a copy that can drift out + * of the declaration, which is the defect this file exists to close. + * + * `fields` is `required: true` on the registration, so an absent one is + * already an ERROR from `validateTree`; it is NOT defaulted to `[]` here — a + * lenient default would make the required declaration untrue in the other + * direction. + * + * A key the author omitted stays omitted, and an authored `undefined` stays + * `undefined`: every read site in `DetailSection` tests the VALUE + * (`section.title &&`, `section.showBorder === false`, + * `section.defaultCollapsed ?? false`), never key presence, so the two are + * the same section. + * + * Rebuilt per render on purpose — no memo. `DetailSection` keys its own + * memos off section MEMBERS (`[section.fields, …]`), never off the object, + * so a stable identity would buy nothing and a stale one would cost + * correctness. + */ + const section: Record = {}; + const hostProps: Record = {}; + for (const [key, value] of Object.entries(props)) { + (FOLDED.has(key) ? section : hostProps)[key] = value; + } + + return ( + )} + section={section as unknown as DetailViewSection} + /> + ); +}; + +DetailSectionNode.displayName = 'DetailSectionNode(DetailSection)'; diff --git a/packages/plugin-detail/src/__tests__/detailSectionAuthoredNode-8626.test.tsx b/packages/plugin-detail/src/__tests__/detailSectionAuthoredNode-8626.test.tsx new file mode 100644 index 0000000000..3f3962ac54 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/detailSectionAuthoredNode-8626.test.tsx @@ -0,0 +1,220 @@ +/** + * 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. + */ + +/** + * objectui#8626 — an authored `detail-section` node RENDERS the eight inputs + * its registration declares. + * + * ## What was broken, measured rather than read + * + * The registration declares eight FLAT inputs; `DetailSection` declares a + * single `section` OBJECT prop and reads `section.*` only. `SchemaRenderer` + * spreads a node's non-metadata keys as React props, so `section` arrived + * `undefined`. + * + * Rendering an authored node through the real `SchemaRenderer` and the real + * registry on `b775500af` produced, verbatim: + * + * Component "detail-section" failed to render + * Cannot read properties of undefined (reading 'defaultCollapsed') + * + * i.e. `SchemaErrorBoundary`'s banner in place of the block — worse than + * inert, and the half the card flagged as "a reading and not an execution". + * `errorBannerAbsent()` below is that exact face, asserted absent. + * + * ## Why each row is a RENDERING verdict + * + * Every declared input is measured by a consequence in the DOM — the text an + * author sees, the column track the grid gets, the tint class the header + * carries. Deliberately NOT "the prop was passed" and NOT "the registration + * declares eight inputs": both pass on a component that reads none of them, + * which is precisely the defect. Each class-shaped row carries its own + * negative (`grid-cols-1` without `md:grid-cols-2`, `bg-muted` without + * `bg-accent`) so it is a verdict and not a substring that was always there. + * + * ## The two guards beside the rendering rows + * + * - FOLD PARITY. The fold set and the registration's declared input names are + * compared in BOTH directions. A ninth input declared without a fold is the + * original defect in miniature — inert, silent — and reds here instead. + * - THE DECLARED SURFACE DID NOT MOVE. The flat eight still draw ZERO + * diagnostics from a manifest built the way `page.tsx` builds the JSX-page + * compiler's, and the nested `section` shape is still REFUSED by it. That is + * the control on the repair NOT taken: this card folds at the seam, and does + * not re-declare the surface authors already write. + */ + +import React from 'react'; +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer } from '@object-ui/react'; +import { manifestFromConfigs, validateTree } from '@object-ui/sdui-parser'; +import { DETAIL_SECTION_NODE_INPUTS } from '../DetailSectionNode'; + +// Module scope, not a hook (AGENTS.md 测试纪律): importing the package index +// executes its registration side-effects, so the entry under test is the very +// one production resolves. +import '../index'; + +const TAG = 'detail-section'; +const NAMESPACE = 'plugin-detail'; + +/** The node an author writes, carrying all eight declared inputs, flat. */ +const authoredNode = (overrides: Record = {}) => + ({ + type: TAG, + title: 'Billing Address', + description: 'Where invoices go', + collapsible: true, + defaultCollapsed: false, + columns: 2, + showBorder: true, + headerColor: 'muted', + fields: [ + { name: 'street', label: 'Street' }, + { name: 'city', label: 'City' }, + ], + ...overrides, + }) as never; + +const RECORD = { street: '1 Market St', city: 'San Francisco' }; + +const renderAuthored = (overrides: Record = {}) => + render(); + +/** + * `SchemaErrorBoundary`'s face, asserted absent. Its copy is + * `Component "" failed to render`, so a future refactor that reintroduces + * the throw lands here rather than on a subtler row. + */ +const errorBannerAbsent = () => { + expect(screen.queryByRole('alert')).toBeNull(); + expect(screen.queryByText(/failed to render/i)).toBeNull(); +}; + +afterEach(() => { + cleanup(); +}); + +describe('objectui#8626 — an authored detail-section node renders its declared inputs', () => { + it('renders its authored TITLE, DESCRIPTION and FIELDS instead of an error banner', () => { + const { container } = renderAuthored(); + + errorBannerAbsent(); + expect(screen.getByText('Billing Address')).toBeInTheDocument(); + expect(screen.getByText('Where invoices go')).toBeInTheDocument(); + // `fields` — both labels AND both values off the bound record, so the + // array reached the component as a section rather than as a lost prop. + expect(screen.getByText('Street')).toBeInTheDocument(); + expect(screen.getByText('City')).toBeInTheDocument(); + expect(screen.getByText('1 Market St')).toBeInTheDocument(); + expect(screen.getByText('San Francisco')).toBeInTheDocument(); + // Non-vacuity: the block rendered real chrome, not an empty container. + expect(container.innerHTML.length).toBeGreaterThan(500); + }); + + it('honours `columns` as the grid track count', () => { + const { container } = renderAuthored({ columns: 2 }); + const grid = container.querySelector('.grid'); + expect(grid).not.toBeNull(); + expect(grid!.className).toContain('md:grid-cols-2'); + // Control: a one-column authoring of the SAME node does not carry it. + cleanup(); + const single = renderAuthored({ columns: 1 }).container.querySelector('.grid'); + expect(single).not.toBeNull(); + expect(single!.className).not.toContain('md:grid-cols-2'); + }); + + it('honours `headerColor` as the header tint class', () => { + const { container } = renderAuthored({ headerColor: 'muted' }); + const header = container.querySelector('.bg-muted'); + expect(header).not.toBeNull(); + // Control: the tint is the authored one, not any tint the chrome happens + // to carry. + expect(container.querySelector('.bg-accent')).toBeNull(); + cleanup(); + const accent = renderAuthored({ headerColor: 'accent' }).container; + expect(accent.querySelector('.bg-accent')).not.toBeNull(); + }); + + /** + * ⚠️ Authored `collapsible: false` ON PURPOSE, and the reason is a defect + * this card did NOT fix: `DetailSection`'s collapsible branch renders a bare + * `` and never reads `section.showBorder`, so the key is honoured only + * on the non-collapsible branch. That is a separate, pre-existing bug inside + * `DetailSection` — the component this card deliberately leaves + * byte-identical — filed rather than repaired here. Pinning `showBorder` + * against the branch that DOES read it keeps this row a reading about the + * fold, not about that bug. + */ + it('honours `showBorder: false` by dropping the card border', () => { + const bordered = renderAuthored({ collapsible: false, showBorder: true }).container; + expect(bordered.querySelector('.border-none')).toBeNull(); + cleanup(); + const borderless = renderAuthored({ collapsible: false, showBorder: false }).container; + expect(borderless.querySelector('.border-none')).not.toBeNull(); + }); + + it('honours `collapsible` + `defaultCollapsed` as the initial disclosure state', () => { + const open = renderAuthored({ collapsible: true, defaultCollapsed: false }).container; + expect(open.querySelector('[aria-expanded="true"]')).not.toBeNull(); + cleanup(); + const collapsed = renderAuthored({ collapsible: true, defaultCollapsed: true }).container; + expect(collapsed.querySelector('[aria-expanded="false"]')).not.toBeNull(); + // The value rows are not on screen while collapsed — the read that used to + // THROW is now the one deciding this. + expect(screen.queryByText('1 Market St')).toBeNull(); + }); + + /** + * DRIFT GUARD, not the pin. The rows above are the pin; this one keeps a + * ninth declared input from arriving inert the way all eight once did. + */ + it('folds exactly the inputs the registration declares, in both directions', () => { + const declared = ( + (ComponentRegistry.getConfig(TAG, NAMESPACE) as unknown as { + inputs?: Array<{ name: string }>; + })?.inputs ?? [] + ).map((i) => i.name); + // Non-vacuity: an empty read (wrong tag/namespace) must fail here. + expect(declared.length).toBeGreaterThan(0); + expect([...declared].sort()).toEqual([...DETAIL_SECTION_NODE_INPUTS].sort()); + }); + + /** + * CONTROL on the repair NOT taken. Built the way + * `packages/components/src/renderers/layout/page.tsx` builds the JSX-page + * compiler's manifest — from the live registry — so these are the verdicts a + * real author gets. + */ + it('leaves the published authoring surface where authors already write it', () => { + const manifest = manifestFromConfigs( + ComponentRegistry.getKnownTypes().map((type) => { + const meta = ComponentRegistry.getMeta(type); + return { + type, + namespace: meta?.namespace, + isContainer: meta?.isContainer, + inputs: meta?.inputs, + }; + }) as unknown as Parameters[0], + ); + + // The flat eight: clean. + expect(validateTree(authoredNode(), manifest).diagnostics).toEqual([]); + + // The nested shape: still refused, so this repair did not quietly open a + // second dialect for the same block. + const nested = validateTree( + { type: TAG, section: { title: 'x', fields: [{ name: 'street' }] } } as never, + manifest, + ).diagnostics; + expect(nested.map((d) => d.code).sort()).toEqual(['missing-required-prop', 'unknown-prop']); + }); +}); diff --git a/packages/plugin-detail/src/index.tsx b/packages/plugin-detail/src/index.tsx index e7da42d00f..00667a4a67 100644 --- a/packages/plugin-detail/src/index.tsx +++ b/packages/plugin-detail/src/index.tsx @@ -16,6 +16,7 @@ import { import { withFieldCarrier } from '@object-ui/fields'; import { DetailView } from './DetailView'; import { DetailSection } from './DetailSection'; +import { DetailSectionNode } from './DetailSectionNode'; import { headerColorVocabulary } from './headerColor'; import { DetailTabs } from './DetailTabs'; import { RelatedList } from './RelatedList'; @@ -319,8 +320,21 @@ ComponentRegistry.register('detail-view', DetailViewRenderer, { } }); -// Register DetailSection component -ComponentRegistry.register('detail-section', DetailSection, { +// Register DetailSection component. +// +// ⚠️ Against `DetailSectionNode`, NOT `DetailSection` — and that is what makes +// the `inputs` below true (objectui#8626). `SchemaRenderer` spreads a node's +// non-metadata keys as React props, so an authored node arrives as `title` / +// `fields` / … while `DetailSection` reads a single `section` OBJECT prop. Bound +// directly, `section` arrived `undefined` and the very first +// `section.defaultCollapsed` read THREW — measured end to end, the author's page +// showed `SchemaErrorBoundary`'s orange "failed to render" banner in place of the +// block. `DetailSectionNode` folds the eight declared inputs into the `section` +// object the component reads; see that file for why the fold sits at this seam +// rather than in `DetailSection` (which every in-repo caller uses directly), and +// why re-declaring these eight as a nested `section` input was the repair NOT +// taken. +ComponentRegistry.register('detail-section', DetailSectionNode, { namespace: 'plugin-detail', label: 'Detail Section', category: 'Detail Components',