diff --git a/.changeset/type-check-the-types-tests.md b/.changeset/type-check-the-types-tests.md new file mode 100644 index 000000000..a9588f81f --- /dev/null +++ b/.changeset/type-check-the-types-tests.md @@ -0,0 +1,56 @@ +--- +'@object-ui/types': patch +--- + +**`@object-ui/types`' tests are type-checked, so the spec-derivation guards actually run (framework#4074).** + +`spec-derived-unions.test.ts` exists to stop a spec-derived union from being +re-forked into a hand-written copy, and its header claimed the `satisfies` checks +in it "are the real enforcement". They were not. `tsconfig.json` excludes test +files — correctly, since it is the package build with `rootDir` / `composite` / +`declaration`, so tests would emit into dist — and no other `tsc` invocation read +them. Measured, not assumed: reverting `ActionParamFieldType` from the spec's +`FieldType` back to its old hand-written subset produced **zero** type errors. + +It now produces `TS1360` on the `satisfies` line. Same for the sibling guards over +`ChartType`, `ReportType`, `ActionType` and `PageType`, which were equally inert — +the anti-regression mechanism left by #2944/#2901 was not running. + +`packages/types/tsconfig.test.json` follows the shape the package already uses for +`tsconfig.examples.json`: a separate, emit-free project chained from `type-check`. +Kept separate rather than deleting the exclude so the BUILD stays honest — the +reexport guard's source scan needs `types: ["node"]`, and folding that into +`tsconfig.json` would let package source reference Node APIs and still compile, in +a package that ships to browsers. + +Turning it on surfaced 39 pre-existing type errors in test files, all fixed here +except one declared gap: + +- **`p2-spec-exports.test.ts`** imported eight `…Schema` names as types from + `../index`. #2561 decision (a) removed those, and the sibling + `spec-ui-schema-reexports.test.ts` asserts their absence — so this file + contradicted its own guard for the whole interval. A type-only import of a + nonexistent name erases at runtime, so the suite stayed green. Its minimal + fixtures were also typed as parsed OUTPUT while being parse INPUT (these schemas + `.default()` several fields); they now use `z.input<>`, the distinction spec + draws itself with `ActionInput`. `operator: 'eq'` is likewise a legacy alias spec + folds at parse time, valid as input and absent from the canonical output union. +- **`app-creation-types.test.ts` / `system-fields.test.ts`** imported the package + by its own name. `turbo`'s `type-check` depends on `^build` (upstream only), so + the package's own `dist` does not exist when it runs; they now use the relative + import every sibling test uses. +- **`p1-spec-alignment.test.ts`** is excluded with a written reason, and is real + debt rather than hygiene: all 14 of its errors sit in tests named + "should accept <shape>" whose entire purpose is asserting the type accepts + that shape, and the type rejects it. The clearest case — + "should accept sharing in ObjectUI format `{ visibility, enabled }`" — describes + a shape that IS handled, by `foldSharing` in core's `normalize-list-view.ts`, but + only as untyped input (`normalizeListViewSchema(schema: T): T`), so no type + names it. Each site is a separate decision (widen the type so the claim becomes + true, or drop the claim) and several touch the public surface, so they are + tracked on framework#4074 instead of being silently rewritten here. + +Only `packages/types` is converted. 28 other packages still exclude their tests +from type-checking, and 5 (`fields`, `cli`, `data-objectstack`, `plugin-charts`, +`plugin-editor`) already include them — this establishes the pattern for the rest +rather than sweeping them. diff --git a/packages/types/package.json b/packages/types/package.json index d2490a583..bc287dca7 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -64,7 +64,7 @@ "scripts": { "build": "tsc", "clean": "rm -rf dist", - "type-check": "tsc --noEmit && tsc -p tsconfig.examples.json", + "type-check": "tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json", "lint": "eslint ." }, "keywords": [ diff --git a/packages/types/src/__tests__/app-creation-types.test.ts b/packages/types/src/__tests__/app-creation-types.test.ts index 3330b6a8d..4be21e89a 100644 --- a/packages/types/src/__tests__/app-creation-types.test.ts +++ b/packages/types/src/__tests__/app-creation-types.test.ts @@ -7,7 +7,7 @@ */ import { describe, it, expect } from 'vitest'; -import { isValidAppName, wizardDraftToAppSchema } from '@object-ui/types'; +import { isValidAppName, wizardDraftToAppSchema } from '../index'; import type { AppWizardDraft, AppWizardStep, @@ -15,7 +15,7 @@ import type { ObjectSelection, EditorMode, AppSchema, -} from '@object-ui/types'; +} from '../index'; describe('App Creation Types', () => { describe('isValidAppName', () => { diff --git a/packages/types/src/__tests__/p2-spec-exports.test.ts b/packages/types/src/__tests__/p2-spec-exports.test.ts index 8ae8fd385..827009f8d 100644 --- a/packages/types/src/__tests__/p2-spec-exports.test.ts +++ b/packages/types/src/__tests__/p2-spec-exports.test.ts @@ -17,26 +17,49 @@ * 2. Runtime Zod schemas from @objectstack/spec/ui validate data correctly */ import { describe, it, expect } from 'vitest'; +// Minimal fixtures below are parse INPUT, not parsed output: these schemas +// `.default()` several fields, so `{ enabled: true }` is valid input while the +// inferred output type requires them. Spec draws the same distinction itself +// (`ActionInput = z.input`). Typing them as the output type +// was wrong, and went unnoticed because nothing type-checked this file +// (objectstack#4074). +import type { z } from 'zod'; +// The `…Schema` names are deliberately NOT part of this package's surface: +// #2561 decision (a) dropped the spec/ui zod-validator re-exports, and +// `spec-ui-schema-reexports.test.ts` asserts their absence. This file went on +// importing eight of them as types for the whole interval, which no `tsc` run +// ever read (objectstack#4074) — a type-only import of a nonexistent name erases +// at runtime, so the suite stayed green while contradicting its own sibling +// guard. Only the value types are re-exported, so only they are imported here; +// the runtime validators come from `@objectstack/spec/ui` below, as #2561 +// prescribes. import type { SharingConfig, - SharingConfigSchema, EmbedConfig, - EmbedConfigSchema, AddRecordConfig, - AddRecordConfigSchema, AppearanceConfig, - AppearanceConfigSchema, UserActionsConfig, - UserActionsConfigSchema, ViewTab, - ViewTabSchema, ViewFilterRule, - ViewFilterRuleSchema, } from '../index'; -import type { - ThemeModeSchema, -} from '../theme'; +/** + * The type-level half of this file's contract (point 1 of the header): the + * non-`…Schema` value types ARE part of this package's surface — #2561 dropped + * only the zod validators — so importing them must keep compiling. Asserted here + * rather than left implicit in fixture annotations, because most fixtures are + * now typed as parse INPUT and stopped referencing these names. + */ +type _ReexportedValueTypes = [ + SharingConfig, + EmbedConfig, + AddRecordConfig, + AppearanceConfig, + UserActionsConfig, + ViewTab, + ViewFilterRule, +]; +void 0 as unknown as _ReexportedValueTypes; // Runtime Zod schemas are imported directly from the spec package import { @@ -62,7 +85,7 @@ describe('P2.3 Spec Protocol Type Re-exports — Sharing & Embedding', () => { }); it('should validate a minimal SharingConfig', () => { - const config: SharingConfig = { enabled: true }; + const config: z.input = { enabled: true }; const result = SharingConfigZod.safeParse(config); expect(result.success).toBe(true); }); @@ -89,7 +112,7 @@ describe('P2.3 Spec Protocol Type Re-exports — Sharing & Embedding', () => { }); it('should validate a minimal EmbedConfig', () => { - const config: EmbedConfig = { enabled: true }; + const config: z.input = { enabled: true }; const result = EmbedConfigZod.safeParse(config); expect(result.success).toBe(true); }); @@ -122,7 +145,7 @@ describe('P2.4 Spec Protocol Type Re-exports — View Configuration', () => { }); it('should validate a minimal AddRecordConfig', () => { - const config: AddRecordConfig = { enabled: true }; + const config: z.input = { enabled: true }; const result = AddRecordConfigZod.safeParse(config); expect(result.success).toBe(true); }); @@ -147,7 +170,7 @@ describe('P2.4 Spec Protocol Type Re-exports — View Configuration', () => { }); it('should validate a minimal AppearanceConfig', () => { - const config: AppearanceConfig = {}; + const config: z.input = {}; const result = AppearanceConfigZod.safeParse(config); expect(result.success).toBe(true); }); @@ -170,13 +193,13 @@ describe('P2.4 Spec Protocol Type Re-exports — View Configuration', () => { }); it('should validate a minimal UserActionsConfig', () => { - const config: UserActionsConfig = {}; + const config: z.input = {}; const result = UserActionsConfigZod.safeParse(config); expect(result.success).toBe(true); }); it('should validate a full UserActionsConfig', () => { - const config: UserActionsConfig = { + const config: z.input = { sort: true, search: true, filter: true, @@ -195,13 +218,15 @@ describe('P2.4 Spec Protocol Type Re-exports — View Configuration', () => { }); it('should validate a minimal ViewTab', () => { - const tab: ViewTab = { name: 'all', label: 'All Records' }; + const tab: z.input = { name: 'all', label: 'All Records' }; const result = ViewTabZod.safeParse(tab); expect(result.success).toBe(true); }); it('should validate a full ViewTab', () => { - const tab: ViewTab = { + // `operator: 'eq'` is a legacy alias spec folds onto `equals` at parse time, + // so it is valid INPUT and absent from the canonical output union. + const tab: z.input = { name: 'active', label: 'Active', icon: 'CheckCircle', @@ -229,7 +254,7 @@ describe('v3.0.10 Spec Protocol New Types', () => { }); it('should validate a ViewFilterRule', () => { - const rule: ViewFilterRule = { field: 'status', operator: 'eq', value: 'active' }; + const rule: z.input = { field: 'status', operator: 'eq', value: 'active' }; const result = ViewFilterRuleZod.safeParse(rule); expect(result.success).toBe(true); }); @@ -262,17 +287,17 @@ describe('Type re-exports from @object-ui/types index', () => { it('should allow type annotations with P2.3 Sharing & Embedding types', () => { // Compile-time check: these lines would fail to compile if types were not re-exported - const sharing: SharingConfig = { enabled: true }; - const embed: EmbedConfig = { enabled: false }; + const sharing: z.input = { enabled: true }; + const embed: z.input = { enabled: false }; expect(sharing.enabled).toBe(true); expect(embed.enabled).toBe(false); }); it('should allow type annotations with P2.4 View Configuration types', () => { - const addRecord: AddRecordConfig = { enabled: true }; + const addRecord: z.input = { enabled: true }; const appearance: AppearanceConfig = { showDescription: true }; - const userActions: UserActionsConfig = { sort: true }; - const tab: ViewTab = { name: 'main', label: 'Main' }; + const userActions: z.input = { sort: true }; + const tab: z.input = { name: 'main', label: 'Main' }; expect(addRecord.enabled).toBe(true); expect(appearance.showDescription).toBe(true); expect(userActions.sort).toBe(true); diff --git a/packages/types/src/__tests__/spec-derived-unions.test.ts b/packages/types/src/__tests__/spec-derived-unions.test.ts index bbdc25631..2c86c7651 100644 --- a/packages/types/src/__tests__/spec-derived-unions.test.ts +++ b/packages/types/src/__tests__/spec-derived-unions.test.ts @@ -27,14 +27,21 @@ * implements it, so a host app typing against @object-ui/types got an error * on working code. * - * The `satisfies` checks below document that contract at the type level — but they - * are NOT enforcement today, and the previous version of this comment claiming - * they were "the real enforcement" was wrong (#4074). Every package tsconfig - * excludes test files by glob (see `packages/types/tsconfig.json`) and there is no - * vitest `typecheck` project, so no `tsc` invocation reads this file at all. They would - * bite the day tests are type-checked; until then the RUNTIME assertions are the - * only thing that fails CI, which is why the #4074 cases below are written as - * identity and membership checks. + * The `satisfies` checks below ARE enforcement — but only since #3005 added + * `packages/types/tsconfig.test.json` and chained it from this package's + * `type-check` script. + * + * For the whole interval before that they were decorative, and an earlier version + * of this comment asserting they were "the real enforcement" was simply false: + * `tsconfig.json` excludes test files (it is the package build, with `rootDir`, + * `composite` and `declaration`, so tests would emit into dist), and no other + * `tsc` invocation read this file. Measured at the time: reverting a derived alias + * to its old hand-written fork produced ZERO type errors. It now produces + * `TS1360` pointing at the `satisfies` line below. + * + * The runtime assertions further down are still the stronger check for anything + * with a runtime witness — a type alias erases, so only reference identity + * distinguishes a re-export from a faithful copy. Keep both. * * The last case is the inverse: `navigation` is a name objectui runs that the * spec does NOT have. It is asserted absent from the spec so that the day the @@ -82,9 +89,6 @@ const _pageCovers = null as unknown as 'record' | 'home' | 'app' | 'utility' | ' const _vizCovers = null as unknown as PageVisualizationAlias satisfies PageType; const _runnableCovers = null as unknown as ActionType satisfies RunnableActionType; // #4074: the action sub-vocabularies that were restated rather than derived. -// `ActionComponent`'s real compile-time enforcement lives in `ui-action.ts`, -// which IS type-checked: `ActionSchema.component?: ActionComponent` stops -// compiling there if the derivation breaks. const _componentCovers = null as unknown as | 'action:button' | 'action:icon' diff --git a/packages/types/src/__tests__/system-fields.test.ts b/packages/types/src/__tests__/system-fields.test.ts index 48064b9df..52ea9d5d4 100644 --- a/packages/types/src/__tests__/system-fields.test.ts +++ b/packages/types/src/__tests__/system-fields.test.ts @@ -7,7 +7,7 @@ */ import { describe, it, expect } from 'vitest'; -import { isSystemManagedField, SYSTEM_MANAGED_FIELD_NAMES } from '@object-ui/types'; +import { isSystemManagedField, SYSTEM_MANAGED_FIELD_NAMES } from '../index'; describe('isSystemManagedField', () => { it('treats fields flagged `system: true` as system-managed (the single source of truth)', () => { diff --git a/packages/types/tsconfig.test.json b/packages/types/tsconfig.test.json new file mode 100644 index 000000000..0275bc8b1 --- /dev/null +++ b/packages/types/tsconfig.test.json @@ -0,0 +1,52 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // + // Why they were excluded, and why that was a hole: `tsconfig.json` is the + // package BUILD (`tsc` -> dist) with "rootDir": "./src", "composite" and + // "declaration", so test files would emit into dist. Excluding them keeps the + // build clean — but it also meant no `tsc` invocation ever read them, and + // `spec-derived-unions.test.ts` had built its whole contract on `satisfies` + // checks that therefore never ran. Its header claimed they were "the real + // enforcement"; reverting a derived alias to its old hand-written fork + // produced zero errors (objectstack#4074). + // + // Same reasoning and same shape as `tsconfig.examples.json`: a separate, + // emit-free project chained from the package's `type-check` script. Keeping it + // separate rather than deleting the exclude also keeps the BUILD honest — the + // "types": ["node"] below is needed by the reexport guard's `readFileSync` + // source scan, and folding that into `tsconfig.json` would let package SOURCE + // reference Node APIs and still compile, in a package that ships to browsers. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + "lib": ["ES2020", "DOM"], + // `spec-ui-schema-reexports.test.ts` reads sibling sources off disk to prove + // no spec/ui zod value hides inside an `export type` block (#2561). + "types": ["node"], + // Same reason as tsconfig.json: drop the root tsconfig's source-tree paths + // so @objectstack/spec resolves through the workspace dependency. + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"], + // ── Known gap, declared rather than silent ───────────────────────────────── + // `p1-spec-alignment.test.ts` has 14 type errors across 8 sites, and they are + // not hygiene: every one sits in a test named "should accept " whose + // whole purpose is to assert the type ACCEPTS that shape. The type rejects it, + // and the test passed anyway because nothing compiled the file. Two examples: + // + // - "should accept sharing in ObjectUI format { visibility, enabled }" — + // `ListViewSchema.sharing` is `{ type, lockedBy }` only. The legacy shape is + // real and IS handled, by `foldSharing` in core's `normalize-list-view.ts`, + // but as untyped input (`normalizeListViewSchema(schema: T): T`), so no + // type names it. + // - several minimal fixtures assigned to output types whose fields carry + // `.default()`, the same input/output confusion fixed in + // `p2-spec-exports.test.ts` in this change. + // + // Each one is a separate decision — widen the type so the test's claim becomes + // true, or delete the claim — and several touch the public surface. Doing that + // silently while landing the type-check plumbing would bury it, so the file is + // excluded here WITH this reason and tracked in objectstack#4074. Removing this + // entry is the definition of done; it must only ever shrink. + "exclude": ["src/__tests__/p1-spec-alignment.test.ts"] +}