From 08dac3ae351cd8096df050aa7040c1b97660a742 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 03:26:47 +0000 Subject: [PATCH] =?UTF-8?q?fix(showcase):=20a=20`type:=20'modal'`=20target?= =?UTF-8?q?=20names=20a=20page=20=E2=80=94=20`showcase=5Fnew=5Ftask`=20bec?= =?UTF-8?q?omes=20a=20form=20action=20(#6739)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `showcase_new_task` declared `type: 'modal'` + `target: 'showcase_component_gallery'` — the showcase HOME PAGE. The dispatch opens the welcome page inside a dialog with zero form controls, so a command labelled "New Task" creates nothing. The issue's suggested one-key fix (point the modal at the `showcase_task` OBJECT) does not build: `defineStack`'s cross-reference walk accepts only declared PAGE names for a modal target. Per the maintainer ruling on #6739, that walk — together with the spec TSDoc and the published docs — IS the contract; objectui's page-then-object resolution is consumer leniency the renderer itself labels "Back-compat" and is being retired on its own sequenced card. So the fix is the TYPE, not the target: `type: 'form'` + `target: 'showcase_task.edit'`, structurally identical to `LogTimeAction`. PR #6737's inline `element:button` action of the same name is re-worked to the same shape in this commit (ruling item 1) — it depended on the object branch and only built because the cross-reference walk never visits an inline action (#6889). Pin tests assert the ruled shape on both sites plus the corpus-wide rule, so the reference corpus cannot drift back. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016R9de1FqP7NvwKvqXi92Gh --- examples/app-showcase/src/ui/actions/index.ts | 29 +++- examples/app-showcase/src/ui/pages/index.ts | 21 ++- examples/app-showcase/test/actions.test.ts | 138 +++++++++++++++++- 3 files changed, 179 insertions(+), 9 deletions(-) diff --git a/examples/app-showcase/src/ui/actions/index.ts b/examples/app-showcase/src/ui/actions/index.ts index ab39ac9228..51abea1802 100644 --- a/examples/app-showcase/src/ui/actions/index.ts +++ b/examples/app-showcase/src/ui/actions/index.ts @@ -173,14 +173,37 @@ export const LogTimeAction = defineAction({ refreshAfter: true, }); -/** global nav command-palette action. */ +/** + * global nav command action — "New Task". + * + * `type: 'form'` + an `.` FORM-view target, structurally + * identical to `LogTimeAction` above. It used to be `type: 'modal'` + + * `target: 'showcase_component_gallery'` — a command labelled "New Task" whose + * target named the showcase HOME PAGE, so the dispatch opened the welcome page + * inside a dialog with zero form controls and nothing to create a task with + * (#6739). + * + * The fix is the TYPE, not the target. A `type: 'modal'` target names a PAGE + * and only a page: the spec TSDoc (`packages/spec/src/ui/action.zod.ts`), the + * published docs (`content/docs/ui/actions.mdx`) and `defineStack`'s + * cross-reference walk (`packages/spec/src/stack.zod.ts`) all say so, and the + * walk REJECTS a registered modal action whose target is not a declared page — + * so "just point the modal at `showcase_task`" is a build error, not a fix + * (maintainer ruling, #6739). Opening an object's form is what `type: 'form'` + * is for, and it is validated: a form target pointing at a LIST view is itself + * a build error (#2554, see LogTimeAction). + * + * Coverage is not lost: `QuickViewAction` above is the corpus's + * modal-targeting-a-page specimen, and there the "open a dialog/page" semantics + * match its "Quick View" label. + */ export const NewTaskAction = defineAction({ name: 'showcase_new_task', label: 'New Task', icon: 'plus', objectName: task, - type: 'modal', - target: 'showcase_component_gallery', + type: 'form', + target: 'showcase_task.edit', locations: ['global_nav'], refreshAfter: true, }); diff --git a/examples/app-showcase/src/ui/pages/index.ts b/examples/app-showcase/src/ui/pages/index.ts index 60442b81ec..0d6a02e4c1 100644 --- a/examples/app-showcase/src/ui/pages/index.ts +++ b/examples/app-showcase/src/ui/pages/index.ts @@ -88,11 +88,22 @@ export const ComponentGalleryPage = definePage({ // so the button rendered, was clickable, and the click did nothing at all // (no request, no dialog, no navigation). #6597. // - // `type: 'modal'` + a string `target` is resolved page-first, then object - // (objectui `useActionModal.resolveModalTarget`); `showcase_task` names no - // page, so it lands on the object and opens the Task create form — which is - // what a button labelled "Create Task" should do. - { type: 'element:button', properties: { label: 'Create Task', icon: 'plus', action: { name: 'showcase_new_task', type: 'modal', target: 'showcase_task', refreshAfter: true } } }, + // #6597's first fix wrote this as `type: 'modal'` + `target: 'showcase_task'`, + // relying on objectui `useActionModal.resolveModalTarget` resolving a string + // target page-first and then falling back to an OBJECT. It is now + // `type: 'form'` + the object's `edit` FORM view, matching the registered + // `NewTaskAction` of the same name (src/ui/actions/index.ts) — one action + // name, one shape, in one corpus. + // + // Why the change (maintainer ruling on #6739): a `type: 'modal'` target + // names a PAGE, only — spec TSDoc, published docs and `defineStack`'s + // cross-reference walk all agree, and the walk rejects a registered modal + // action targeting a non-page. The object fallback is consumer leniency the + // renderer itself labels "Back-compat" and is being retired. This line only + // ever built because the cross-reference walk visits `config.actions` and + // never an INLINE action (#6889) — so it depended on a branch under + // retirement and on a validation hole, both at once. + { type: 'element:button', properties: { label: 'Create Task', icon: 'plus', action: { name: 'showcase_new_task', type: 'form', target: 'showcase_task.edit', refreshAfter: true } } }, ], }, { diff --git a/examples/app-showcase/test/actions.test.ts b/examples/app-showcase/test/actions.test.ts index 5e144efa4f..4e26bac856 100644 --- a/examples/app-showcase/test/actions.test.ts +++ b/examples/app-showcase/test/actions.test.ts @@ -3,7 +3,8 @@ import { describe, it, expect } from 'vitest'; import { actionBodyRunnerFactory, QuickJSScriptRunner } from '@objectstack/runtime'; -import { allActions, MarkDoneAction, PortfolioSnapshotAction } from '../src/ui/actions/index.js'; +import * as pages from '../src/ui/pages/index.js'; +import { allActions, MarkDoneAction, NewTaskAction, PortfolioSnapshotAction } from '../src/ui/actions/index.js'; /** * Execution-path coverage for declared actions. @@ -154,3 +155,138 @@ describe('showcase actions — the object-less (`global`) specimen', () => { }); }); }); + +/** + * `type: 'modal'` targets a PAGE, only — the corpus pin (#6739). + * + * `showcase_new_task` used to declare `type: 'modal'` + + * `target: 'showcase_component_gallery'`: a command labelled "New Task" whose + * target named the showcase HOME PAGE. The dispatch was measured in a browser + * while #6597 was being fixed — the dialog rendered the welcome page inside + * itself, with **zero** form controls. + * + * The tempting one-key fix (point the modal at the `showcase_task` OBJECT) is + * a BUILD ERROR, not a fix: `defineStack`'s cross-reference walk + * (`packages/spec/src/stack.zod.ts`) accepts only declared PAGE names for a + * modal target, which is also what the spec TSDoc and the published docs say. + * objectui's page-then-object resolution is consumer leniency the renderer + * itself labels "Back-compat" and is being retired (maintainer ruling on + * #6739). Opening an object's form is `type: 'form'`'s job. + * + * These assertions pin the ruled shape on BOTH sites the name appears at, so + * the corpus — which is reference material humans and AI authors copy — cannot + * drift back: + * - registered actions: no `type: 'modal'` target that is not a declared page. + * That mirrors the build gate, so a regression fails here first with a + * readable message instead of as an import-time crash in nine other files; + * - INLINE page-element actions: the same rule, which the build gate does NOT + * enforce — the cross-reference walk visits `config.actions` only and never + * an inline action (#6889). This is the corpus's own guard over that hole, + * and it is exactly how the old `element:button` line built while depending + * on the object branch; + * - `showcase_new_task` itself is `type: 'form'` at a FORM view, structurally + * identical to `showcase_log_time`. + */ +describe("showcase actions — a `type: 'modal'` target names a page (#6739)", () => { + type AnyAction = { name?: unknown; type?: unknown; target?: unknown }; + type AnyComponent = { type?: unknown; properties?: Record }; + + /** Every page name the showcase declares — the set a modal target may name. */ + const pageNames = new Set( + (Object.values(pages) as unknown[]) + .filter( + (p): p is { name: string } => + !!p && typeof p === 'object' && !Array.isArray(p) && typeof (p as { name?: unknown }).name === 'string', + ) + .map((p) => p.name), + ); + + /** Every component on every page — regions and slots alike, nesting included. */ + function allComponents(page: Record): AnyComponent[] { + const out: AnyComponent[] = []; + const visit = (node: unknown): void => { + if (!node || typeof node !== 'object' || Array.isArray(node)) return; + const component = node as AnyComponent; + out.push(component); + const props = component.properties; + if (!props) return; + for (const item of Array.isArray(props.items) ? props.items : []) { + const children = (item as { children?: unknown })?.children; + for (const child of Array.isArray(children) ? children : []) visit(child); + } + for (const child of Array.isArray(props.children) ? props.children : []) visit(child); + }; + for (const region of (page.regions as { components?: unknown[] }[] | undefined) ?? []) { + for (const c of region.components ?? []) visit(c); + } + for (const slot of Object.values((page.slots as Record) ?? {})) { + for (const c of Array.isArray(slot) ? slot : [slot]) visit(c); + } + return out; + } + + /** Inline actions authored on a page element (`element:button`'s `action`). */ + const inlineActions = (): { page: string; action: AnyAction }[] => { + const out: { page: string; action: AnyAction }[] = []; + for (const page of Object.values(pages) as unknown[]) { + if (!page || typeof page !== 'object' || Array.isArray(page)) continue; + const p = page as Record; + if (typeof p.name !== 'string') continue; + for (const component of allComponents(p)) { + const action = component.properties?.action; + if (action && typeof action === 'object' && !Array.isArray(action)) { + out.push({ page: p.name, action: action as AnyAction }); + } + } + } + return out; + }; + + it('the page set the corpus can target is non-empty', () => { + // Guards the assertions below against passing vacuously: an empty page set + // would make "targets a declared page" trivially unfalsifiable. + expect(pageNames.size).toBeGreaterThan(10); + expect(pageNames.has('showcase_component_gallery')).toBe(true); + }); + + it('every REGISTERED modal action targets a declared page', () => { + const modals = (allActions as AnyAction[]).filter((a) => a.type === 'modal'); + // Keep the modal-targeting-a-page specimen alive: if this ever hits zero + // the rule below stops being exercised by anything. + expect(modals.length).toBeGreaterThan(0); + for (const a of modals) { + expect( + pageNames.has(String(a.target)), + `action '${String(a.name)}': a modal target names a PAGE, but '${String(a.target)}' is not a declared page — ` + + `use type: 'form' with an . target to open a form (#6739)`, + ).toBe(true); + } + }); + + it('every INLINE page-element modal action targets a declared page too', () => { + // The build gate cannot see these (#6889): `defineStack`'s cross-reference + // walk visits `config.actions` only. This is the corpus's own guard. + const inline = inlineActions(); + expect(inline.length).toBeGreaterThan(0); + for (const { page, action } of inline) { + if (action.type !== 'modal') continue; + expect( + pageNames.has(String(action.target)), + `page '${page}': inline action '${String(action.name)}' is type:'modal' targeting ` + + `'${String(action.target)}', which is not a declared page (#6739)`, + ).toBe(true); + } + }); + + it("`showcase_new_task` opens the Task form — type: 'form' at a FORM view", () => { + expect(NewTaskAction.type).toBe('form'); + expect(NewTaskAction.target).toBe('showcase_task.edit'); + + // Both sites carrying the name agree, so the corpus teaches one shape. + const inline = inlineActions() + .map(({ action }) => action) + .filter((a) => a.name === 'showcase_new_task'); + expect(inline).toHaveLength(1); + expect(inline[0]).toMatchObject({ type: 'form', target: 'showcase_task.edit' }); + }); +});