From ed5c81a482631d3bb2df0822e57160c9b510c911 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 01:49:24 +0000 Subject: [PATCH 1/2] =?UTF-8?q?refactor(spec)!:=20finish=20#5775's=20SDUI?= =?UTF-8?q?=20props=20count=20=E2=80=94=204=20declared,=20page:tabs.type?= =?UTF-8?q?=20=E2=86=92=20tabStyle=20(#6776)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5775 recorded that "the rest of the keys the renderers honour are declared". Re-counting against objectui origin/main (b3439f4) found five more: for each, objectui's published manifest called the key legal, validateComponentProps (#5068) called it undeclared, and the renderer honoured it anyway — #5435's shape with the wrong half on the spec side. Declared (no behaviour change, the contract catching up): - page:header.recordChrome / .showStar / .showCopyId - page:accordion.variant Renamed (tombstone + ADR-0087 D2 conversion + RETIRED_KEYS_BY_MAJOR[17]): - page:tabs.type → tabStyle. A props key named `type` collides with the page component's own dispatch key: SchemaRenderer refuses to hoist properties.type, sdui-parser's BASE_PROPS skips it before any validation, and in a flat/JSX carrier `type` is the tag name. Unauthorable in every carrier but the nested properties object, and unvalidated even there. Route A per the tier-3 ruling; all three objectui premises re-verified at origin/main before implementing. Also: mapPageComponents now walks pages[].slots., not only regions[].components[]. PageSchema.slots is a closed map of seven declared PageComponent slots and packages/lint's walkPageComponents has always visited both. page:tabs IS one of those slots and all four in-repo sites are slots.tabs, so region-only reach would have made the tombstone's `os migrate meta` prescription a false promise. Coupled factual correction (comment-only) in packages/lint: the falsified "rest of the keys are declared" sentence in authoring-rules.ts, and this card added to validateComponentProps' error-upgrade inventory alongside #5728. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- .../component-props-header-tabs-accordion.md | 81 ++++++++++ content/docs/references/ui/component.mdx | 7 +- docs/protocol-upgrade-guide.md | 3 + .../src/ui/pages/project-detail.page.ts | 5 +- packages/lint/src/authoring-rules.ts | 12 +- .../lint/src/validate-component-props.test.ts | 42 +++++ packages/lint/src/validate-component-props.ts | 23 ++- .../src/pages/sys-organization.page.ts | 5 +- .../src/pages/sys-position.page.ts | 3 +- .../src/pages/sys-user.page.ts | 3 +- packages/spec/authorable-defaults/ui.json | 6 +- packages/spec/authorable-surface/ui.json | 7 +- packages/spec/spec-changes.json | 12 ++ .../spec/src/conversions/conversions.test.ts | 145 +++++++++++++++++- packages/spec/src/conversions/registry.ts | 136 ++++++++++++++++ packages/spec/src/conversions/walk.ts | 108 +++++++++---- packages/spec/src/migrations/registry.ts | 23 +++ packages/spec/src/ui/component.test.ts | 101 +++++++++++- packages/spec/src/ui/component.zod.ts | 89 ++++++++++- 19 files changed, 761 insertions(+), 50 deletions(-) create mode 100644 .changeset/component-props-header-tabs-accordion.md diff --git a/.changeset/component-props-header-tabs-accordion.md b/.changeset/component-props-header-tabs-accordion.md new file mode 100644 index 0000000000..b08c7c4c02 --- /dev/null +++ b/.changeset/component-props-header-tabs-accordion.md @@ -0,0 +1,81 @@ +--- +"@objectstack/spec": major +--- + +refactor(spec)!: finish #5775's count — 4 SDUI component props declared, `page:tabs.type` renamed to `tabStyle` (#6776) + +#5775 reconciled `ComponentPropsMap` with the renderers that serve it and then +recorded that "the rest of the keys the renderers honour are declared". That +sentence did not hold. Re-counting against objectui `origin/main` found **five** +more author-facing props that objectui's renderers read and this schema did not +declare — so for each of them three platform authorities disagreed at once: +objectui's published manifest and generated `sdui-intrinsics.d.ts` told an author +(very often an AI author, ADR-0033) the key was **legal**, `validateComponentProps` +(#5068) reported it **undeclared**, and the renderer **honoured it anyway**. That +is #5435's shape — a platform authority pointing at a key its own gate rejects — +with the wrong half on the spec side this time. + +Four are plain declarations. No behaviour changes; the contract catches up with +what has always shipped: + +| Key | Type | Default | What it does | +| :-- | :-- | :-- | :-- | +| `page:header.recordChrome` | `boolean` | `true` | `false` drops the record chip and renders the bare heading — what a dashboard or landing page wants, since there is no record to describe | +| `page:header.showStar` | `boolean` | `true` | the follow (favourite) star beside the record title | +| `page:header.showCopyId` | `boolean` | `true` | the copy-record-id button beside the record title | +| `page:accordion.variant` | `'flush' \| 'card'` | `'flush'` | `flush` draws the divider under each panel; `card` leaves the border to each panel's own content | + +The fifth is a rename, and the only one whose defect is structural rather than an +oversight. + +## BREAKING: `page:tabs` property `type` → `tabStyle` + +The concept — the tab strip's visual style, `line` / `card` / `pill` — was +declared all along, under a spelling **no author can write in most carriers**: a +props key named `type` collides with the page component's own dispatch key. +Three independent consequences, each measured on objectui `origin/main`: + +- `SchemaRenderer.tsx:253,264` hoists `properties` onto the node but skips `type` + and `id` deliberately, or the inner value would shadow which renderer to + dispatch to. Its comment names this exact case. +- `sdui-parser`'s `BASE_PROPS` (`validate.ts:20-30`) contains `type`, and + `validate.ts:68` skips every base prop before the unknown/typed checks — so a + manifest input by that name is **never validated**. +- In the flat and JSX carriers a node reads + `{ type: 'page:tabs', items: [...], tabStyle: 'card' }`: `type` is the tag + name, and the declared spelling has nowhere left to go. + +`tabStyle` is what objectui's registry publishes as the designer input and what +`containers.tsx:381` reads in every carrier. Converging on the spelling that +works rather than the one that declares well is #5775's `displayField` → +`labelField` again, and one spelling rather than two is Prime Directive #12 — +declaring `tabStyle` as an alias of `type` was considered and refused, because +the dialect that would survive is the one that silently fails to validate. + +FROM → TO: + +- `pages[].regions[].components[]` and `pages[].slots.` where + `type === 'page:tabs'`: `properties.type` → `properties.tabStyle`. The value + (`line` | `card` | `pill`) is unchanged. + +**The one-line fix:** rename the key. `os migrate meta --from 16` rewrites it +automatically; the ADR-0087 D2 conversion is +`page-tabs-type-to-tab-style` (`retiredFromLoadPath` — the tombstone owns the +refusal, so a 17 loader does not accept the old spelling), and the tombstone +carries the same prescription at `tsc` and at parse time for anyone jumping +several majors at once. + +## Also in this change + +`mapPageComponents` (the conversion layer's page walk) now visits +`pages[].slots.` as well as `pages[].regions[].components[]`. Its comment +used to call region level "the whole surface", on the reasoning that everything +else lives inside a free-form `properties` bag — but `PageSchema.slots` is a +closed map of seven named slots, each declared +`z.union([PageComponentSchema, z.array(PageComponentSchema)])`, as typed as any +region component, and `packages/lint`'s `walkPageComponents` has always visited +both. #6776 is where the gap cost something: `page:tabs` **is** one of those +slots, all four in-repo authoring sites are `slots.tabs`, and a region-only +rewrite would have left `os migrate meta` unable to touch the only shape the key +is written in — while the tombstone promised it could. Every other page-component +conversion gains the same reach, in the direction it already declares. diff --git a/content/docs/references/ui/component.mdx b/content/docs/references/ui/component.mdx index 116bc70c3b..ed24735643 100644 --- a/content/docs/references/ui/component.mdx +++ b/content/docs/references/ui/component.mdx @@ -201,6 +201,7 @@ const result = AIChatWindowProps.parse(data); | :--- | :--- | :--- | :--- | | **items** | `{ label: string \| Record; icon?: string; collapsed: boolean; children: any[] }[]` | ✅ | | | **allowMultiple** | `boolean` | ✅ | Allow multiple panels to be expanded simultaneously | +| **variant** | `Enum<'flush' \| 'card'>` | ✅ | Panel framing: 'flush' draws a divider under each panel; 'card' leaves the border to each panel's own content | | **aria** | `{ ariaLabel?: string \| Record; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes | @@ -245,6 +246,9 @@ const result = AIChatWindowProps.parse(data); | **icon** | `string` | optional | Icon name | | **breadcrumb** | `boolean` | ✅ | Show breadcrumb | | **actions** | `string[]` | optional | Action IDs to show in header | +| **recordChrome** | `boolean` | ✅ | Render the record chrome — the title as a record chip with its follow star and copy-id button. Set false on a non-record page (dashboard, landing) to fall back to the bare heading layout. | +| **showStar** | `boolean` | ✅ | Show the follow (favourite) star beside the record title. Part of the record chrome — no effect when `recordChrome` is false. | +| **showCopyId** | `boolean` | ✅ | Show the copy-record-id button beside the record title. Part of the record chrome — no effect when `recordChrome` is false. | | **aria** | `{ ariaLabel?: string \| Record; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes | @@ -256,7 +260,8 @@ const result = AIChatWindowProps.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **type** | `Enum<'line' \| 'card' \| 'pill'>` | optional | | +| **tabStyle** | `Enum<'line' \| 'card' \| 'pill'>` | optional | Tab-strip visual style: 'line' underlines the active tab, 'card' frames each tab, 'pill' renders rounded pills | +| **type** | `never` | optional | [REMOVED] `page:tabs` property `type` was removed in @objectstack/spec 17.0.0 (#6776, ADR-0087 D2) — a props key named `type` collides with the page component's own dispatch key, so it is unauthorable in the flat and JSX carriers and was never validated in them. Rename the key to `tabStyle`; the value (`line` \| `card` \| `pill`) is unchanged. Run `os migrate meta --from 16` to rewrite it automatically. | | **position** | `Enum<'top' \| 'left'>` | optional | | | **items** | `{ label: string \| Record; icon?: string; visibleWhen?: string \| object; value?: string; … }[]` | ✅ | | | **aria** | `{ ariaLabel?: string \| Record; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 46e82e1b37..e4faa711d8 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -218,6 +218,8 @@ The last of the #4001 enforce-or-remove batch lands on two more `ui/` files (#50 Last, it reconciles the SDUI component-props surface with the renderers that serve it (#5775). #5068 wired the first parse `ComponentPropsMap` ever had, and the corpus it landed on diverged in BOTH directions: keys objectui honours that the schema never declared, and keys the schema declared — one of them REQUIRED — that no renderer reads. The maintainer ruled direction A (2026-08-06), the #5611 rule again: the delivered and authorized shape is the contract. So the honoured keys are declared (`element:record_picker` `labelField`/`valueField`/`label`/`emptyText`, `record:path` `stages[].terminal`, `page:tabs` `items[].value`/`items[].count`, `page:card` `children`, and `children` on `page:section`/`page:footer`/`page:sidebar`, which were declared `EmptyProps` while their renderers rendered a child list), and four keys retire. Two are synonym renames: `element:record_picker.displayField` → `labelField` (the required key no renderer read, while `labelField ?? 'name'` is what actually renders the row — so an author who followed the schema got a picker listing `name` with no diagnostic, the ADR-0078 shape), and `page:card.body` → `children` (one composition key across every container; the card renderer already reads both, and the showcase authors `children`). Two are enforce-or-remove deletions: `element:record_picker.searchFields` and `.multiple` — the control is a shadcn single-select with no search input, binding ONE record id into a page variable, so `searchFields` narrowed nothing and `multiple: true` selected nothing extra while reporting success. Either returns the day the capability is implemented (#5021 / #4988). Not in scope, and deliberately: `page:card.visible` is a component-level visibility predicate written into `properties` and hoisted by the renderer — a page to rewrite onto the ADR-0089 `visibleWhen`, not a key to declare. +That count turned out to be incomplete, and #6776 finishes it: five more keys the renderers read were still undeclared. Four are plain additions with no behaviour change (`page:header` `recordChrome`/`showStar`/`showCopyId`, which select between the record-chip header and the bare heading a dashboard wants, and `page:accordion.variant`, which decides whether the accordion draws its own dividers or leaves the border to each panel). The fifth is a rename, and the only one in the family whose defect is structural rather than an oversight: the tab strip's visual style was declared as `page:tabs.type`, which collides with the page component's OWN dispatch key. objectui's `SchemaRenderer` refuses to hoist `properties.type` for exactly that reason, `sdui-parser`'s `BASE_PROPS` contains `type` and skips it before any validation runs, and in a flat or JSX carrier the node reads `{ type: 'page:tabs', … }` so the name is already taken. The key was therefore unauthorable in every carrier but the nested `properties` object, and unvalidated even there. It becomes `tabStyle` — the spelling objectui publishes and the renderer already reads first in the flat carriers — which is `displayField` → `labelField` again: converge on the spelling that works, not the one that declares well, and keep one spelling rather than two (Prime Directive #12). + Finally it narrows the aggregation vocabulary: `array_agg` and `string_agg` leave `AggregationFunction` (#6188, ADR-0049). The enum declared eight functions and the SQL family compiles five — `SqlDriver.mapAggregateFunc` and the Turso `RemoteTransport.aggregate` each lower `count`/`sum`/`avg`/`min`/`max` and route the rest to one refusal — so three were declared-but-unenforced against the backends this platform targets. What makes these two worse than an ordinary inert declaration is that another package had to carry a denylist for them: `service-analytics` subtracted `array_agg` and `string_agg` by name in `UNSUPPORTED_AGGREGATES`, because without that subtraction they reached the Cube strategy's `default` and returned `COUNT(*)` — a row count in place of the requested value, with no error and no log. The maintainer SPLIT the three rather than retiring them as a block (2026-08-07), and the split is the point: `count_distinct` STAYS and takes the enforce leg — one portable lowering (`COUNT(DISTINCT x)`), a dashboard staple, already lowered by `service-analytics` — with its SQL implementation following on its own card, so that declaration leads its implementation by decision rather than by drift. These two take the remove leg: display conveniences with no measured pull, and `string_agg` never had one shape to lower to (the delimiter is a second argument in PostgreSQL, a `SEPARATOR` clause in MySQL, a differently named function in SQL Server). This is an enum VALUE, not a key, so — as with `crypto.hash` above — there is no `retiredKey()` tombstone: the enum error map carries the prescription, keyed on the received value so only the two spellings that used to be legal are told they "were removed". Of the two authoring surfaces only one is stored metadata: the conversion rewrites `dataset.measures[].aggregate`, dropping the measure outright (a measure with neither `aggregate` nor `derived` fails the dataset's own refinement, so stripping just the key would emit an item that cannot parse) plus any derived measure the drop strands, with a notice each. Nothing is lost: `compileDataset` refused both by name already, so such a measure never produced a number. `QueryAST.aggregations[].function` is a request surface with no stored source — one semantic TODO below. The mongodb and in-memory backends that implemented these two are inside the #5499 freeze and are untouched; their code is simply no longer reachable through a spec-valid request. One entry in this step is not a removal at all but a SECURE-DEFAULT FLIP, the shape protocol 12 last used for `api.requireAuth`: an omitted `ActionDescriptor.resumeAuthority` resolves to `'service'` instead of `'any'`, so a pausing node type that never states who may continue its pauses is refused on the generic resume route rather than open to it (#5561, ADR-0044's 2026-07-28 amendment). Nothing is removed and no metadata shape changes — the field has been optional since step one of the same issue — so tsc reports nothing and only the MEANING of silence moved. That is exactly why it needs a ledger entry: a third-party plugin author has no compile error to discover it with, and the one-line prescription (declare `resumeAuthority` on the descriptor) has to arrive before a user meets a run that will not continue. @@ -276,6 +278,7 @@ One entry in this step is not a removal at all but a SECURE-DEFAULT FLIP, the sh | `record-picker-inert-keys-removed` | `page.component.element:record_picker.searchFields / page.component.element:record_picker.multiple` | record-picker component props 'searchFields'/'multiple' removed (#5775 — the control is a plain single-select with no search box; neither key had a reader) | retired — `migrate meta` only | | `page-card-body-to-children` | `page.component.page:card.body` | page:card component prop 'body' → 'children' (#5775 — one composition key across every container; the card renderer already reads both) | retired — `migrate meta` only | | `inline-action-api-params-to-body-extra` | `page.component.element:button.action.params` | inline type:'api' action prop 'params' (object form) → 'bodyExtra' (#5777 — the payload gets its own key; `params` stays the ActionParam[] definition array) | live — protocol 17 loader accepts the old shape | +| `page-tabs-type-to-tab-style` | `page.component.page:tabs.type` | page:tabs component prop 'type' → 'tabStyle' (#6776 — a props key named `type` collides with the node's dispatch key and is unauthorable in flat/JSX carriers; `tabStyle` is the spelling the renderer reads in all of them) | retired — `migrate meta` only | ### Semantic (delegated to you, with acceptance criteria) diff --git a/examples/app-showcase/src/ui/pages/project-detail.page.ts b/examples/app-showcase/src/ui/pages/project-detail.page.ts index 75994af4d3..5d1d026923 100644 --- a/examples/app-showcase/src/ui/pages/project-detail.page.ts +++ b/examples/app-showcase/src/ui/pages/project-detail.page.ts @@ -35,7 +35,10 @@ export const ProjectDetailPage = definePage({ tabs: { type: 'page:tabs', properties: { - type: 'line', + // `tabStyle`, not `type` (#6776): a props key named `type` collides + // with the component node's own dispatch key, so the old spelling + // could not be written in a flat or JSX page at all. + tabStyle: 'line', items: [ { // Explicit details sections — each section's `fields` is a diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index a3c2154945..7970b44f0c 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -561,9 +561,15 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ // to enforce declarations the platform does not keep. The error upgrade is a // separate step, once the warning-period inventory is empty. // - // #5775 has since settled its half: `displayField` is retired in favour of the - // `labelField` the renderer actually reads, and the rest of the keys the - // renderers honour are declared. #5728 and two page rewrites are what remain. + // #5775 settled the record picker's half: `displayField` is retired in favour + // of the `labelField` the renderer actually reads. Its claim that "the rest of + // the keys the renderers honour are declared" did NOT hold — #6776 found five + // more (`page:header` `recordChrome`/`showStar`/`showCopyId`, + // `page:accordion.variant`, and the tab strip's visual style, whose declared + // spelling `page:tabs.type` collided with the component node's own dispatch + // key and so was unauthorable in the flat and JSX carriers). All five are + // declared as of #6776, the last as the renamed `tabStyle`. What remains + // before the error upgrade is #5728 and two page rewrites. { name: 'validateComponentProps', tier: 'advisory', diff --git a/packages/lint/src/validate-component-props.test.ts b/packages/lint/src/validate-component-props.test.ts index 9711d60ff9..06791cf1a9 100644 --- a/packages/lint/src/validate-component-props.test.ts +++ b/packages/lint/src/validate-component-props.test.ts @@ -311,6 +311,48 @@ describe('validateComponentProps — value verdicts', () => { expect(findings).toEqual([]); }); + /** + * #6776 — the five keys #5775's count missed. Each is read by an objectui + * renderer and each was reported here as an undeclared key, so objectui's + * published manifest and this gate disagreed about the same author's page. + * This is the acceptance test for the declaration half: the shapes that used + * to warn must now be silent. + */ + it('reports nothing on the page:header / page:accordion keys the renderers honour (#6776)', () => { + const findings = validateComponentProps( + stackWith([ + // objectui `apps/console/src/preview-samples.ts:68` verbatim — the + // measured warning site, on a non-record page. + { type: 'page:header', properties: { title: 'Welcome to the CRM', recordChrome: false } }, + { type: 'page:header', properties: { title: 'Lead', showStar: false, showCopyId: false } }, + { type: 'page:accordion', properties: { items: [], variant: 'card' } }, + { type: 'page:tabs', properties: { tabStyle: 'card', items: [] } }, + ]), + ); + expect(findings).toEqual([]); + }); + + /** + * The other direction of the same card. `page:tabs.type` is now a tombstone, + * and a tombstone is REFUSED rather than reported as unknown — so the finding + * arrives as `component-props-invalid` carrying the rename prescription, not + * as `component-props-unknown-key` carrying nothing. The conversion that + * rewrites it is `retiredFromLoadPath`, so it deliberately does NOT run on + * this normalized input: an already-stored page keeps the old key and the + * author is told, which is the whole point of the tombstone. + */ + it('refuses the retired `page:tabs.type` by name, with the prescription (#6776)', () => { + const findings = validateComponentProps( + stackWith([{ type: 'page:tabs', properties: { type: 'card', items: [] } }]), + ); + expect(invalid(findings).map((f) => f.path)).toEqual([ + 'pages[0].regions[0].components[0].properties.type', + ]); + expect(invalid(findings)[0].message).toContain('tabStyle'); + // Not an unknown key — the schema still declares it, as a tombstone. + expect(unknownKeys(findings)).toEqual([]); + }); + /** * The routing that keeps this gate whole across a future `strictObject` * batch. `AriaPropsSchema` is the one CLOSED shape inside these props (#4001 diff --git a/packages/lint/src/validate-component-props.ts b/packages/lint/src/validate-component-props.ts index a271967e83..3fe1374287 100644 --- a/packages/lint/src/validate-component-props.ts +++ b/packages/lint/src/validate-component-props.ts @@ -62,11 +62,24 @@ * warning-period inventory is the acceptance baseline for the error upgrade, * and the upgrade is its own step once the inventory is empty. * - * #5775 has since settled the spec side of that inventory — the keys the - * renderers honour are declared, and the four nothing read are tombstoned with - * ADR-0087 conversions. What stands between this rule and `error` is now #5728 - * and two page rewrites (`page:card.visible` → the ADR-0089 component-level - * `visibleWhen`; #5776's tab `key` → `value`), not the props map. + * #5775 settled most of the spec side of that inventory — four keys nothing read + * are tombstoned with ADR-0087 conversions. Its stronger claim, that every key + * the renderers honour is now declared, was **incomplete**: #6776 re-counted + * against objectui `origin/main` and found five more author-facing props the + * renderers read and this map did not declare — `page:header`'s + * `recordChrome` / `showStar` / `showCopyId`, `page:accordion.variant`, and the + * tab strip's visual style, which the spec did declare but under a spelling + * (`page:tabs.type`) that collides with a page component's own dispatch key and + * is therefore unauthorable in the flat and JSX carriers. #6776 declares the + * four and renames the fifth to the `tabStyle` every carrier can express. Both + * cards are named here rather than only in a changelog because THIS comment is + * the inventory the error upgrade is measured against, and a count that reports + * itself complete while it is not is how the second half went unnoticed for a + * release. + * + * What stands between this rule and `error` is now #5728 and two page rewrites + * (`page:card.visible` → the ADR-0089 component-level `visibleWhen`; #5776's tab + * `key` → `value`), not the props map. * * ## Unregistered types are SKIPPED — a required semantic, not leniency * diff --git a/packages/platform-objects/src/pages/sys-organization.page.ts b/packages/platform-objects/src/pages/sys-organization.page.ts index c5514ff084..2f56173ff2 100644 --- a/packages/platform-objects/src/pages/sys-organization.page.ts +++ b/packages/platform-objects/src/pages/sys-organization.page.ts @@ -48,7 +48,10 @@ export const SysOrganizationDetailPage: Page = { tabs: { type: 'page:tabs', properties: { - type: 'line', + // `tabStyle`, not `type` (#6776): the node's own dispatch key is + // already `type`, so the old spelling was unauthorable in the flat and + // JSX carriers and unvalidated everywhere. + tabStyle: 'line', position: 'top', items: [ { diff --git a/packages/platform-objects/src/pages/sys-position.page.ts b/packages/platform-objects/src/pages/sys-position.page.ts index dd06659d16..0a9db276f1 100644 --- a/packages/platform-objects/src/pages/sys-position.page.ts +++ b/packages/platform-objects/src/pages/sys-position.page.ts @@ -46,7 +46,8 @@ export const SysPositionDetailPage: Page = { tabs: { type: 'page:tabs', properties: { - type: 'line', + // `tabStyle`, not `type` (#6776) — see sys-organization.page.ts. + tabStyle: 'line', position: 'top', items: [ { diff --git a/packages/platform-objects/src/pages/sys-user.page.ts b/packages/platform-objects/src/pages/sys-user.page.ts index a872be2d6a..3000612fb3 100644 --- a/packages/platform-objects/src/pages/sys-user.page.ts +++ b/packages/platform-objects/src/pages/sys-user.page.ts @@ -136,7 +136,8 @@ export const SysUserDetailPage: Page = { tabs: { type: 'page:tabs', properties: { - type: 'line', + // `tabStyle`, not `type` (#6776) — see sys-organization.page.ts. + tabStyle: 'line', position: 'top', items: [ { diff --git a/packages/spec/authorable-defaults/ui.json b/packages/spec/authorable-defaults/ui.json index a4ebc64337..4405695db3 100644 --- a/packages/spec/authorable-defaults/ui.json +++ b/packages/spec/authorable-defaults/ui.json @@ -73,11 +73,15 @@ "ui/Page:template = \"default\"", "ui/Page:type = \"record\"", "ui/PageAccordionProps:allowMultiple = false", + "ui/PageAccordionProps:variant = \"flush\"", "ui/PageCardProps:bordered = true", "ui/PageComponent:properties = {}", "ui/PageHeaderProps:breadcrumb = true", + "ui/PageHeaderProps:recordChrome = true", + "ui/PageHeaderProps:showCopyId = true", + "ui/PageHeaderProps:showStar = true", "ui/PageTabsProps:position = \"top\"", - "ui/PageTabsProps:type = \"line\"", + "ui/PageTabsProps:tabStyle = \"line\"", "ui/PageVariable:type = \"string\"", "ui/PaginationConfig:pageSize = 25", "ui/RecordActivityProps:enableMentions = true", diff --git a/packages/spec/authorable-surface/ui.json b/packages/spec/authorable-surface/ui.json index f56d598fdf..b15a0009d5 100644 --- a/packages/spec/authorable-surface/ui.json +++ b/packages/spec/authorable-surface/ui.json @@ -775,6 +775,7 @@ "ui/PageAccordionProps:allowMultiple", "ui/PageAccordionProps:aria", "ui/PageAccordionProps:items", + "ui/PageAccordionProps:variant", "ui/PageCardProps:actions", "ui/PageCardProps:aria", "ui/PageCardProps:body [RETIRED]", @@ -800,6 +801,9 @@ "ui/PageHeaderProps:aria", "ui/PageHeaderProps:breadcrumb", "ui/PageHeaderProps:icon", + "ui/PageHeaderProps:recordChrome", + "ui/PageHeaderProps:showCopyId", + "ui/PageHeaderProps:showStar", "ui/PageHeaderProps:subtitle", "ui/PageHeaderProps:title", "ui/PageNavItem:badge", @@ -821,7 +825,8 @@ "ui/PageTabsProps:aria", "ui/PageTabsProps:items", "ui/PageTabsProps:position", - "ui/PageTabsProps:type", + "ui/PageTabsProps:tabStyle", + "ui/PageTabsProps:type [RETIRED]", "ui/PageVariable:defaultValue", "ui/PageVariable:name", "ui/PageVariable:source", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 3bd80030b9..199907d370 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -367,6 +367,12 @@ "to": "inline type:'api' action prop 'params' (object form) → 'bodyExtra' (#5777 — the payload gets its own key; `params` stays the ActionParam[] definition array)", "conversionId": "inline-action-api-params-to-body-extra", "toMajor": 17 + }, + { + "surface": "page.component.page:tabs.type", + "to": "page:tabs component prop 'type' → 'tabStyle' (#6776 — a props key named `type` collides with the node's dispatch key and is unauthorable in flat/JSX carriers; `tabStyle` is the spelling the renderer reads in all of them)", + "conversionId": "page-tabs-type-to-tab-style", + "toMajor": 17 } ], "migrated": [ @@ -1208,6 +1214,12 @@ "to": "inline type:'api' action prop 'params' (object form) → 'bodyExtra' (#5777 — the payload gets its own key; `params` stays the ActionParam[] definition array)", "conversionId": "inline-action-api-params-to-body-extra", "toMajor": 17 + }, + { + "surface": "page.component.page:tabs.type", + "to": "page:tabs component prop 'type' → 'tabStyle' (#6776 — a props key named `type` collides with the node's dispatch key and is unauthorable in flat/JSX carriers; `tabStyle` is the spelling the renderer reads in all of them)", + "conversionId": "page-tabs-type-to-tab-style", + "toMajor": 17 } ], "migrated": [ diff --git a/packages/spec/src/conversions/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index 721072c6ff..a0de955157 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -6,7 +6,7 @@ import { CreateRecordConfigSchema } from '../automation/builtin-node-config.zod. import { FlowSchema } from '../automation/flow.zod.js'; import { ScriptConfigSchema } from '../automation/schemaless-node-config.zod.js'; import { normalizeStackInput } from '../shared/metadata-collection.zod.js'; -import { ElementButtonPropsSchema, PageHeaderProps } from '../ui/component.zod.js'; +import { ElementButtonPropsSchema, PageHeaderProps, PageTabsProps } from '../ui/component.zod.js'; import { PageSchema } from '../ui/page.zod.js'; import { applyConversions, collectConversionNotices } from './apply.js'; import { ALL_CONVERSIONS, CONVERSIONS_BY_MAJOR } from './registry.js'; @@ -1049,4 +1049,147 @@ describe('conversion layer (ADR-0087 D2)', () => { expect(PageSchema.safeParse(page(after)).success).toBe(true); }); }); + + /** + * `page-tabs-type-to-tab-style` (#6776). + * + * The fixture pair above pins before → after and the notice count. What needs + * its own cover here is everything the fixture cannot show: + * + * - the **discriminator** — this entry keys on the component's `type`, and + * the key it rewrites is also called `type`, so "which `type`" is the + * whole correctness question; + * - the **reach** — `page:tabs` is one of the seven named slots, and all + * four in-repo authoring sites are `slots.tabs`, so a region-only walk + * would rewrite nothing that actually exists; + * - **idempotence** and the #4923 both-keys rule; + * - the **acceptance face in both directions**, since a rename moves what + * the schema accepts as well as what it refuses. + */ + describe('page-tabs-type-to-tab-style (#6776)', () => { + const regionPage = (properties: Record) => ({ + pages: [{ name: 'sys_position_detail', regions: [{ name: 'main', components: [{ type: 'page:tabs', properties }] }] }], + }); + const slottedPage = (properties: Record) => ({ + pages: [{ name: 'sys_user_detail', regions: [], slots: { tabs: { type: 'page:tabs', properties } } }], + }); + const convert = (stack: Record) => { + const notices: ConversionNotice[] = []; + const out = applyConversions(stack, { includeRetired: true, onNotice: (n) => notices.push(n) }); + return { out, notices }; + }; + type Comp = { type: string; properties: Record }; + type Pg = { regions: { components: Comp[] }[]; slots?: { tabs: Comp | Comp[] } }; + const pageOf = (stack: Record) => (stack.pages as Pg[])[0]!; + const propsOf = (stack: Record, where: 'region' | 'slot') => { + const page = pageOf(stack); + return where === 'region' + ? page.regions[0]!.components[0]!.properties + : (page.slots!.tabs as Comp).properties; + }; + + it('rewrites `properties.type` → `tabStyle` on a region-level page:tabs', () => { + const { out, notices } = convert(regionPage({ type: 'card', items: [] })); + expect(propsOf(out, 'region')).toEqual({ tabStyle: 'card', items: [] }); + expect(notices.map((n) => n.conversionId)).toEqual(['page-tabs-type-to-tab-style']); + expect(notices[0]!.path).toBe('pages[0].regions[0].components[0].properties.tabStyle'); + }); + + it('reaches `slots.tabs` — the shape every in-repo site actually uses', () => { + // Region-only reach was the pre-#6776 behaviour of `mapPageComponents`, + // and for THIS key it would have converted nothing: `page:tabs` is a + // named slot, and all four sites in this repo are slotted record pages. + // A conversion that cannot reach the corpus makes the tombstone's + // "run `os migrate meta`" prescription a false promise. + const { out, notices } = convert(slottedPage({ type: 'pill', position: 'top', items: [] })); + expect(propsOf(out, 'slot')).toEqual({ tabStyle: 'pill', position: 'top', items: [] }); + expect(notices[0]!.path).toBe('pages[0].slots.tabs.properties.tabStyle'); + }); + + it('reaches an ARRAY-valued slot too, indexing the path', () => { + const stack = { + pages: [{ + name: 'sys_user_detail', + regions: [], + slots: { tabs: [{ type: 'page:tabs', properties: { type: 'card', items: [] } }] }, + }], + }; + const { out, notices } = convert(stack); + const slot = pageOf(out).slots!.tabs as Comp[]; + expect(slot[0]!.properties).toEqual({ tabStyle: 'card', items: [] }); + expect(notices[0]!.path).toBe('pages[0].slots.tabs[0].properties.tabStyle'); + }); + + it('never touches the node\'s OWN `type` — the dispatch key is not the prop', () => { + // The one confusion this entry has to be immune to: `component.type` is + // `'page:tabs'` and stays that way; only `properties.type` moves. + const { out } = convert(regionPage({ type: 'card', items: [] })); + expect(pageOf(out).regions[0]!.components[0]!.type).toBe('page:tabs'); + }); + + it('leaves a non-tabs component alone, including a nested `type` in its props', () => { + const stack = { + pages: [{ + name: 'p', + regions: [{ + name: 'main', + components: [{ type: 'element:button', properties: { label: 'Open', action: { type: 'url', target: '/x' } } }], + }], + }], + }; + // Identity, not just equality: nothing converted, so copy-on-write shares. + expect(applyConversions(stack, { includeRetired: true })).toBe(stack); + }); + + it('keeps BOTH when `tabStyle` already says something different (#4923 house rule)', () => { + const stack = regionPage({ tabStyle: 'pill', type: 'card', items: [] }); + const { out, notices } = convert(stack); + expect(out).toBe(stack); + expect(notices).toEqual([]); + }); + + it('drops the redundant twin when both spellings agree (#4923)', () => { + const { out, notices } = convert(regionPage({ tabStyle: 'pill', type: 'pill', items: [] })); + expect(propsOf(out, 'region')).toEqual({ tabStyle: 'pill', items: [] }); + expect(notices).toHaveLength(1); + }); + + it('is idempotent — the converted result replays to itself with no second notice', () => { + const once = applyConversions(regionPage({ type: 'card', items: [] }), { includeRetired: true }); + const notices: ConversionNotice[] = []; + const twice = applyConversions(once, { includeRetired: true, onNotice: (n) => notices.push(n) }); + expect(twice).toBe(once); + expect(notices).toEqual([]); + }); + + /** + * The acceptance face, both directions — a KEY verdict, so the criterion is + * the props schema's own judgement of the key (#5046's distinction), not a + * full-parse-green demand on a value. + * + * `PageSchema` stays green on both spellings because + * `PageComponent.properties` is an open bag that never judged the key at + * all; that gap is the defect's mechanism, and the #5068 props gate is the + * only place either verdict is visible. + */ + it('the props schema refuses `type` BY NAME and accepts `tabStyle`; PageSchema accepts both', () => { + expect(PageTabsProps.safeParse({ type: 'card', items: [] }).success).toBe(false); + // The refusal carries the prescription, not a bare "unrecognized key" — + // a `retiredKey` tombstone rather than an undeclared key, which is what + // makes the removal audible to an upgrading (often AI) author. + expect(() => PageTabsProps.parse({ type: 'card', items: [] })) + .toThrow(/`type`.*removed.*`tabStyle`/s); + expect(PageTabsProps.safeParse({ tabStyle: 'card', items: [] }).success).toBe(true); + + const page = (properties: Record) => ({ + name: 'sys_position_detail', + label: 'Position', + type: 'record' as const, + object: 'sys_position', + regions: [{ name: 'main', components: [{ type: 'page:tabs', properties }] }], + }); + expect(PageSchema.safeParse(page({ type: 'card', items: [] })).success).toBe(true); + expect(PageSchema.safeParse(page({ tabStyle: 'card', items: [] })).success).toBe(true); + }); + }); }); diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index fa4607ee94..2bc1bce250 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -5153,6 +5153,141 @@ const inlineActionApiParamsToBodyExtra: MetadataConversion = { }, }; +/** + * `page:tabs.type` → `tabStyle` (protocol 17, #6776). + * + * The concept — the tab strip's visual style, `line` | `card` | `pill` — was + * declared all along. What was wrong is the **spelling**: a props key named + * `type` sits on the one name a page component cannot spare, because the node's + * own dispatch key is `type` too. Three independently-measured consequences on + * objectui `origin/main`, none of them cosmetic: + * + * - `SchemaRenderer.tsx:253,264` hoists `properties` onto the node but skips + * `type` and `id` explicitly, or the inner value would shadow which + * renderer to dispatch to. Its comment names this very case ("tab visual + * style: 'line' | 'card' | 'pill'"). + * - `sdui-parser`'s `BASE_PROPS` (`validate.ts:20-30`) contains `'type'`, and + * `validate.ts:68` `continue`s on any base prop — so a manifest input named + * `type` is skipped before the unknown/typed checks and is never validated. + * - In the flat and JSX carriers a node reads + * `{ type: 'page:tabs', items: […], tabStyle: 'card' }` — `type` is the tag + * name, and the spec's spelling has nowhere left to go. + * + * So `tabStyle` is not drift: it is the only spelling those carriers can + * express, it is what objectui's registry publishes as the designer input, and + * `containers.tsx:381` reads BOTH (`schema?.properties?.type || schema?.tabStyle`). + * Converging on the spelling the renderer reads — rather than the one that + * declares well — is exactly {@link recordPickerDisplayFieldToLabelField}'s + * shape (#5775), and one spelling rather than two is Prime Directive #12. The + * alternative considered and refused was declaring `tabStyle` as an alias of + * `type`: that fossilizes a second dialect for one concept, and the dialect + * that would survive is the one that silently fails to validate. + * + * A rename rather than a deletion because the two keys are synonyms down to the + * value vocabulary — the same three-value enum, no re-mapping. Precedence is + * {@link renameKey}'s house rule (#4923): a redundant twin is dropped, a + * DISAGREEING pair is left for the author to reconcile rather than the loader + * picking a look. + * + * Region level is the reach, as for {@link pageCardBodyToChildren}: + * `PageComponentSchema` declares no children key, so a `page:tabs` nested inside + * another component's free-form `properties` is not typed page-component shape. + * The tombstone covers the rest — `tsc` at the authoring site and the parse at + * load, both carrying the prescription whether or not the walk reaches there. + * + * `retiredFromLoadPath: true`: no alias window, deliberately. The tombstone owns + * the refusal; this entry exists so `spec-changes.json`, the upgrade guide and + * `os migrate meta` still carry the rewrite. + */ +const pageTabsTypeToTabStyle: MetadataConversion = { + id: 'page-tabs-type-to-tab-style', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'page.component.page:tabs.type', + summary: + "page:tabs component prop 'type' → 'tabStyle' (#6776 — a props key named `type` collides with the node's dispatch key and is unauthorable in flat/JSX carriers; `tabStyle` is the spelling the renderer reads in all of them)", + apply(stack, emit) { + return mapPageComponents(stack, (component, path) => { + if (component.type !== 'page:tabs') return component; + const properties = component.properties; + if (!isDict(properties)) return component; + const renamed = renameKey(properties, 'type', 'tabStyle'); + if (!renamed) return component; + emit({ from: 'type', to: 'tabStyle', path: `${path}.properties.tabStyle` }); + return { ...component, properties: renamed }; + }); + }, + fixture: { + before: { + pages: [ + { + name: 'sys_position_detail', + regions: [ + { + name: 'main', + components: [ + { type: 'page:tabs', properties: { type: 'card', items: [{ label: 'Holders' }] } }, + // Both spellings, SAME value: the redundant twin goes (#4923). + { type: 'page:tabs', properties: { tabStyle: 'pill', type: 'pill', items: [] } }, + // Both spellings, DIFFERENT looks: kept, so the author picks one + // rather than the loader picking for them. + { type: 'page:tabs', properties: { tabStyle: 'pill', type: 'card', items: [] } }, + // A `type` one level down inside another component's properties + // is a different key entirely — the walk is region-level and + // never descends into a props bag. + { + type: 'element:button', + properties: { label: 'Open', action: { type: 'url', target: '/x' } }, + }, + ], + }, + ], + }, + // The shape this key is really authored in: `page:tabs` IS one of the + // seven named slots, and all four in-repo sites are `slots.tabs` on a + // `kind: 'slotted'` record page. Region-only reach would have missed + // every one of them (#6776 — see `mapPageComponents`). + { + name: 'sys_user_detail', + regions: [], + slots: { + tabs: { type: 'page:tabs', properties: { type: 'line', position: 'top', items: [] } }, + }, + }, + ], + }, + after: { + pages: [ + { + name: 'sys_position_detail', + regions: [ + { + name: 'main', + components: [ + { type: 'page:tabs', properties: { tabStyle: 'card', items: [{ label: 'Holders' }] } }, + { type: 'page:tabs', properties: { tabStyle: 'pill', items: [] } }, + { type: 'page:tabs', properties: { tabStyle: 'pill', type: 'card', items: [] } }, + { + type: 'element:button', + properties: { label: 'Open', action: { type: 'url', target: '/x' } }, + }, + ], + }, + ], + }, + { + name: 'sys_user_detail', + regions: [], + slots: { + tabs: { type: 'page:tabs', properties: { tabStyle: 'line', position: 'top', items: [] } }, + }, + }, + ], + }, + expectedNotices: 3, + }, +}; + export const CONVERSIONS_BY_MAJOR: Readonly> = { 11: [flowNodeHttpRename, pageKindJsxToHtml, flowNodeFilterAlias, objectCompactLayoutRename], 13: [stackRolesToPositions, owdLegacyReadAliases, sharingRecipientRoleToPosition], @@ -5211,6 +5346,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly Dict } /** - * Immutably map every **region-level** page component in - * `stack.pages[].regions[].components[]`. + * Immutably map every **declared-shape** page component — the two places a + * `PageComponentSchema` actually lives: `stack.pages[].regions[].components[]` + * and `stack.pages[].slots.` (which is `PageComponent | PageComponent[]`). * * `mapper` receives each component dict and its path - * (`pages[i].regions[j].components[k]`) and returns the same reference (no - * change) or a new dict. The stack, `pages`, a page, its `regions`, a region - * and its `components` are each copied only when a descendant actually - * changed — {@link mapPages}' contract, one level deeper. + * (`pages[i].regions[j].components[k]`, `pages[i].slots.tabs`, + * `pages[i].slots.tabs[0]`) and returns the same reference (no change) or a new + * dict. Every container on the way — the stack, `pages`, a page, its `regions`, + * a region, its `components`, its `slots` — is copied only when a descendant + * actually changed: {@link mapPages}' contract, one level deeper. * - * Region level is the whole surface a page-component conversion can reach: - * `PageComponentSchema` declares no children key, so anything nested (tab - * panels, card bodies) sits inside another component's free-form `properties` - * and is not typed page-component shape. Same boundary, drawn for the same - * reason, as `translatePage` in `system/i18n-resolver.ts`. + * That is the whole surface, and it is bounded by the type rather than by the + * shape of any one page: `PageComponentSchema` declares no children key, so + * anything nested (tab panels, card bodies) sits inside another component's + * free-form `properties` and is NOT typed page-component shape — the tombstone + * (`tsc` + the parse) covers those, as every retirement entry's doc says. + * + * **`slots` was missing until #6776, and the gap was load-bearing.** This + * walker's own comment used to call region level "the whole surface", on the + * reasoning that everything else is inside a free-form bag. `slots` is the + * counter-example: `PageSchema.slots` is a closed map of seven named slots, + * each declared `z.union([PageComponentSchema, z.array(PageComponentSchema)])` + * — exactly as typed as a region component, and the canonical authoring shape + * for a `kind: 'slotted'` record page. `walkPageComponents` in `packages/lint` + * has always visited both, so every conversion here reached strictly less than + * the lint rule that judges the result. #6776 is where that cost something + * real: all four in-repo `page:tabs` authoring sites are `slots.tabs`, so a + * region-only rewrite would have left `os migrate meta` unable to touch the + * only shape that key is written in, while the tombstone's prescription + * promised it would. */ export function mapPageComponents( stack: Dict, mapper: (component: Dict, path: string) => Dict, ): Dict { return mapPages(stack, (page, pagePath) => { + let nextPage = page; + const regions = page.regions; - if (!Array.isArray(regions)) return page; + if (Array.isArray(regions)) { + let regionsChanged = false; + const nextRegions = regions.map((region, ri) => { + if (!isDict(region)) return region; + const components = region.components; + if (!Array.isArray(components)) return region; - let regionsChanged = false; - const nextRegions = regions.map((region, ri) => { - if (!isDict(region)) return region; - const components = region.components; - if (!Array.isArray(components)) return region; + let componentsChanged = false; + const nextComponents = components.map((component, ci) => { + if (!isDict(component)) return component; + const mapped = mapper(component, `${pagePath}.regions[${ri}].components[${ci}]`); + if (mapped !== component) componentsChanged = true; + return mapped; + }); - let componentsChanged = false; - const nextComponents = components.map((component, ci) => { - if (!isDict(component)) return component; - const mapped = mapper(component, `${pagePath}.regions[${ri}].components[${ci}]`); - if (mapped !== component) componentsChanged = true; - return mapped; + if (!componentsChanged) return region; + regionsChanged = true; + return { ...region, components: nextComponents }; }); - if (!componentsChanged) return region; - regionsChanged = true; - return { ...region, components: nextComponents }; - }); + if (regionsChanged) nextPage = { ...nextPage, regions: nextRegions }; + } + + const slots = page.slots; + if (isDict(slots)) { + let slotsChanged = false; + const nextSlots: Dict = { ...slots }; + for (const [slot, value] of Object.entries(slots)) { + // A slot holds one component or an array of them — the same + // normalization `walkPageComponents` does, and the path spelling + // matches it so a conversion notice and a lint finding name one site + // with one string. + if (Array.isArray(value)) { + let listChanged = false; + const nextList = value.map((component, i) => { + if (!isDict(component)) return component; + const mapped = mapper(component, `${pagePath}.slots.${slot}[${i}]`); + if (mapped !== component) listChanged = true; + return mapped; + }); + if (!listChanged) continue; + nextSlots[slot] = nextList; + slotsChanged = true; + } else { + if (!isDict(value)) continue; + const mapped = mapper(value, `${pagePath}.slots.${slot}`); + if (mapped === value) continue; + nextSlots[slot] = mapped; + slotsChanged = true; + } + } + if (slotsChanged) nextPage = { ...nextPage, slots: nextSlots }; + } - if (!regionsChanged) return page; - return { ...page, regions: nextRegions }; + return nextPage; }); } diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index ca8a18e2e1..cb8bb60005 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -1100,6 +1100,22 @@ const step17: MigrationStep = { + 'is implemented (#5021 / #4988). Not in scope, and deliberately: `page:card.visible` is a ' + 'component-level visibility predicate written into `properties` and hoisted by the ' + 'renderer — a page to rewrite onto the ADR-0089 `visibleWhen`, not a key to declare.\n\n' + + 'That count turned out to be incomplete, and #6776 finishes it: five more keys the ' + + 'renderers read were still undeclared. Four are plain additions with no behaviour change ' + + '(`page:header` `recordChrome`/`showStar`/`showCopyId`, which select between the ' + + 'record-chip header and the bare heading a dashboard wants, and `page:accordion.variant`, ' + + 'which decides whether the accordion draws its own dividers or leaves the border to each ' + + 'panel). The fifth is a rename, and the only one in the family whose defect is structural ' + + 'rather than an oversight: the tab strip\'s visual style was declared as ' + + '`page:tabs.type`, which collides with the page component\'s OWN dispatch key. objectui\'s ' + + '`SchemaRenderer` refuses to hoist `properties.type` for exactly that reason, ' + + '`sdui-parser`\'s `BASE_PROPS` contains `type` and skips it before any validation runs, ' + + 'and in a flat or JSX carrier the node reads `{ type: \'page:tabs\', … }` so the name is ' + + 'already taken. The key was therefore unauthorable in every carrier but the nested ' + + '`properties` object, and unvalidated even there. It becomes `tabStyle` — the spelling ' + + 'objectui publishes and the renderer already reads first in the flat carriers — which is ' + + '`displayField` → `labelField` again: converge on the spelling that works, not the one ' + + 'that declares well, and keep one spelling rather than two (Prime Directive #12).\n\n' + 'Finally it narrows the aggregation vocabulary: `array_agg` and `string_agg` leave ' + '`AggregationFunction` (#6188, ADR-0049). The enum declared eight functions and the SQL ' + 'family compiles five — `SqlDriver.mapAggregateFunc` and the Turso ' @@ -1190,6 +1206,7 @@ const step17: MigrationStep = { 'page-card-body-to-children', 'dataset-measure-array-string-agg-removed', 'inline-action-api-params-to-body-extra', + 'page-tabs-type-to-tab-style', ], semantic: [ { @@ -2933,6 +2950,12 @@ export const RETIRED_KEYS_BY_MAJOR: Readonly> 'ui/ElementRecordPickerProps:multiple', 'ui/ElementRecordPickerProps:searchFields', 'ui/PageCardProps:body', + // #6776 — #5775's count was incomplete. The tab strip's visual style is the + // one prop whose declared spelling collides with the page component's own + // dispatch key, so `type` could never be authored in a flat or JSX carrier + // and was skipped unvalidated by `sdui-parser`'s `BASE_PROPS`. Renamed to + // the `tabStyle` every carrier can express and the renderer already reads. + 'ui/PageTabsProps:type', ], }; diff --git a/packages/spec/src/ui/component.test.ts b/packages/spec/src/ui/component.test.ts index d27a168ca1..d9f3ed523a 100644 --- a/packages/spec/src/ui/component.test.ts +++ b/packages/spec/src/ui/component.test.ts @@ -50,28 +50,63 @@ describe('PageHeaderProps', () => { }); }); +// #6776 — the three record-chrome switches objectui's header renderer has always +// read (`containers.tsx:979-981`) and `PageHeaderProps` never declared. Until +// this declaration objectui's published manifest called them legal while +// `validateComponentProps` (#5068) called them undeclared — two platform +// authorities disagreeing about one key (#5435), with the renderer siding with +// the author. +describe('PageHeaderProps recordChrome / showStar / showCopyId (#6776)', () => { + it('defaults all three ON — an unauthored header keeps the record chrome', () => { + const result = PageHeaderProps.parse({ title: 'Lead' }); + expect(result.recordChrome).toBe(true); + expect(result.showStar).toBe(true); + expect(result.showCopyId).toBe(true); + }); + + it('accepts the console preview sample verbatim (`recordChrome: false` on a non-record page)', () => { + // objectui `apps/console/src/preview-samples.ts:68` — the exact shape that + // was reported as an undeclared key before this card. + const result = PageHeaderProps.parse({ title: 'Welcome to the CRM', recordChrome: false }); + expect(result.recordChrome).toBe(false); + }); + + it('accepts the star and copy-id switches independently', () => { + const result = PageHeaderProps.parse({ title: 'Lead', showStar: false, showCopyId: false }); + expect(result.showStar).toBe(false); + expect(result.showCopyId).toBe(false); + // Still a record header — only the two chips inside it are off. + expect(result.recordChrome).toBe(true); + }); + + it('rejects a non-boolean rather than silently stripping it', () => { + expect(() => PageHeaderProps.parse({ title: 'Lead', recordChrome: 'false' })).toThrow(); + expect(() => PageHeaderProps.parse({ title: 'Lead', showStar: 'no' })).toThrow(); + }); +}); + describe('PageTabsProps', () => { it('should accept valid tabs with defaults', () => { const tabs = { items: [{ label: 'Tab 1', children: [] }], }; const result = PageTabsProps.parse(tabs); - expect(result.type).toBe('line'); + expect(result.tabStyle).toBe('line'); expect(result.position).toBe('top'); expect(result.items).toHaveLength(1); }); it('should accept tabs with all options', () => { const tabs = { - type: 'card' as const, + tabStyle: 'card' as const, position: 'left' as const, items: [{ label: 'Tab 1', icon: 'settings', children: ['child1'] }], }; expect(() => PageTabsProps.parse(tabs)).not.toThrow(); }); - it('should reject invalid type enum', () => { - expect(() => PageTabsProps.parse({ type: 'invalid', items: [] })).toThrow(); + it('should reject invalid tabStyle enum', () => { + expect(() => PageTabsProps.parse({ tabStyle: 'invalid', items: [] })).toThrow(); }); it('should reject tabs without items', () => { @@ -119,6 +154,64 @@ describe('PageTabsProps', () => { }); }); +// #6776 — the tab strip's visual style moves from `type` to `tabStyle`. +// +// This is an acceptance-face change in BOTH directions, so both are pinned: the +// new key is accepted, and the old one is REFUSED BY NAME with the prescription +// rather than being stripped in silence (the retiredKey contract). The reason +// the concept had to change spelling at all is structural, not aesthetic: a +// props key named `type` collides with the page component's own dispatch key, +// which is why objectui's `SchemaRenderer.tsx:253,264` refuses to hoist +// `properties.type` and why `sdui-parser`'s `BASE_PROPS` (`validate.ts:20-30`) +// skips it before any validation runs. +describe('PageTabsProps tabStyle — renamed from `type` (#6776)', () => { + it('accepts the three declared styles under the new key', () => { + for (const tabStyle of ['line', 'card', 'pill'] as const) { + expect(PageTabsProps.parse({ tabStyle, items: [] }).tabStyle).toBe(tabStyle); + } + }); + + it('rejects the retired `type` with the rename prescription', () => { + // Not `.toThrow()` alone: an undeclared key on this non-strict schema would + // be stripped silently, and a bare throw assertion cannot tell the two + // apart. The message IS the migration doc, so it is what gets asserted. + expect(() => PageTabsProps.parse({ type: 'card', items: [] })) + .toThrow(/`type`.*removed.*`tabStyle`/s); + }); + + it('does not materialize the retired `type` on a clean parse', () => { + expect(PageTabsProps.parse({ tabStyle: 'card', items: [] })).not.toHaveProperty('type'); + }); + + it('still refuses a value outside the enum under the new key', () => { + expect(() => PageTabsProps.parse({ tabStyle: 'underline', items: [] })).toThrow(); + }); +}); + +// #6776 — `page:accordion.variant`, read at objectui `containers.tsx:734` and +// visible on screen (`flush` draws the divider, `card` leaves the border to the +// panel's own content), declared nowhere until now. +describe('PageAccordionProps variant (#6776)', () => { + const accordion = ComponentPropsMap['page:accordion']; + + it('defaults to `flush` — the renderer default, now stated in the contract', () => { + const result = accordion.parse({ items: [] }) as { variant?: string }; + expect(result.variant).toBe('flush'); + }); + + it('accepts the `card` opt-in the renderer invites authors to write', () => { + const result = accordion.parse({ + items: [{ label: 'Details', children: [] }], + variant: 'card', + }) as { variant?: string }; + expect(result.variant).toBe('card'); + }); + + it('rejects a variant outside the two the renderer branches on', () => { + expect(() => accordion.parse({ items: [], variant: 'bordered' })).toThrow(); + }); +}); + // #5775 — the two tab-item keys the renderer honours and the schema did not // declare. `value` is the load-bearing one: it is the `?tab=` token, and the // index-derived fallback (`tab-`) silently points at a different tab as soon diff --git a/packages/spec/src/ui/component.zod.ts b/packages/spec/src/ui/component.zod.ts index 08b25eef49..158b713078 100644 --- a/packages/spec/src/ui/component.zod.ts +++ b/packages/spec/src/ui/component.zod.ts @@ -227,12 +227,85 @@ export const PageHeaderProps = z.object({ icon: z.string().optional().describe('Icon name'), breadcrumb: z.boolean().default(true).describe('Show breadcrumb'), actions: z.array(z.string()).optional().describe('Action IDs to show in header'), + /** + * Which of the two page-header layouts the renderer builds (#6776). + * + * ON (the default) the header carries the **record chrome**: the title + * renders as a record chip with the follow star and the copy-record-id + * button beside it. OFF it falls back to a bare heading — one title line and + * nothing record-shaped — which is what a dashboard or a landing page wants, + * since there is no record for the chip to describe. + * + * Declared here because the renderer has always read it and the schema had + * not caught up: `containers.tsx:979` resolves + * `schema?.recordChrome === false || schema?.properties?.recordChrome === false` + * and `:1453` branches the whole header on it, while objectui's own console + * preview sample authors `recordChrome: false` on a non-record page. Until + * this declaration that page was legal per objectui's published manifest and + * `warning: undeclared` per `validateComponentProps` (#5068) — two platform + * authorities disagreeing about one key (#5435). + */ + recordChrome: z.boolean().default(true).describe( + 'Render the record chrome — the title as a record chip with its follow star and copy-id button. Set false on a non-record page (dashboard, landing) to fall back to the bare heading layout.', + ), + /** + * Follow (favourite) star beside the record title — `RecordTitleChip + * showStar` (#6776). Part of the record chrome, so it has no effect when + * `recordChrome` is false. Read at `containers.tsx:980`, consumed at `:1531`. + */ + showStar: z.boolean().default(true).describe( + 'Show the follow (favourite) star beside the record title. Part of the record chrome — no effect when `recordChrome` is false.', + ), + /** + * Copy-record-id button beside the record title — `RecordTitleChip + * showCopyId` (#6776). Same record-chrome scoping as `showStar`. Read at + * `containers.tsx:981`, consumed at `:1532`. + */ + showCopyId: z.boolean().default(true).describe( + 'Show the copy-record-id button beside the record title. Part of the record chrome — no effect when `recordChrome` is false.', + ), /** ARIA accessibility */ aria: AriaPropsSchema.optional().describe('ARIA accessibility attributes'), }); export const PageTabsProps = z.object({ - type: z.enum(['line', 'card', 'pill']).default('line'), + /** + * Tab-strip visual style. **Renamed from `type` at protocol 17 (#6776, + * ADR-0087 D2)** — the same concept, the same three values, a spelling an + * author can actually write. + * + * A props key named `type` collides with the component node's own dispatch + * key, and the collision is structural rather than cosmetic: + * + * - objectui's `SchemaRenderer` hoists `properties` onto the node but + * deliberately skips `type` and `id`, or the inner value would shadow + * which renderer to dispatch to — its comment names this exact case + * ("tab visual style: 'line' | 'card' | 'pill'"). + * - `sdui-parser`'s `BASE_PROPS` contains `'type'`, so a manifest input by + * that name is skipped as a base prop and never validated at all. + * - In the flat and JSX carriers a node reads `{ type: 'page:tabs', … }`, + * so `type` is the tag name and this prop has no spelling left. + * + * `tabStyle` is what objectui's registry publishes and what the renderer + * reads in every carrier (`containers.tsx:381`), so the contract converges on + * the spelling that works rather than the one that reads well — the #5775 + * `displayField` → `labelField` shape, and one spelling rather than two + * (Prime Directive #12). + */ + tabStyle: z.enum(['line', 'card', 'pill']).default('line') + .describe("Tab-strip visual style: 'line' underlines the active tab, 'card' frames each tab, 'pill' renders rounded pills"), + /** + * REMOVED (#6776). The declared spelling of `tabStyle`, unauthorable in any + * flat or JSX carrier because a page component's own dispatch key is also + * called `type`. The live mechanism is `tabStyle`. + */ + type: retiredKey( + '`page:tabs` property `type` was removed in @objectstack/spec 17.0.0 (#6776, ADR-0087 D2) — ' + + 'a props key named `type` collides with the page component\'s own dispatch key, so it is ' + + 'unauthorable in the flat and JSX carriers and was never validated in them. Rename the key ' + + 'to `tabStyle`; the value (`line` | `card` | `pill`) is unchanged. ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', + ), position: z.enum(['top', 'left']).default('top'), items: z.array(z.object({ label: I18nLabelSchema, @@ -516,6 +589,20 @@ export const PageAccordionProps = z.object({ children: z.array(z.unknown()).describe('Child components'), })), allowMultiple: z.boolean().default(false).describe('Allow multiple panels to be expanded simultaneously'), + /** + * Panel framing (#6776). `flush` is the renderer's own default and draws the + * divider itself (`border-b last:border-b-0` on every panel but the last); + * `card` hands the border to whatever each panel contains, so a panel holding + * a `page:card` does not get a second frame around the first. + * + * Declared here because the renderer has always read it — `containers.tsx:734` + * resolves `schema?.variant ?? schema?.properties?.variant ?? 'flush'`, and + * its own comment invites authors in ("Authors opt in by setting + * `variant: 'card'`"). The difference is visible on screen, so this was an + * author-facing option that `PageAccordionProps` simply never declared. + */ + variant: z.enum(['flush', 'card']).default('flush') + .describe("Panel framing: 'flush' draws a divider under each panel; 'card' leaves the border to each panel's own content"), /** ARIA accessibility */ aria: AriaPropsSchema.optional().describe('ARIA accessibility attributes'), }); From 19bec36bab009620309db4a06363d1c435f251fc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 02:21:04 +0000 Subject: [PATCH 2/2] chore(changeset): declare the ADR-0087 disposition for page-tabs-type-to-tab-style (#6776) The Check Changeset gate (#6148) requires a breaking changeset to answer the ledger question in writing. This one's answer is 'registered': the conversion entry, its D3 chain step and the RETIRED_KEYS_BY_MAJOR[17] key all land in this PR. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- .changeset/component-props-header-tabs-accordion.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.changeset/component-props-header-tabs-accordion.md b/.changeset/component-props-header-tabs-accordion.md index b08c7c4c02..5339fdfcda 100644 --- a/.changeset/component-props-header-tabs-accordion.md +++ b/.changeset/component-props-header-tabs-accordion.md @@ -79,3 +79,6 @@ slots, all four in-repo authoring sites are `slots.tabs`, and a region-only rewrite would have left `os migrate meta` unable to touch the only shape the key is written in — while the tombstone promised it could. Every other page-component conversion gains the same reach, in the direction it already declares. + + +