diff --git a/.changeset/8604-record-details-columns-enum.md b/.changeset/8604-record-details-columns-enum.md new file mode 100644 index 0000000000..819689b212 --- /dev/null +++ b/.changeset/8604-record-details-columns-enum.md @@ -0,0 +1,46 @@ +--- +'@object-ui/types': minor +--- + +**BREAKING** — the body-wide `RecordDetailsComponentProps.columns` is the +contract's string enum, not a number. + +**FROM** `columns: 2` **TO** `columns: '2'`. + +```ts +// before — compiled here, refused at publish +const props: RecordDetailsComponentProps = { columns: 2 }; +// after +const props: RecordDetailsComponentProps = { columns: '2' }; +``` + +`@objectstack/spec` declares the top-level key as +`z.enum(['1','2','3','4']).default('2')` — a closed set of **string** literals. +This type declared `columns?: number`, the wrong primitive type rather than +merely a wider range, so `{ columns: 2 }` type-checked locally and the contract +refused it at publish with `invalid_value` at `columns`. Measured on the +installed pin, `@objectstack/spec` 17.4.0, against a control that fires on the +same instrument: `{ columns: '2' }` parses green and its value survives; the +declaration is byte-identical to the 17.3.0 the card was filed against. +Contract-first (Commandment #0.1): the code moves to the contract's spelling, +and the spec is not widened. + +⚠️ `sections[].columns` one level down is **unchanged and stays `number`**. The +per-section key is `z.number().int().min(1).max(4)`, so a section takes `2` and +refuses `'2'` — exactly inverting the body-wide key. The same word names two +different types one level apart, which is why this is not a sweep: copying +either declaration onto the other is refused at publish, in one direction or +the other. Both levels, and their non-equality, are pinned against the installed +spec in `record-details-columns-8604.test.ts`. + +⚠️ The census behind this narrowing covers this repository only: one in-repo +call site wrote the number (`p1-spec-alignment.test.ts`), and it is corrected in +the same change. A TypeScript consumer of `@object-ui/types` outside this repo +that wrote `columns: 2` is not observable from here and gets a compile error +(TS2322) naming the key — which is why the FROM/TO is spelled out above. Nothing +to migrate at runtime: the renderer passes the authored value straight through, +and the registry manifest (`@object-ui/plugin-detail`) already published this +key as `type: 'enum', enum: ['1','2','3','4']`, so the published TypeScript face +was the only layer that disagreed. + +objectui#8604. diff --git a/packages/types/src/__tests__/p1-spec-alignment.test.ts b/packages/types/src/__tests__/p1-spec-alignment.test.ts index 3f514cbf49..8159d50873 100644 --- a/packages/types/src/__tests__/p1-spec-alignment.test.ts +++ b/packages/types/src/__tests__/p1-spec-alignment.test.ts @@ -526,8 +526,15 @@ describe('P1.4 Page Composition Spec Alignment', () => { // ============================================================================ describe('P1.5 Record Components', () => { it('should define RecordDetailsComponentProps', () => { + // `columns` is the STRING `'2'`, not the number, since objectui#8604: the + // contract declares the body-wide key as `z.enum(['1','2','3','4'])` and + // refuses `2` with `invalid_value`. The number spelling this literal + // carried compiled here and was refused at publish — the exact defect the + // card was filed for. `sections[].columns` one level down keeps `number` + // (`z.number().int().min(1).max(4)`); the two are pinned side by side in + // `record-details-columns-8604.test.ts`. const props: RecordDetailsComponentProps = { - columns: 2, + columns: '2', layout: 'stacked', sections: [ { label: 'Basic Info', fields: ['name', 'email', 'phone'], collapsible: true }, @@ -536,7 +543,7 @@ describe('P1.5 Record Components', () => { fields: ['name', 'email'], aria: { ariaLabel: 'Account Details' }, }; - expect(props.columns).toBe(2); + expect(props.columns).toBe('2'); expect(props.sections).toHaveLength(2); expect(props.layout).toBe('stacked'); }); diff --git a/packages/types/src/__tests__/record-details-columns-8604.test.ts b/packages/types/src/__tests__/record-details-columns-8604.test.ts new file mode 100644 index 0000000000..9132e8e6c7 --- /dev/null +++ b/packages/types/src/__tests__/record-details-columns-8604.test.ts @@ -0,0 +1,196 @@ +/** + * 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. + */ + +/** + * objectui#8604 — the BODY-WIDE `RecordDetailsComponentProps.columns` is the + * contract's string enum, and `sections[].columns` one level down is still a + * number. One word, two types, one level apart. + * + * The defect: this package declared the top-level key as `columns?: number`, + * while `@objectstack/spec` declares it `z.enum(['1','2','3','4'])`. So + * `{ columns: 2 }` type-checked here and was refused at publish with + * `invalid_value` — a green local build and a rejection at the only point that + * matters. Direction is contract-first (Commandment #0.1, and triage's ruling + * on the card): the code moves to the contract, the contract is not widened. + * + * ⚠️ The near-miss this file exists to make un-repeatable: `sections[].columns` + * is `z.number().int().min(1).max(4)`, so `number` is CORRECT there — verified + * key-for-key on objectui#8583 / PR #8601. A fix that copied either + * declaration onto the other would be refused at publish in the opposite + * direction. Both levels are pinned below, in both directions, so a future + * "consistency" edit that unifies them turns this file red. + * + * Two instruments, deliberately: + * - `tsc` sees the `@ts-expect-error` legs and the `Equal` assertions. Those + * are the half that reaches a TypeScript author, and they mean nothing + * unless `type-check` runs — vitest strips types. + * - vitest runs the `safeParse` legs against the INSTALLED published spec + * artifact, each with a control that would have fired. + */ + +import { describe, it, expect } from 'vitest'; +import type { z } from 'zod'; +import { RecordDetailsProps } from '@objectstack/spec/ui'; +import type { RecordDetailsComponentProps } from '../record-components'; + +/** What an author writes for the spec's `record:details` props bag. */ +type SpecProps = z.input; + +/** One authored `sections[]` entry, on each of the two faces. */ +type Section = NonNullable[number]; +type SpecSection = NonNullable[number]; + +/** Invariant type equality. `A extends B` is NOT this: `never` and `any` pass that. */ +type Equal = + (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false; + +/** The only assertion form used here — its constraint is what refuses `false`. */ +type Expect = T; + +/* ── Direction proofs: a broken instrument makes THIS file red ─────────────── */ + +// @ts-expect-error objectui#8604 — `Expect` must refuse `false`. Widen its constraint and this directive goes unused (TS2578). +type _ExpectRefusesFalse = Expect; + +// @ts-expect-error objectui#8604 — `never` must NOT read as equal to `true`. An `extends`-shaped comparison would let it through. +type _EqualRefusesNever = Expect>; + +// @ts-expect-error objectui#8604 — `any` must NOT read as equal to `true`, for the same reason. +type _EqualRefusesAny = Expect>; + +/* ── The fix: the top-level key carries the contract's own authoring type ──── */ + +/** RED before objectui#8604 (`number` vs the string enum), green after. */ +type _TopLevelColumns = Expect< + Equal +>; + +/** The per-section key was already right, and must stay right. */ +type _SectionColumns = Expect>; + +/** + * The near-miss, asserted as a NON-equality: the two `columns` are different + * types. A future edit that "makes them consistent" — in either direction — + * turns this line red before it can reach publish. + */ +type _TwoLevelsDisagree = Expect< + Equal, false> +>; + +/* ── Literals: what a TypeScript author can and cannot write ───────────────── */ + +/** The contract's spelling compiles. */ +const bodyWidthAccepted: RecordDetailsComponentProps = { columns: '2' }; + +/** + * The card's repro. `2` compiled before objectui#8604 and was refused at + * publish; now `tsc` refuses it here, which is the whole point of the change. + * Revert the narrowing and this directive goes unused (TS2578). + */ +const bodyWidthNumberRefused: RecordDetailsComponentProps = { + // @ts-expect-error objectui#8604 — the body-wide `columns` is `'1'|'2'|'3'|'4'`, never the number `2`. + columns: 2, +}; + +/** The section key takes the number, and refuses the string — the inverse. */ +const sectionWidthAccepted: RecordDetailsComponentProps = { + sections: [{ fields: ['phone'], columns: 2 }], +}; + +const sectionWidthStringRefused: RecordDetailsComponentProps = { + // @ts-expect-error objectui#8604 — `sections[].columns` is `number`; the string spelling belongs to the body-wide key only. + sections: [{ fields: ['phone'], columns: '2' }], +}; + +/** A value outside the closed set is refused on both faces. */ +const bodyWidthOutOfRange: RecordDetailsComponentProps = { + // @ts-expect-error objectui#8604 — `'5'` is outside the contract's closed set. + columns: '5', +}; + +describe('objectui#8604 — record:details `columns`, both levels, against the installed spec', () => { + it('the PREMISE: the two levels really are declared with different primitive types', () => { + // Read off the live schema object rather than transcribed, so a spec that + // converges the two spellings fails HERE first — before the pins below + // start asserting a shape the contract no longer has. + const body = RecordDetailsProps.safeParse({ columns: '3' }); + const section = RecordDetailsProps.safeParse({ sections: [{ fields: ['a'], columns: 3 }] }); + expect(body.success).toBe(true); + expect(section.success).toBe(true); + + // And each refuses the other's spelling. Without this half, "different + // types" would be satisfied by a contract that accepted both everywhere. + expect(RecordDetailsProps.safeParse({ columns: 3 }).success).toBe(false); + expect( + RecordDetailsProps.safeParse({ sections: [{ fields: ['a'], columns: '3' }] }).success, + ).toBe(false); + }); + + it("the body-wide key refuses the NUMBER with `invalid_value` at `columns` — the card's repro", () => { + const refused = RecordDetailsProps.safeParse({ columns: 2 }); + expect(refused.success).toBe(false); + const issue = refused.error?.issues.find((i) => i.path.join('.') === 'columns'); + expect(issue).toBeDefined(); + expect(issue?.code).toBe('invalid_value'); + + // THE CONTROL, on the same instrument: the string the contract declares + // parses green and its value survives. A refusal with no control that + // would have fired is not a measurement — it is also what a schema that + // refused everything would produce. + const accepted = RecordDetailsProps.safeParse({ columns: '2' }); + expect(accepted.success).toBe(true); + expect(accepted.data?.columns).toBe('2'); + expect(typeof accepted.data?.columns).toBe('string'); + }); + + it('the body-wide key is a CLOSED set, and omitting it applies the schema default', () => { + expect(RecordDetailsProps.safeParse({ columns: '5' }).success).toBe(false); + expect(RecordDetailsProps.safeParse({ columns: '1' }).success).toBe(true); + expect(RecordDetailsProps.safeParse({ columns: '4' }).success).toBe(true); + + // `.default('2')` — an omitted key is not an absent one downstream, which + // is why the input face is optional while the output face is not. + const omitted = RecordDetailsProps.safeParse({}); + expect(omitted.success).toBe(true); + expect(omitted.data?.columns).toBe('2'); + }); + + it('`sections[].columns` takes the NUMBER, refuses the string, and bounds 1-4', () => { + const accepted = RecordDetailsProps.safeParse({ sections: [{ fields: ['phone'], columns: 2 }] }); + expect(accepted.success).toBe(true); + expect(accepted.data?.sections?.[0]?.columns).toBe(2); + + const stringRefused = RecordDetailsProps.safeParse({ + sections: [{ fields: ['phone'], columns: '2' }], + }); + expect(stringRefused.success).toBe(false); + const issue = stringRefused.error?.issues.find( + (i) => i.path.join('.') === 'sections.0.columns', + ); + expect(issue?.code).toBe('invalid_type'); + + // The range, so "number" is not mistaken for "any number". + expect( + RecordDetailsProps.safeParse({ sections: [{ fields: ['phone'], columns: 5 }] }).success, + ).toBe(false); + expect( + RecordDetailsProps.safeParse({ sections: [{ fields: ['phone'], columns: 0 }] }).success, + ).toBe(false); + }); + + it('the literals above are real values, not type-only decoration', () => { + // vitest strips types, so these expectations are NOT the assertion — the + // annotations are. They exist so the file also fails visibly if the + // literals are ever silently emptied out. + expect(bodyWidthAccepted.columns).toBe('2'); + expect(bodyWidthNumberRefused.columns as unknown).toBe(2); + expect(bodyWidthOutOfRange.columns as unknown).toBe('5'); + expect(sectionWidthAccepted.sections?.[0]?.columns).toBe(2); + expect(sectionWidthStringRefused.sections?.[0]?.columns as unknown).toBe('2'); + }); +}); diff --git a/packages/types/src/record-components.ts b/packages/types/src/record-components.ts index a3c22d5e10..f7ab7b3459 100644 --- a/packages/types/src/record-components.ts +++ b/packages/types/src/record-components.ts @@ -37,8 +37,29 @@ export interface RecordComponentAriaProps { * Aligned with @objectstack/spec RecordDetailsProps. */ export interface RecordDetailsComponentProps { - /** Number of columns for field layout (1-4) */ - columns?: number; + /** + * Field-grid width for the WHOLE body, as the STRING the contract declares + * (`@objectstack/spec` `RecordDetailsProps.columns` is + * `z.enum(['1','2','3','4'])`, schema default `'2'`). + * + * It was `number` here until objectui#8604, which is the wrong PRIMITIVE + * TYPE, not merely a wider range: `{ columns: 2 }` compiled locally and the + * contract refused it at publish with `invalid_value` at `columns` (measured + * on the installed pin, 17.4.0, against a control — `columns: '2'` — that + * parses green on the same instrument). Contract-first (Commandment #0.1): + * the code moves to the contract's spelling, and today's `columns: 2` + * authors are the defect surfacing rather than collateral damage. + * + * WARNING — this is NOT the spelling `sections[].columns` uses one level + * down. That key is `z.number().int().min(1).max(4)`, so a section takes the + * NUMBER `2` and refuses the string, exactly inverting this key. The same + * word names two different types one level apart; copying either declaration + * onto the other is refused at publish. See the `columns` member on the + * `sections[]` entry below, and + * `__tests__/record-details-columns-8604.test.ts`, which pins both directions + * against the installed spec. + */ + columns?: '1' | '2' | '3' | '4'; /** Detail layout mode */ layout?: 'stacked' | 'inline' | 'compact'; /** Sections to organize fields */ @@ -75,6 +96,13 @@ export interface RecordDetailsComponentProps { * and `DetailSection` derives the width from the field count. Permitted * beside `group`: it describes how this page lays the section out, not * anything the group itself declares. + * + * WARNING — `number` is correct HERE and only here (objectui#8604): the + * per-section key is `z.number().int().min(1).max(4)`, while the body-wide + * `columns` at the top of this interface is a string enum. A section + * carrying `columns: '2'` is refused with `invalid_type` at + * `sections.N.columns`; the top-level key refuses `2`. Two types, one word, + * one level apart. */ columns?: number; /**