diff --git a/.changeset/retire-spec-shadow-forks.md b/.changeset/retire-spec-shadow-forks.md new file mode 100644 index 000000000..5ada65463 --- /dev/null +++ b/.changeset/retire-spec-shadow-forks.md @@ -0,0 +1,44 @@ +--- +"@object-ui/types": minor +--- + +refactor(types): retire the five forks that shadowed a `@objectstack/spec` vocabulary (#2944) + +Five declarations in `@object-ui/types` restated a spec vocabulary, four of them +re-exported under **the spec's own symbol name** — so an importer could not tell +which definition they had. Every one had already drifted: + +| Declaration | Was | Spec | +|---|---|---| +| `ChartTypeSchema` (`zod/data-display.zod.ts`) | 7 values | **19** | +| `ChartType` (`data-display.ts`) | 7 values | **19** | +| `PageTypeSchema` (`zod/layout.zod.ts`) | 4 — no `list` | 5 | +| `PageType` (`layout.ts`) | 10 — five the spec repudiates | 5 + local | +| `ReportType` (`reports.ts`) | 3 — no `joined` | 4 | +| `ActionType` (`ui-action.ts`) | 5 — no `form` | 6 | + +All are now the spec's schema by reference, or its type re-exported/derived. + +**This is why #2901 was filed with an inverted premise.** It read the 7-value +`ChartTypeSchema` as the protocol and concluded `plugin-charts` had outgrown it +with renderer-local dialect. The spec has 19; the 7-value list was this fork. + +**Widening only for consumers.** `ActionType` gains `form` (which +`ActionRunner.executeForm` already implemented, so a host app previously got a +type error on working code), `ReportType` gains `joined`, `ChartType` goes 7 → 19, +and `PageTypeSchema` gains `list`. Nothing was removed, so no existing value +stops type-checking or validating. Verified against the whole repo: 76/76 +type-check tasks and 8215 tests pass. + +**`PageType` keeps a named local extension.** `grid`/`gallery`/`kanban`/ +`calendar`/`timeline` are visualizations, not page kinds — `ui/page.zod.ts` says +so outright — but narrowing them away is a breaking type change for anyone +assigning `pageType: 'kanban'`. They are now `PageVisualizationAlias`, a +sanctioned and documented local extension (issue #2231's prescription) rather +than five names hidden inside a hand-written union. Removing it is the separate +"visualizations are not page types" cleanup. + +Guarded going forward: `spec-subschema-parity.test.ts` pins the two zod schemas +**by reference** (a faithful copy fails, because a copy is a fork), and the new +`spec-derived-unions.test.ts` covers the type aliases, which reference identity +cannot reach. diff --git a/packages/types/src/__tests__/spec-derived-unions.test.ts b/packages/types/src/__tests__/spec-derived-unions.test.ts new file mode 100644 index 000000000..1b09930a2 --- /dev/null +++ b/packages/types/src/__tests__/spec-derived-unions.test.ts @@ -0,0 +1,104 @@ +/** + * 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. + */ + +/** + * Hand-written unions that shadowed a spec vocabulary. (#2944, #2901) + * + * `spec-subschema-parity.test.ts` pins zod schemas by REFERENCE, which is the + * strongest possible check — a faithful copy fails, because a copy is a fork. + * TS type aliases cannot be pinned that way: they erase at runtime, and a + * restated union that happens to match is indistinguishable from a derived one. + * + * So these assert the runtime vocabulary instead, which is what a restated union + * would have drifted from. Each of these WAS drifted: + * + * - `ChartType` carried 7 of the spec's 19 values. Because the sibling zod + * schema was re-exported under the spec's own symbol name, #2901 was filed + * reading this copy as the protocol and concluding the renderer had outgrown + * it — the premise was backwards. + * - `ReportType` was missing `joined`. + * - `ActionType` was missing `form`, under a doc comment claiming to be "the + * canonical definition from @objectstack/spec". `ActionRunner.executeForm` + * implements it, so a host app typing against @object-ui/types got an error + * on working code. + * + * The `satisfies` checks below are the real enforcement: they stop compiling if + * the objectui alias stops covering the spec vocabulary. The runtime assertions + * guard against the spec's own enum being emptied or renamed underneath us. + */ +import { describe, it, expect } from 'vitest'; +import { + ChartTypeSchema as SpecChartTypeSchema, + ReportType as SpecReportType, + ActionType as SpecActionType, + PageTypeSchema as SpecPageTypeSchema, +} from '@objectstack/spec/ui'; +import type { ChartType } from '../data-display'; +import type { ReportType } from '../reports'; +import type { ActionType } from '../ui-action'; +import type { PageType, PageVisualizationAlias } from '../layout'; + +/** + * Compile-time coverage: every spec member must be assignable to the objectui + * alias. A restated union that drops a value fails to compile here. + */ +type SpecChart = typeof SpecChartTypeSchema extends { options: readonly (infer T)[] } ? T : never; +const _chartCovers = null as unknown as SpecChart satisfies ChartType; +const _reportCovers = null as unknown as 'tabular' | 'summary' | 'matrix' | 'joined' satisfies ReportType; +const _actionCovers = null as unknown as 'script' | 'url' | 'modal' | 'flow' | 'api' | 'form' satisfies ActionType; +const _pageCovers = null as unknown as 'record' | 'home' | 'app' | 'utility' | 'list' satisfies PageType; +// The sanctioned local extension is still part of the union. +const _vizCovers = null as unknown as PageVisualizationAlias satisfies PageType; +void _chartCovers; void _reportCovers; void _actionCovers; void _pageCovers; void _vizCovers; + +/** Read a spec enum's members, failing loudly if the shape ever changes. */ +const optionsOf = (schema: unknown, name: string): string[] => { + const raw = (schema as { options?: readonly string[] })?.options; + if (!Array.isArray(raw) || raw.length === 0) { + throw new Error(`could not read ${name}.options from @objectstack/spec`); + } + return [...raw]; +}; + +describe('unions derived from a spec vocabulary stay derived (#2944)', () => { + it('the spec still exposes each enum', () => { + // Guards every assertion below from passing on an empty list. + expect(optionsOf(SpecChartTypeSchema, 'ChartTypeSchema').length).toBeGreaterThan(0); + expect(optionsOf(SpecReportType, 'ReportType').length).toBeGreaterThan(0); + expect(optionsOf(SpecActionType, 'ActionType').length).toBeGreaterThan(0); + expect(optionsOf(SpecPageTypeSchema, 'PageTypeSchema').length).toBeGreaterThan(0); + }); + + it('ActionType includes `form` — the member the fork dropped', () => { + expect(optionsOf(SpecActionType, 'ActionType')).toContain('form'); + }); + + it('ReportType includes `joined` — the member the fork dropped', () => { + expect(optionsOf(SpecReportType, 'ReportType')).toContain('joined'); + }); + + it('PageTypeSchema includes `list` — the member the fork dropped', () => { + expect(optionsOf(SpecPageTypeSchema, 'PageTypeSchema')).toContain('list'); + }); + + it('ChartTypeSchema is far wider than the 7 the fork carried', () => { + const spec = optionsOf(SpecChartTypeSchema, 'ChartTypeSchema'); + const forked = ['line', 'bar', 'area', 'pie', 'donut', 'radar', 'scatter']; + expect(spec.length).toBeGreaterThan(forked.length); + // Everything the fork had is still real, so no consumer of the old union breaks. + expect(forked.filter((v) => !spec.includes(v))).toEqual([]); + }); + + it('the page visualization names are NOT spec page types', () => { + // `ui/page.zod.ts` says so explicitly; they survive only as the sanctioned + // local extension in `layout.ts`. If the spec ever adopts one, drop it there. + const spec = optionsOf(SpecPageTypeSchema, 'PageTypeSchema'); + const local = ['grid', 'gallery', 'kanban', 'calendar', 'timeline']; + expect(local.filter((v) => spec.includes(v))).toEqual([]); + }); +}); diff --git a/packages/types/src/__tests__/spec-subschema-parity.test.ts b/packages/types/src/__tests__/spec-subschema-parity.test.ts index 9cfb13c32..b79f717a0 100644 --- a/packages/types/src/__tests__/spec-subschema-parity.test.ts +++ b/packages/types/src/__tests__/spec-subschema-parity.test.ts @@ -41,6 +41,8 @@ import { AnimationSchema as SpecAnimationSchema, ZIndexSchema as SpecZIndexSchema, ThemeModeSchema as SpecThemeModeSchema, + ChartTypeSchema as SpecChartTypeSchema, + PageTypeSchema as SpecPageTypeSchema, ThemeSchema as SpecThemeSchema, } from '@objectstack/spec/ui'; import { @@ -61,6 +63,8 @@ import { ThemeModeSchema, ThemeDefinitionSchema, } from '../zod/theme.zod.js'; +import { ChartTypeSchema } from '../zod/data-display.zod.js'; +import { PageTypeSchema } from '../zod/layout.zod.js'; describe('spec sub-schema re-exports are the spec objects (by reference)', () => { const pairs: Array<[string, unknown, unknown]> = [ @@ -77,6 +81,13 @@ describe('spec sub-schema re-exports are the spec objects (by reference)', () => ['ZIndexSchema', ZIndexSchema, SpecZIndexSchema], ['ThemeModeSchema', ThemeModeSchema, SpecThemeModeSchema], ['ThemeDefinitionSchema', ThemeDefinitionSchema, SpecThemeSchema], + // #2944 — these two were forks that had already drifted, re-exported under + // the spec's own symbol name so an importer could not tell them apart. + // `ChartTypeSchema` carried 7 of the spec's 19 values and is why #2901 was + // filed against the wrong side of the contract; `PageTypeSchema` was missing + // `list`. Pinned by reference so neither can be re-forked silently. + ['ChartTypeSchema', ChartTypeSchema, SpecChartTypeSchema], + ['PageTypeSchema', PageTypeSchema, SpecPageTypeSchema], ]; it.each(pairs.map(([name]) => [name] as const))('%s', (name) => { diff --git a/packages/types/src/data-display.ts b/packages/types/src/data-display.ts index 24f0b7a8d..1c080ef6c 100644 --- a/packages/types/src/data-display.ts +++ b/packages/types/src/data-display.ts @@ -15,6 +15,7 @@ * @packageDocumentation */ +import type { ChartType as SpecChartType } from '@objectstack/spec/ui'; import type { BaseSchema, SchemaNode } from './base'; /** @@ -686,7 +687,14 @@ export interface TreeViewSchema extends BaseSchema { /** * Chart type */ -export type ChartType = 'line' | 'bar' | 'area' | 'pie' | 'donut' | 'radar' | 'scatter'; +/** + * Chart Type — `@objectstack/spec`'s own `ChartType` re-exported (issue + * #2231/#2901; formerly a hand-written union that had drifted to 7 of 19 values). + * + * Re-exported rather than restated, so a chart family the spec adds cannot go + * missing here. + */ +export type ChartType = SpecChartType; /** * Chart data series diff --git a/packages/types/src/layout.ts b/packages/types/src/layout.ts index 8f2c40a5a..f9642ff72 100644 --- a/packages/types/src/layout.ts +++ b/packages/types/src/layout.ts @@ -16,6 +16,7 @@ * @packageDocumentation */ +import type { PageType as SpecPageType } from '@objectstack/spec/ui'; import type { BaseSchema, SchemaNode } from './base'; /** @@ -425,27 +426,39 @@ export interface AspectRatioSchema extends BaseSchema { } /** - * Page Type - * Determines page behavior and default layout template. - * Aligned with @objectstack/spec Page.type + * List visualization names that are still accepted in the `pageType` slot. + * + * These are **not** page kinds. `@objectstack/spec` `ui/page.zod.ts` states it + * outright: they are visualizations of a `list` page, selected via + * `interfaceConfig.appearance.allowedVisualizations`. They are retained here as + * a **named, sanctioned local extension** (issue #2231's prescription) pending + * the "visualizations are not page types" cleanup, so that the spec-owned half + * of `PageType` can be derived while the objectui-only half stays visible + * instead of hiding inside a hand-written union. + * + * Narrowing this to `never` is the cleanup; it is a breaking type change for + * anyone assigning `pageType: 'kanban'`, so it is a separate decision. */ -export type PageType = - | 'record' - | 'home' - | 'app' - | 'utility' - // The roadmap page types (dashboard/form/record_detail/record_review/overview/ - // blank) were removed: they have no renderer and were dropped from - // @objectstack/spec PageTypeSchema (framework#2265, enforce-or-remove). - // NOTE: grid/list/gallery/kanban/calendar/timeline below are list - // VISUALIZATIONS, not page kinds — retained pending a separate cleanup. +export type PageVisualizationAlias = | 'grid' - | 'list' | 'gallery' | 'kanban' | 'calendar' | 'timeline'; +/** + * Page Type + * Determines page behavior and default layout template. + * + * The spec-owned half is `@objectstack/spec`'s `PageType` **by reference** + * (issue #2231/#2901; formerly a hand-written union). That mirror had drifted in + * BOTH directions at once — it carried the five visualization names above, which + * the spec explicitly repudiates, while the sibling zod `PageTypeSchema` in + * `zod/layout.zod.ts` was missing `list`. Three disagreeing definitions of one + * vocabulary lived in this package. + */ +export type PageType = SpecPageType | PageVisualizationAlias; + /** * Page Variable * Local page state that can be read/written by components and expressions. diff --git a/packages/types/src/reports.ts b/packages/types/src/reports.ts index 240bffd6b..1408cbf54 100644 --- a/packages/types/src/reports.ts +++ b/packages/types/src/reports.ts @@ -30,6 +30,8 @@ * migration window. */ +import type { z } from 'zod'; +import type { ReportType as SpecReportType } from '@objectstack/spec/ui'; import type { BaseSchema, SchemaNode } from './base'; import type { ChartSchema } from './data-display'; import type { DataSource } from './data'; @@ -329,12 +331,19 @@ export interface ReportExportConfig { } /** - * Report Type - * - tabular: flat table - * - summary: grouped with subtotals - * - matrix: pivot table + * Report Type — derived from `@objectstack/spec`'s `ReportType` enum (issue + * #2231/#2901; formerly a hand-written union). + * + * - `tabular` — flat list, no grouping + * - `summary` — row-wise grouping with aggregations + * - `matrix` — row × column pivot + * - `joined` — multiple independent report blocks stacked + * + * The mirror was missing `joined`, so a spec-valid joined report did not + * type-check against `ReportSchema.reportType`. Derived rather than restated so + * a report format the spec adds cannot go missing here. */ -export type ReportType = 'tabular' | 'summary' | 'matrix'; +export type ReportType = z.infer; /** * Report Schema - Main report configuration diff --git a/packages/types/src/ui-action.ts b/packages/types/src/ui-action.ts index 69938d4cf..1cc1dae59 100644 --- a/packages/types/src/ui-action.ts +++ b/packages/types/src/ui-action.ts @@ -19,6 +19,7 @@ * @packageDocumentation */ import { z } from 'zod'; +import type { ActionType as SpecActionType } from '@objectstack/spec/ui'; // ============================================================================ // Spec-Canonical Action Sub-types — imported from @objectstack/spec/ui @@ -66,15 +67,18 @@ export type ActionComponent = | 'action:group'; // Action group/dropdown /** - * Action execution type - * Canonical definition from @objectstack/spec/ui ActionSchema.type. + * Action execution type — derived from `@objectstack/spec`'s `ActionType` enum + * (issue #2231/#2901; formerly a hand-written union). + * + * `script` | `url` | `modal` | `flow` | `api` | `form` + * + * The previous union claimed to be the "canonical definition from + * @objectstack/spec" and was missing `form` — so a host app typing against + * `@object-ui/types` got a type error on `type: 'form'` even though + * `ActionRunner.executeForm` implements it. Derived now, so the claim is + * enforced by the compiler rather than asserted in a comment. */ -export type ActionType = - | 'script' // Execute JavaScript/expression - | 'url' // Navigate to URL - | 'modal' // Open modal dialog - | 'flow' // Start workflow/automation - | 'api'; // Call API endpoint +export type ActionType = z.infer; /** * Field type for action parameters diff --git a/packages/types/src/zod/data-display.zod.ts b/packages/types/src/zod/data-display.zod.ts index 4e61f8a31..f6fad672c 100644 --- a/packages/types/src/zod/data-display.zod.ts +++ b/packages/types/src/zod/data-display.zod.ts @@ -17,6 +17,7 @@ */ import { z } from 'zod'; +import { ChartTypeSchema as SpecChartTypeSchema } from '@objectstack/spec/ui'; import { BaseSchema, SchemaNodeSchema } from './base.zod.js'; /** @@ -204,17 +205,16 @@ export const TreeViewSchema = BaseSchema.extend({ }); /** - * Chart Type Enum + * Chart Type Enum — `@objectstack/spec/ui` schema re-exported **by reference** + * (issue #2231; formerly a hand-written mirror). + * + * The mirror had drifted to 7 of the spec's 19 values, and because it was + * re-exported under the spec's own symbol name, an importer of + * `@object-ui/types` could not tell which `ChartTypeSchema` they had. That is + * how #2901 came to be filed against the wrong side of the contract: it read + * this copy as the protocol and concluded the renderer had outgrown it. */ -export const ChartTypeSchema = z.enum([ - 'line', - 'bar', - 'area', - 'pie', - 'donut', - 'radar', - 'scatter', -]); +export const ChartTypeSchema = SpecChartTypeSchema; /** * Chart Series Schema diff --git a/packages/types/src/zod/layout.zod.ts b/packages/types/src/zod/layout.zod.ts index 519ba74ff..e7cfa3234 100644 --- a/packages/types/src/zod/layout.zod.ts +++ b/packages/types/src/zod/layout.zod.ts @@ -17,6 +17,7 @@ */ import { z } from 'zod'; +import { PageTypeSchema as SpecPageTypeSchema } from '@objectstack/spec/ui'; import { BaseSchema, SchemaNodeSchema } from './base.zod.js'; /** @@ -253,9 +254,16 @@ export const PageVariableSchema = z.object({ }); /** - * Page Type Schema + * Page Type Schema — `@objectstack/spec/ui` schema re-exported **by reference** + * (issue #2231; formerly a hand-written mirror). + * + * The mirror was missing `list`, so a spec-valid `list` page failed validation + * here — and it shadowed the spec's export under the same symbol name, so an + * importer could not tell the two apart. Note the sibling TS `PageType` in + * `layout.ts` had drifted the OPPOSITE way (it carried five visualization names + * the spec explicitly repudiates); both now come from the spec. */ -export const PageTypeSchema = z.enum(['record', 'home', 'app', 'utility']); +export const PageTypeSchema = SpecPageTypeSchema; /** * Page Schema - Top-level page layout