diff --git a/.changeset/spec-symbol-ledger-first-tranche.md b/.changeset/spec-symbol-ledger-first-tranche.md new file mode 100644 index 000000000..df2557ade --- /dev/null +++ b/.changeset/spec-symbol-ledger-first-tranche.md @@ -0,0 +1,55 @@ +--- +"@object-ui/types": patch +--- + +refactor(types): bind seven spec-named symbols to the spec instead of re-declaring them — objectstack#4115 ledger burn-down + +The `check-spec-symbol-derivation` ledger opened at **156** untriaged collisions. +This is the first tranche: **149** remain, and every symbol removed was *proved* +equivalent to the spec's before being replaced, not assumed equivalent because +its doc comment said so. Four of the seven carried exactly such a comment — +"Mirrors the server's `ImportWriteMode` (`@objectstack/spec`)", "(ObjectStack +Spec v2.0.1)" — which is the claim this issue exists to make true. + +Bound as re-exports (`@objectstack/spec/api`, `/kernel`, `/ui`): +`BreakpointName`, `ExportJobStatus`, `ImportJobStatus`, `ImportWriteMode`, +`ValidationError`. + +Derived with `z.infer` (`@objectstack/spec/data`): `JoinStrategy`, +`WindowFunction` — the spec exports these as zod enums rather than as types, so +a re-export would not compile against them. + +All seven are structurally unchanged, so no consumer changes: the full repo +type-check passes 76/76. + +**What decided the tranche.** Mutual assignability (`[Local] extends [Spec]` and +back) looks like the obvious test for "is this a safe re-export", and it lies in +three ways, all of them present in this repo: + +- The **spec's own** export resolves to `any` — `NavigationItem`, `JoinNode`, + `FormField`. Binding these would replace a precise local interface with `any`, + a type-safety regression wearing a burn-down's clothes. A naive probe reports + them as "identical to the spec" and recommends exactly the wrong edit. +- The **local** declaration resolves to `any` — recursive zod schemas annotated + `z.ZodType` (`FilterConditionSchema`, `NavigationItemSchema`). +- The local declaration carries `[key: string]: any` — the objectstack#4075 + mechanism, which absorbs any extra member so two types compare equal while + accepting wildly different objects (`FormField`, `AppSchema`, `PageSchema`, + `ThemeSchema`, and 12 more). + +A zod schema needs one question more than a type does: `FormFieldSchema` has an +**identical `_output` and a divergent `_input`**, so re-exporting it would have +silently changed what authoring input parses. All of this is now written into the +ledger's burn-down instructions, with the detection probe for each case. + +`spec-derived-unions.test.ts` gains an **inverted pin** for the three spec-side +`any` cases: it asserts they are *still* `any`. The day the spec types any of +them properly the assertion stops compiling, and the failure is the instruction +to re-run the triage and burn that symbol down. + +**Guard fix:** `referencesSpec` walked the declaration's own name node, so a +symbol whose name was also bound to a spec import counted as derived from +itself. TypeScript rejects that particular pair as a duplicate identifier, so it +was not reachable in compiling code — but a guard that depends on the compiler +having run first is a guard with a hole in it. The clean-tree result is +unchanged, confirming it was masking nothing. diff --git a/packages/types/src/__tests__/spec-derived-unions.test.ts b/packages/types/src/__tests__/spec-derived-unions.test.ts index f6b708369..57a24da21 100644 --- a/packages/types/src/__tests__/spec-derived-unions.test.ts +++ b/packages/types/src/__tests__/spec-derived-unions.test.ts @@ -58,7 +58,19 @@ import { ActionLocationSchema as SpecActionLocationSchema, ActionSchema as SpecActionSchema, } from '@objectstack/spec/ui'; -import { FieldType as SpecFieldType } from '@objectstack/spec/data'; +import { + FieldType as SpecFieldType, + JoinStrategy as SpecJoinStrategy, + WindowFunction as SpecWindowFunction, +} from '@objectstack/spec/data'; +import type { JoinNode as SpecJoinNode } from '@objectstack/spec/data'; +import type { + NavigationItem as SpecNavigationItem, + FormField as SpecFormField, +} from '@objectstack/spec/ui'; +import type { BreakpointName } from '../mobile'; +import type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError } from '../data'; +import type { JoinStrategy, WindowFunction } from '../data-protocol'; import { OBJECTUI_LOCAL_ACTION_TYPES, OBJECTUI_LOCAL_PARAM_FIELD_TYPES, @@ -112,9 +124,58 @@ const _resolvableCovers = null as unknown as ActionParamFieldType satisfies Reso // since objectui#3009 made this file compile.) const _fieldBackedParam: ActionParam = { field: 'status' }; const _minimalTypedParam: ActionParam = { name: 'priority', label: 'Priority', type: 'select' }; +// objectstack#4115 ledger burn-down: the symbols whose local declaration was +// PROVED equivalent to the spec's and then replaced by a binding. Each listed +// member is what the local fork carried, so re-declaring it narrower fails here +// as well as at the guard. +const _breakpointCovers = null as unknown as 'xs' | 'sm' | 'md' | 'lg' | 'xl' | '2xl' satisfies BreakpointName; +const _importModeCovers = null as unknown as 'insert' | 'update' | 'upsert' satisfies ImportWriteMode; +const _importStatusCovers = null as unknown as + | 'pending' | 'running' | 'succeeded' | 'failed' | 'cancelled' satisfies ImportJobStatus; +const _exportStatusCovers = null as unknown as + | 'pending' | 'processing' | 'completed' | 'failed' | 'cancelled' | 'expired' satisfies ExportJobStatus; +const _validationErrorShape: ValidationError = { field: 'name', message: 'required' }; +// The spec exports these two as zod enums, not types, so objectui derives them +// with `z.infer`. Reading the members back off the schema keeps the check honest +// if the spec widens either enum. +type SpecJoin = typeof SpecJoinStrategy extends { options: readonly (infer T)[] } ? T : never; +const _joinStrategyCovers = null as unknown as SpecJoin satisfies JoinStrategy; +type SpecWindow = typeof SpecWindowFunction extends { options: readonly (infer T)[] } ? T : never; +const _windowFunctionCovers = null as unknown as SpecWindow satisfies WindowFunction; + +/** + * Inverted pins — three collisions that are NOT burnable, and the tripwire that + * says when they become burnable. + * + * `NavigationItem`, `JoinNode` and `FormField` collide with a spec export whose + * own declaration resolves to `any` (the spec annotates the recursive schemas + * behind them as `z.ZodType`, and `z.infer` of that is `any`). Binding + * objectui's local interface to the spec would replace a precise, documented + * shape with `any` — a type-safety regression wearing a burn-down's clothes. So + * they stay in the ledger with their local declarations intact, which is the + * right answer for as long as the spec cannot describe them. + * + * Filed upstream as objectstack#4171 (4 of the spec's 2240 exported types). + * + * Mutual assignability CANNOT distinguish this case on its own: `any` answers + * every `extends` question affirmatively, so a naive probe reports these three + * as "identical to the spec" and recommends exactly the wrong edit. + * + * The day the spec types any of them properly, `IsAny<…>` flips to `false`, + * `true satisfies false` stops compiling, and the failure is the instruction: + * re-run the triage and burn that symbol down. + */ +type IsAny = 0 extends 1 & T ? true : false; +const _specNavigationItemIsStillAny = true satisfies IsAny; +const _specJoinNodeIsStillAny = true satisfies IsAny; +const _specFormFieldIsStillAny = true satisfies IsAny; + void _chartCovers; void _reportCovers; void _actionCovers; void _pageCovers; void _vizCovers; void _runnableCovers; void _componentCovers; void _paramFieldCovers; void _resolvableCovers; void _fieldBackedParam; void _minimalTypedParam; +void _breakpointCovers; void _importModeCovers; void _importStatusCovers; void _exportStatusCovers; +void _validationErrorShape; void _joinStrategyCovers; void _windowFunctionCovers; +void _specNavigationItemIsStillAny; void _specJoinNodeIsStillAny; void _specFormFieldIsStillAny; /** Read a spec enum's members, failing loudly if the shape ever changes. */ const optionsOf = (schema: unknown, name: string): string[] => { @@ -250,4 +311,17 @@ describe('unions derived from a spec vocabulary stay derived (#2944)', () => { const local = ['grid', 'gallery', 'kanban', 'calendar', 'timeline']; expect(local.filter((v) => spec.includes(v))).toEqual([]); }); + + it('JoinStrategy / WindowFunction come off the spec enum, not a restatement (objectstack#4115)', () => { + // The type aliases erase, so the runtime witness is the schema they are + // `z.infer`-ed from. Both were hand-written unions carrying a "(ObjectStack + // Spec v2.0.1)" doc header — a version claim nothing checked. + const joins = optionsOf(SpecJoinStrategy, 'JoinStrategy'); + expect(joins).toEqual(expect.arrayContaining(['auto', 'database', 'hash', 'loop'])); + + const windows = optionsOf(SpecWindowFunction, 'WindowFunction'); + for (const member of ['row_number', 'rank', 'dense_rank', 'lag', 'lead', 'sum', 'count']) { + expect(windows).toContain(member); + } + }); }); diff --git a/packages/types/src/data-protocol.ts b/packages/types/src/data-protocol.ts index 70e80d174..3c49aecb8 100644 --- a/packages/types/src/data-protocol.ts +++ b/packages/types/src/data-protocol.ts @@ -20,6 +20,15 @@ import type { SortConfig as BaseSortConfig } from './objectql'; import type { FilterOperator as BaseFilterOperator } from './complex'; +// Spec-owned vocabulary, bound rather than re-declared (objectstack#4115). The +// spec exports both of these as zod enums, not as types, so they are derived +// through `z.infer` — `export type { … } from` would not compile against a value. +import type { z } from 'zod'; +import type { + JoinStrategy as SpecJoinStrategy, + WindowFunction as SpecWindowFunction, +} from '@objectstack/spec/data'; + /** * ============================================================================= * Phase 3.3: QuerySchema AST Implementation @@ -81,9 +90,9 @@ export interface WhereNode extends QueryASTNode { } /** - * Join execution strategy hint (ObjectStack Spec v2.0.1) + * Join execution strategy hint — derived from the spec's `JoinStrategy` zod enum. */ -export type JoinStrategy = 'auto' | 'database' | 'hash' | 'loop'; +export type JoinStrategy = z.infer; /** * JOIN clause node (Phase 3.3.4) @@ -155,22 +164,9 @@ export interface AggregateNode extends QueryASTNode { } /** - * Window function type (ObjectStack Spec v2.0.1) - */ -export type WindowFunction = - | 'row_number' - | 'rank' - | 'dense_rank' - | 'percent_rank' - | 'lag' - | 'lead' - | 'first_value' - | 'last_value' - | 'sum' - | 'avg' - | 'count' - | 'min' - | 'max'; + * Window function type — derived from the spec's `WindowFunction` zod enum. + */ +export type WindowFunction = z.infer; /** * Window frame unit (ObjectStack Spec v2.0.1) diff --git a/packages/types/src/data.ts b/packages/types/src/data.ts index 2d0d31654..eb6a7f1b9 100644 --- a/packages/types/src/data.ts +++ b/packages/types/src/data.ts @@ -16,6 +16,15 @@ * @packageDocumentation */ +// Spec-owned names are bound here, not re-declared (objectstack#4115). Each of +// these already carried a doc comment claiming it mirrored the spec; the import +// is what makes the claim true, and what makes a spec change break the build +// instead of drifting quietly. +import type { ExportJobStatus, ImportJobStatus, ImportWriteMode } from '@objectstack/spec/api'; +import type { ValidationError } from '@objectstack/spec/kernel'; + +export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError }; + /** * Query parameters for data fetching. * Follows OData/REST conventions for universal compatibility. @@ -773,13 +782,12 @@ export interface DataSource { } /** - * How each incoming import row is committed against existing data. Mirrors the - * server's `ImportWriteMode` (`@objectstack/spec`). + * How each incoming import row is committed against existing data. Imported from + * `@objectstack/spec/api` at the top of this module. * - `insert` — always create a new record (default; ignores `matchFields`) * - `update` — update the record matched by `matchFields`; skip when none match * - `upsert` — update when matched, else create */ -export type ImportWriteMode = 'insert' | 'update' | 'upsert'; /** * A single source-column → target-field mapping with optional per-column @@ -875,15 +883,9 @@ export interface ImportRecordsResult { } /** - * Lifecycle status of an asynchronous import job. Mirrors the server's - * `ImportJobStatus` enum (`@objectstack/spec`). + * Lifecycle status of an asynchronous import job. Imported from + * `@objectstack/spec/api` at the top of this module. */ -export type ImportJobStatus = - | 'pending' - | 'running' - | 'succeeded' - | 'failed' - | 'cancelled'; /** * Result of {@link DataSource.createImportJob}. `jobId` is the polling key. @@ -1024,16 +1026,9 @@ export interface ExportDownloadRequest { } /** - * Lifecycle status of a server-driven export job. - * Mirrors the `ExportJobStatus` enum from `@objectstack/spec/export`. + * Lifecycle status of a server-driven export job. Imported from + * `@objectstack/spec/api` at the top of this module. */ -export type ExportJobStatus = - | 'pending' - | 'processing' - | 'completed' - | 'failed' - | 'cancelled' - | 'expired'; /** * Output formats supported by async export jobs. @@ -1256,24 +1251,9 @@ export interface DataBinding { } /** - * Validation error + * Validation error — imported from `@objectstack/spec/kernel` at the top of this + * module. */ -export interface ValidationError { - /** - * Field name - */ - field: string; - - /** - * Error message - */ - message: string; - - /** - * Error code - */ - code?: string; -} /** * API error response diff --git a/packages/types/src/mobile.ts b/packages/types/src/mobile.ts index 7dc65daf4..b75cd11f2 100644 --- a/packages/types/src/mobile.ts +++ b/packages/types/src/mobile.ts @@ -16,12 +16,20 @@ * @packageDocumentation */ +import type { BreakpointName } from '@objectstack/spec/ui'; + // ============================================================================ // Responsive Configuration // ============================================================================ -/** Breakpoint names */ -export type BreakpointName = 'xs' | 'sm' | 'md' | 'lg' | 'xl' | '2xl'; +/** + * Breakpoint names. + * + * Bound to the spec rather than re-declared (objectstack#4115): a local union + * under a spec export's name is read by the next reader as the spec's own + * definition, so a copy that is correct today is a planted premise tomorrow. + */ +export type { BreakpointName }; /** Responsive value - different values for different breakpoints */ export type ResponsiveValue = T | Partial>; diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs index a7b497212..68359e192 100644 --- a/scripts/check-spec-symbol-derivation.mjs +++ b/scripts/check-spec-symbol-derivation.mjs @@ -83,7 +83,7 @@ const ALLOW = { // Same governance as `check-type-check-coverage.mjs`'s DEBT map: declared, // shrink-only. These are the same-name symbols that predate this guard and have // not been triaged one-by-one yet. This check exists to stop the BLEEDING — a -// new fork fails on the PR that writes it — not to retro-fix 166 symbols at once. +// new fork fails on the PR that writes it — not to retro-fix every symbol at once. // // Named symbols rather than a per-package COUNT, deliberately: a count is a // budget. It lets the next fork land as long as an unrelated one was fixed in @@ -95,6 +95,39 @@ const ALLOW = { // local dialect (`ObjectUiLocal…`, with a tripwire test asserting the spec does // not own the name), or move it to ALLOW with the reason it deliberately // differs. Then delete it from this list — leaving it here fails the ratchet. +// +// ── Triage first, and do not trust `extends` alone ─────────────────────────── +// The obvious way to decide whether a collision is a safe re-export is to ask +// the compiler whether the two types are mutually assignable: +// +// type A = [Local] extends [Spec] ? true : false +// type B = [Spec] extends [Local] ? true : false // A && B → "identical" +// +// That question lies in three ways, and all three occur in this repo. Check for +// them BEFORE acting on an "identical" verdict: +// +// 1. The local declaration resolves to `any` — a recursive zod schema +// annotated `z.ZodType` (`FilterConditionSchema`, +// `NavigationItemSchema`). `any` answers every assignability question +// affirmatively. Detect: `0 extends (1 & Local) ? true : false`. +// 2. The SPEC export resolves to `any` (`NavigationItem`, `JoinNode`, +// `FormField`). Re-exporting these REPLACES a precise local interface with +// `any` — a type-safety regression wearing a burn-down's clothes. These +// cannot be burned down here at all; the fix belongs upstream in the spec, +// filed as objectstack#4171. `spec-derived-unions.test.ts` carries an +// inverted pin that fails the day the spec types one of them properly. +// Detect: the same `0 extends (1 & Spec)` probe. +// 3. The local declaration carries `[key: string]: any` (`FormField`, +// `AppSchema`, `PageSchema`, `ThemeSchema`, …) — the objectstack#4075 +// mechanism. An index signature absorbs any extra member, so the two types +// compare equal while accepting wildly different objects. +// Detect: `string extends keyof Local ? true : false`. +// +// A zod schema needs one more question than a type does: `_output` equality is +// not enough, because two schemas can agree on output and still accept different +// AUTHORING input. `FormFieldSchema` is exactly that — identical `_output`, +// divergent `_input` — so re-exporting it would silently change what parses. +// Compare `_input` too before touching a schema const. const DEBT_ISSUE = 4115; const DEBT = { "@object-ui/types": [ @@ -102,7 +135,6 @@ const DEBT = { "AppContextSelectorSchema", "AppSchema", "BatchOperationResult", - "BreakpointName", "CacheStrategy", "ChartSeries", "ChartSeriesSchema", @@ -115,7 +147,6 @@ const DEBT = { "DatasourceSchema", "DriverInterface", "EventHandler", - "ExportJobStatus", "ExpressionSchema", "FeedItemType", "FileMetadata", @@ -129,11 +160,8 @@ const DEBT = { "GestureType", "GlobalFilterSchema", "GroupByNode", - "ImportJobStatus", "ImportRowResult", - "ImportWriteMode", "JoinNode", - "JoinStrategy", "JoinedReportBlock", "ListViewSchema", "MutationEvent", @@ -163,12 +191,10 @@ const DEBT = { "StateMachineValidation", "Theme", "ThemeSchema", - "ValidationError", "ValidationRule", "ValidationRuleSchema", "WidgetManifest", "WidgetSource", - "WindowFunction", ], "@object-ui/app-shell": [ "ConversationSummary", @@ -413,11 +439,19 @@ const hasExportModifier = (node) => * `skipLiterals` stops the walk at object/array literals and type-literal member * blocks: a spec name mentioned inside a members block is a hand-written shape * that merely USES a spec type, which is exactly what a fork looks like. + * + * `exclude` drops the declaration's OWN name node from the walk. Without it a + * declaration whose name is itself bound to a spec import reads as derived from + * itself — `import type { X } from spec` next to `export type X = 'a' | 'b'` + * would be waved through, since the alias's own `X` identifier is in `bindings`. + * TypeScript rejects that particular pair as a duplicate identifier, so it is + * not reachable in compiling code, but a guard that depends on the compiler + * having run first is a guard with a hole in it. */ -function referencesSpec(node, bindings, skipLiterals) { +function referencesSpec(node, bindings, skipLiterals, exclude) { let found = false; const visit = (n) => { - if (found || !n) return; + if (found || !n || n === exclude) return; if (skipLiterals) { if (ts.isTypeLiteralNode(n) || ts.isObjectLiteralExpression(n) || ts.isArrayLiteralExpression(n)) return; } @@ -483,7 +517,7 @@ function scanFile(file, specNames) { if (!hasExportModifier(stmt)) continue; if (ts.isTypeAliasDeclaration(stmt)) { - record(stmt.name.text, "type", referencesSpec(stmt, specBindings, true), stmt); + record(stmt.name.text, "type", referencesSpec(stmt, specBindings, true, stmt.name), stmt); } else if (ts.isInterfaceDeclaration(stmt)) { // Only `extends` counts — see the header. const extendsSpec = (stmt.heritageClauses ?? []).some((h) => referencesSpec(h, specBindings, false)); @@ -494,7 +528,12 @@ function scanFile(file, specNames) { } else if (ts.isVariableStatement(stmt)) { for (const decl of stmt.declarationList.declarations) { if (!ts.isIdentifier(decl.name)) continue; - record(decl.name.text, "const", decl.initializer ? referencesSpec(decl, specBindings, true) : false, decl); + record( + decl.name.text, + "const", + decl.initializer ? referencesSpec(decl, specBindings, true, decl.name) : false, + decl + ); } } else if (ts.isEnumDeclaration(stmt)) { // An enum cannot be derived from anything — a spec-named one is a fork.