diff --git a/.changeset/page-header-inputs-declare-subtitle.md b/.changeset/page-header-inputs-declare-subtitle.md new file mode 100644 index 000000000..82c5321d1 --- /dev/null +++ b/.changeset/page-header-inputs-declare-subtitle.md @@ -0,0 +1,39 @@ +--- +"@object-ui/layout": patch +--- + +The legacy `page-header` alias stops advertising `description` as an authorable +key (objectui#3226). + +FROM: `registerLayout()` declared `inputs: [title, description]`. TO: +`inputs: [title, subtitle]` — the key `@objectstack/spec/ui`'s `PageHeaderProps` +declares, and the one the canonical `page:header` renderer in +`@object-ui/components` already declares. + +`inputs` is a DECLARATION surface, not documentation: the designer builds its +property palette from it, and the framework's `check:react-declaration-parity` +diffs it against the spec schemas. Declaring `description` therefore did not +merely tolerate a legacy spelling — it published a second dialect for the one +concept the protocol calls `subtitle`, and told authors (an AI author most +readily, since the registry is what it reads to learn the shape) that the +non-spec key was legal. Metadata that took the offer renders a subtitle under +`page-header` and silently loses it under `page:header`: same JSON, two results, +which is the outcome a single contract exists to prevent. + +No runtime behaviour changes. `PageHeader` still reads `subtitle ?? description`, +deliberately: this alias exists for out-of-repo consumer schemas, so "no in-repo +author writes `description`" (verified — zero hits) is not evidence that nobody +does, and dropping the read today would delete an external page's second line +while its title kept rendering, the least reportable failure mode there is. That +read is retired together with an ADR-0087 D2 conversion entry +(`page-header-subtitle-alias`, `description` → `subtitle` rewritten at load +time), which lives in the framework repo and is tracked separately. Narrowing the +declaration did not need to wait on it and breaks no consumer; leaving the +declaration wrong in the meantime keeps minting the metadata the conversion would +then have to absorb. + +New tests pin both halves so neither can drift back: the registration may not +declare `description`, must declare `subtitle`, and — checked against the spec's +own shape rather than a hand-written allowlist — may declare nothing +`@objectstack/spec` does not; while the runtime fallback is pinned as a sequencing +guard, to be deleted in the same change that lands the conversion entry. diff --git a/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx b/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx new file mode 100644 index 000000000..8402d0d3e --- /dev/null +++ b/packages/layout/src/__tests__/page-header-authorable-keys.test.tsx @@ -0,0 +1,110 @@ +/** + * 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 `page-header` alias declares only keys `@objectstack/spec` declares + * (objectui#3226). + * + * The legacy kebab key `page-header` and the canonical protocol key + * `page:header` (in `@object-ui/components`) render the same concept, but this + * one used to DECLARE a different authorable key for the secondary line: + * `description`, where the spec's `PageHeaderProps` — and therefore + * `page:header` — declares `subtitle`. That is not a tolerated legacy spelling, + * it is a second dialect published on the declaration surface: `inputs` is what + * the designer offers as fields and what the framework's + * `check:react-declaration-parity` diffs against the spec schemas, so an author + * (especially an AI one) was being TOLD `description` was legal. Metadata that + * took the offer renders a subtitle under `page-header` and silently loses it + * under `page:header` — the same JSON, two results, which is the failure mode + * one contract exists to prevent. + * + * The cross-check below is deliberately derived from the spec's own shape + * rather than a hand-written allowlist: a future input added here that the spec + * does not declare fails for the same reason `description` did, without anyone + * having to remember this issue. + * + * SEQUENCING — read before "finishing the job". `PageHeader.tsx` still READS + * `subtitle ?? description` at runtime, and the last test in this file pins + * that on purpose. The alias exists precisely for out-of-repo consumer schemas, + * so "no in-repo author writes `description`" (true, verified) says nothing + * about whether anyone does; dropping the read today would delete an external + * page's second line while its title kept rendering — the least reportable + * failure there is. The read goes away together with the ADR-0087 D2 conversion + * entry `page-header-subtitle-alias` (`description` → `subtitle` rewritten at + * load time), which lives in the framework repo. Narrowing the DECLARATION did + * not need to wait on it and changes no runtime behaviour; deleting the READ + * does. When that conversion lands, delete the fallback AND the last test here + * in one change. + */ + +import { describe, it, expect, beforeAll } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { PageHeaderProps as SpecPageHeaderProps } from '@objectstack/spec/ui'; + +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 config = ComponentRegistry.getConfig(type, namespace); + if (!config) throw new Error(`"${namespace ? `${namespace}:${type}` : type}" is not registered`); + return (config.inputs ?? []).map((input) => input.name); +}; + +beforeAll(() => { + registerLayout(); +}); + +describe('the `page-header` registration declares the spec key, not a dialect', () => { + it('is registered under the bare key and its namespace', () => { + expect(ComponentRegistry.getConfig('page-header')).toBeTruthy(); + expect(ComponentRegistry.getConfig('page-header', 'layout')).toBeTruthy(); + }); + + // The one assertion this issue is about: whatever else changes, the + // declaration surface must never advertise `description` again. + it.each([ + ['page-header', undefined], + ['page-header', 'layout'], + ])('does not advertise `description` on %s (namespace: %s)', (type, namespace) => { + expect(declaredInputNames(type, namespace)).not.toContain('description'); + }); + + it('declares `subtitle` — the spec key for the secondary line', () => { + expect(declaredInputNames('page-header')).toContain('subtitle'); + expect(specKeys.has('subtitle')).toBe(true); + // …and the spec has no `description` at all, which is the whole reason the + // old declaration was wrong rather than merely redundant. + expect(specKeys.has('description')).toBe(false); + }); + + it('declares nothing `@objectstack/spec` does not', () => { + const offSpec = declaredInputNames('page-header').filter((name) => !specKeys.has(name)); + expect(offSpec).toEqual([]); + }); +}); + +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 + // considered and rejected: external schemas authored with `description` would + // lose their subtitle silently. Delete this test in the same change that + // deletes the fallback, once the conversion rewrites the key upstream. + it('still renders a legacy `description` as the secondary line', () => { + render(); + expect(screen.getByText('View and edit customer information')).toBeTruthy(); + }); + + it('lets the spec key win when both are present', () => { + render(); + expect(screen.getByText('From the spec')).toBeTruthy(); + expect(screen.queryByText('From the alias')).toBeNull(); + }); +}); diff --git a/packages/layout/src/index.ts b/packages/layout/src/index.ts index deb747992..09b9e19dd 100644 --- a/packages/layout/src/index.ts +++ b/packages/layout/src/index.ts @@ -27,13 +27,33 @@ export function registerLayout() { // namespace. We intentionally do NOT re-register `page:header` here — // doing so would (depending on package load order) clobber the // record-aware renderer in components with this thinner one. + // + // `inputs` declares the AUTHORABLE surface, and it must name the same keys + // the spec does — the designer and the framework's + // `check:react-declaration-parity` read this list and treat everything in it + // as a legal input (objectui#3226). `@objectstack/spec/ui`'s + // `PageHeaderProps` declares `subtitle`; it has no `description`. This list + // used to declare `description`, so the alias did not merely tolerate a + // legacy spelling — it ADVERTISED a second dialect for the one concept + // `page:header` calls `subtitle`, and metadata authored against it renders + // a subtitle here and nothing at all under the canonical key. + // + // The runtime `subtitle ?? description` fallback in `PageHeader.tsx` stays + // for now ON PURPOSE: this alias exists for out-of-repo consumer schemas, so + // "no in-repo author writes `description`" is not evidence that nobody does, + // and dropping the read would silently delete their second line. That read + // is retired together with an ADR-0087 D2 conversion entry + // (`page-header-subtitle-alias`, `description` → `subtitle` at load time), + // 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. ComponentRegistry.register('page-header', PageHeader, { namespace: 'layout', label: 'Page Header', category: 'Layout', inputs: [ - { name: 'title', type: 'string' }, - { name: 'description', type: 'string' } + { name: 'title', type: 'string', label: 'Title' }, + { name: 'subtitle', type: 'string', label: 'Subtitle' } ] });