From 7fe11e01a24d26883f349940649f32fc55f389eb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 06:03:24 +0000 Subject: [PATCH] fix(types): StackSchema ships its declared members instead of collapsing under BaseSchema's index signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `StackSchema` was declared `extends Omit` and shipped a declaration carrying exactly one property, `type`. `Omit` is `Pick>`, and `keyof T` on a type with a string index signature is `string | number` — the literal member names are absorbed. `FlexSchema` inherits `BaseSchema`'s `[key: string]: any` (#5155), so the `Pick` rebuilt a type with the index signature and none of the 25 named members. Nothing errored, because the index signature answers every absent key with `any`. The cost fell on the tools that read the declaration: editor completion on a stack node offered `type` alone, and #6143's docs sweep read `stack.mdx`'s `gap`/`children`/`className` as documenting keys that do not exist. Fixed at the mechanism: the six flex/stack members move to a new exported interface `FlexLayoutProps`, which does not inherit `BaseSchema`. `FlexSchema` and `StackSchema` each extend `BaseSchema, FlexLayoutProps`, so no `Omit` crosses the index signature and the members are declared once. Extending `FlexSchema` directly is unavailable — `'stack'` is not a subtype of `type: 'flex'` (TS2430, measured). `FlexSchema` is unchanged: its member declarations moved byte-identically and its emitted member set is the same 25 names. Guarded by a pin that reads the EMITTED declaration, not the source — a source-level assertion passes on the broken code because the index signature answers with `any`, and that gap is the defect. Part of #6151 --- .changeset/6151-stack-schema-omit-collapse.md | 52 ++++ .../stack-schema-emitted-members.test.ts | 233 ++++++++++++++++++ packages/types/src/index.ts | 1 + packages/types/src/layout.ts | 54 +++- 4 files changed, 336 insertions(+), 4 deletions(-) create mode 100644 .changeset/6151-stack-schema-omit-collapse.md create mode 100644 packages/types/src/__tests__/stack-schema-emitted-members.test.ts diff --git a/.changeset/6151-stack-schema-omit-collapse.md b/.changeset/6151-stack-schema-omit-collapse.md new file mode 100644 index 0000000000..92f2ab2c8a --- /dev/null +++ b/.changeset/6151-stack-schema-omit-collapse.md @@ -0,0 +1,52 @@ +--- +'@object-ui/types': minor +--- + +`StackSchema` now SHIPS the members it declares (objectui#6151). Its emitted declaration +carried one property — `type` — where it was meant to carry twenty-five. + +The interface was written `extends Omit`: "everything `FlexSchema` has, +with a different `type`". That spelling erases every named member. `Omit` of a type over a +key set is `Pick` over `Exclude` of `keyof` that type, and `keyof` a type carrying a string +index signature is `string | number` — the literal member names are absorbed. `FlexSchema` +inherits `BaseSchema`'s `[key: string]: any` (objectui#5155), so excluding `'type'` from +`string | number` still leaves `string | number`, and the `Pick` rebuilt a type holding the +index signature and none of the named members. Measured against the built `dist`: +`FlexSchema` declared 25 properties, `StackSchema` declared 1. + +Nothing errored, which is why it survived four releases: the index signature keeps every +absent key assignable and readable as `any`. The cost fell entirely on the tools that READ +the declaration. Editor completion on a `stack` node offered `type` and nothing else — no +`gap`, no `align`, no `justify`, no `children`. And a docs-versus-type sweep read +`stack.mdx` as documenting keys that do not exist: objectui#6143 flagged `gap`, `children` +and `className` there as divergences when the docs were right and the type was wrong. + +Fixed at the mechanism rather than by restating the members. The six flex/stack members now +live in a new exported interface, `FlexLayoutProps`, which does NOT inherit `BaseSchema`, +and `FlexSchema` and `StackSchema` each extend `BaseSchema` and `FlexLayoutProps`. No +`Omit` crosses the index signature any more, and the members are declared once rather than +duplicated. Extending `FlexSchema` directly was measured unavailable: an interface may +narrow an inherited property only to a subtype, and `'stack'` is not a subtype of +`FlexSchema`'s `type: 'flex'` (TS2430). + +`FlexSchema` is unchanged — its six member declarations moved byte-identically, and its +emitted member set is the same 25 names before and after. The only declaration whose shape +changes is `StackSchema`, which goes from 1 property to the same 25. + +**The one way this can newly error**, and why it ships as `minor`: keys on a `stack` node +were previously answered by the index signature as `any`, so `gap: 'large'` type-checked. +`gap` is now `number | undefined` and that line is a `tsc` error. Every value this newly +rejects is one the renderer never honoured — `stack.tsx` feeds `gap` to a Tailwind numeric +scale — so the change reports a defect that was already there rather than removing a +capability. All three in-repo packages that name `StackSchema` or `FlexSchema` +(`@object-ui/components`, `@object-ui/core`, the schema-catalog example) type-check green +unchanged. + +Guarded by `packages/types/src/__tests__/stack-schema-emitted-members.test.ts`, which +asserts against the EMITTED declaration rather than the source. That distinction is the +whole point: a source-level assertion passes on the broken code, because the index +signature answers for the missing key with `any`. The guard emits declarations with the +package's own tsconfig and asserts (1) `StackSchema` declares exactly what `FlexSchema` +declares, and (2) no member of the `LayoutSchema` union has lost any of `BaseSchema`'s +named members — so the next heritage clause that collapses under the index signature reds +for the whole class, not just for this one interface. diff --git a/packages/types/src/__tests__/stack-schema-emitted-members.test.ts b/packages/types/src/__tests__/stack-schema-emitted-members.test.ts new file mode 100644 index 0000000000..ceaef3e240 --- /dev/null +++ b/packages/types/src/__tests__/stack-schema-emitted-members.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Every `LayoutSchema` member SHIPS the members it declares (objectui#6151). + * + * ## The defect + * + * `StackSchema` was declared `extends Omit` — "everything + * `FlexSchema` has, with a different `type`". The shipped declaration carried + * only `type`. + * + * `Omit` is `Pick>`, and `keyof T` on a type + * carrying a string index signature is `string | number` — the literal member + * names are ABSORBED. `FlexSchema` inherits `BaseSchema`'s `[key: string]: any` + * (objectui#5155), so `Exclude` is still + * `string | number`, and the `Pick` reconstructed a type with the index + * signature and none of the named members. Measured on the emitted + * `dist/layout.d.ts` before the fix: + * + * FlexSchema -> 25 declared properties + * StackSchema -> 1 declared property: type + * + * `gap`, `children`, `align`, `justify`, `direction`, `wrap` and all 19 other + * `BaseSchema` members were absent. Nothing errored — the index signature keeps + * every absent key assignable and readable as `any` — so the only symptom was + * in the tools that READ the declaration: editor completion on a `stack` node + * offered `type` and nothing else, and objectui#6143's docs-vs-type sweep read + * `stack.mdx`'s `gap` / `children` / `className` as documenting keys that do not + * exist. The docs were right; the type was wrong. + * + * ## Why this reads the EMITTED declaration and not the source + * + * ⚠️ This is the load-bearing part of the guard. A source-level type assertion + * (`Expect>`) passes on the broken + * code: the index signature answers for `gap` with `any`, and `any` satisfies + * everything. The gap between "what the source says" and "what the `.d.ts` + * declares" IS this bug, so a guard that never opens the `.d.ts` cannot see it. + * + * ## Why it emits its own declarations instead of reading `dist/` + * + * This repo's per-PR `test` job runs `pnpm test` with NO build step ahead of it + * (turbo's `test` task only `dependsOn: ["^build"]` — the DEPENDENCY closure, + * never the package's own build), and `packages/types` has no workspace + * dependencies, so nothing builds it. A guard that read `dist/layout.d.ts` would + * be absent-or-stale on a cold CI cache — vacuous exactly where it is needed. + * The same trap is recorded in `package-exports-manifest.test.ts`'s header. + * + * So this file runs the package's OWN tsconfig through the compiler API and + * emits declarations to a scratch directory, then measures the result with the + * checker. That is the artifact a consumer resolves, derived deterministically + * and with no dependence on CI job ordering. + * + * ## What each assertion catches + * + * 1. non-vacuity — the emit really produced a declaration the checker can + * read, and `FlexSchema` in it carries its six flex members. Without this, + * a broken emit would make every assertion below pass over an empty set. + * 2. the measurement — `StackSchema` declares EXACTLY what `FlexSchema` + * declares. Set equality, not a spot check: it fails both when a member is + * erased again and when the two drift apart. + * 3. the class tripwire — every member of the `LayoutSchema` union declares + * all of `BaseSchema`'s named members. Each one extends `BaseSchema`, so + * any future heritage clause that crosses a mapped type over an + * index-signature-bearing type reds here, not only the one this card fixed. + * Sixteen union members satisfy it today; `StackSchema` was the one that + * did not. + */ + +import { describe, it, expect, afterAll } from 'vitest'; +import { mkdtempSync, rmSync, existsSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; + +const packageRoot = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); + +/** + * Emit declarations with the package's OWN build settings, into a scratch dir + * under `node_modules/` — which is gitignored, and from which Node's module + * resolution still walks up to `packages/types/node_modules`, so the emitted + * `import type … from '@objectstack/spec/ui'` still resolves. + */ +function emitDeclarations(): { dir: string; layout: string } { + const configPath = join(packageRoot, 'tsconfig.json'); + const readConfig = ts.readConfigFile(configPath, ts.sys.readFile); + if (readConfig.error) { + throw new Error(ts.flattenDiagnosticMessageText(readConfig.error.messageText, '\n')); + } + const parsed = ts.parseJsonConfigFileContent(readConfig.config, ts.sys, packageRoot); + + const dir = mkdtempSync(join(packageRoot, 'node_modules', '.stack-schema-pin-')); + const program = ts.createProgram([join(packageRoot, 'src', 'layout.ts')], { + ...parsed.options, + outDir: dir, + declaration: true, + emitDeclarationOnly: true, + declarationMap: false, + noEmit: false, + // The real build is `composite`/incremental; neither is meaningful for a + // one-shot emit into a scratch dir, and both would write build info next to + // the package's real artifacts. + composite: false, + incremental: false, + tsBuildInfoFile: undefined, + }); + const emitted = program.emit(); + const layout = join(dir, 'layout.d.ts'); + if (!existsSync(layout)) { + const diagnostics = [...emitted.diagnostics, ...program.getSemanticDiagnostics()] + .map((d) => ts.flattenDiagnosticMessageText(d.messageText, '\n')) + .slice(0, 10); + throw new Error(`declaration emit produced no layout.d.ts:\n${diagnostics.join('\n')}`); + } + return { dir, layout }; +} + +const { dir: scratchDir, layout: emittedLayout } = emitDeclarations(); +afterAll(() => rmSync(scratchDir, { recursive: true, force: true })); + +const program = ts.createProgram([emittedLayout], { + noEmit: true, + skipLibCheck: true, + strict: true, + target: ts.ScriptTarget.ESNext, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, +}); +const checker = program.getTypeChecker(); + +function moduleExports(file: string): Map { + const sourceFile = program.getSourceFile(file); + const moduleSymbol = sourceFile && checker.getSymbolAtLocation(sourceFile); + if (!moduleSymbol) throw new Error(`no module symbol for ${file}`); + return new Map(checker.getExportsOfModule(moduleSymbol).map((s) => [s.getName(), s])); +} + +/** The property names the EMITTED declaration of `name` declares. */ +function declaredMembers(name: string): string[] { + const symbol = moduleExports(emittedLayout).get(name); + if (!symbol) throw new Error(`${name} is not exported from the emitted layout.d.ts`); + return checker + .getPropertiesOfType(checker.getDeclaredTypeOfSymbol(symbol)) + .map((p) => p.getName()) + .sort(); +} + +/** `BaseSchema`'s named members, read from the emitted `base.d.ts` beside it. */ +function baseSchemaMembers(): string[] { + const base = join(scratchDir, 'base.d.ts'); + const symbol = moduleExports(base).get('BaseSchema'); + if (!symbol) throw new Error('BaseSchema is not exported from the emitted base.d.ts'); + return checker + .getPropertiesOfType(checker.getDeclaredTypeOfSymbol(symbol)) + .map((p) => p.getName()) + .sort(); +} + +/** The member interfaces of the emitted `LayoutSchema` union, by name. */ +function layoutUnionMembers(): { name: string; members: string[] }[] { + const symbol = moduleExports(emittedLayout).get('LayoutSchema'); + if (!symbol) throw new Error('LayoutSchema is not exported from the emitted layout.d.ts'); + const union = checker.getDeclaredTypeOfSymbol(symbol); + const parts = union.isUnion() ? union.types : [union]; + return parts.map((t) => ({ + name: t.symbol?.getName() ?? '', + members: checker.getPropertiesOfType(t).map((p) => p.getName()).sort(), + })); +} + +/* ── 1. Non-vacuity ──────────────────────────────────────────────────────── */ + +describe('the emitted declaration is readable (guards every assertion below)', () => { + it('emits a layout.d.ts whose FlexSchema carries its six flex members', () => { + const flex = declaredMembers('FlexSchema'); + // If the emit collapsed or the checker read the wrong file, this is where it + // shows — the assertions below all compare against this same population. + expect(flex).toEqual(expect.arrayContaining([ + 'align', 'direction', 'gap', 'justify', 'wrap', 'children', + ])); + expect(flex.length).toBeGreaterThan(20); + }); + + it('reads a plausible BaseSchema member set', () => { + expect(baseSchemaMembers().length).toBeGreaterThan(15); + }); +}); + +/* ── 2. The measurement ──────────────────────────────────────────────────── */ + +describe('StackSchema ships the members it declares (objectui#6151)', () => { + it('declares EXACTLY what FlexSchema declares', () => { + // Before the fix: StackSchema declared ['type'] against FlexSchema's 25. + expect(declaredMembers('StackSchema')).toEqual(declaredMembers('FlexSchema')); + }); + + it.each(['gap', 'children', 'align', 'justify', 'direction', 'wrap', 'className'])( + 'declares `%s` in the emitted declaration, not merely via the index signature', + (member) => { + // `getPropertyOfType` returned undefined for every one of these before the + // fix, while `StackSchema['gap']` in SOURCE resolved to `any` and hid it. + expect(declaredMembers('StackSchema')).toContain(member); + }, + ); + + it('still discriminates the union — `type` is the stack literal', () => { + const symbol = moduleExports(emittedLayout).get('StackSchema'); + const type = checker.getDeclaredTypeOfSymbol(symbol!); + const typeProp = checker.getPropertyOfType(type, 'type'); + expect(typeProp).toBeDefined(); + expect(checker.typeToString(checker.getTypeOfSymbol(typeProp!))).toBe('"stack"'); + }); +}); + +/* ── 3. The class tripwire ───────────────────────────────────────────────── */ + +describe('no LayoutSchema member loses BaseSchema’s members to a mapped type', () => { + it('every union member declares all of BaseSchema’s named members', () => { + const base = baseSchemaMembers(); + const collapsed = layoutUnionMembers() + .map(({ name, members }) => ({ name, missing: base.filter((b) => !members.includes(b)) })) + .filter(({ missing }) => missing.length > 0); + + // Every LayoutSchema member extends BaseSchema, so a missing BaseSchema + // member means the heritage clause crossed a mapped type and collapsed — + // `Omit`, `Pick`, or anything else built on `keyof`. + expect(collapsed, `member erasure in the emitted declaration: ${JSON.stringify(collapsed)}`) + .toEqual([]); + }); + + it('checks a plausible number of union members (non-vacuity)', () => { + expect(layoutUnionMembers().length).toBeGreaterThan(10); + }); +}); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 79062b8e51..819c46cbc6 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -124,6 +124,7 @@ export type { IconSchema, SeparatorSchema, ContainerSchema, + FlexLayoutProps, FlexSchema, StackSchema, GridSchema, diff --git a/packages/types/src/layout.ts b/packages/types/src/layout.ts index fb017e81e4..be77666f3b 100644 --- a/packages/types/src/layout.ts +++ b/packages/types/src/layout.ts @@ -185,10 +185,45 @@ export interface ContainerSchema extends BaseSchema { } /** - * Flexbox layout component + * The flex/stack layout members, declared ONCE and free of `BaseSchema`. + * + * This interface exists so that `StackSchema` can be "everything `FlexSchema` + * has, with a different `type`" WITHOUT crossing an `Omit` over a type that + * carries an index signature (objectui#6151). + * + * `StackSchema` used to be spelled `extends Omit`. That + * erased every named member from the SHIPPED declaration, silently: + * `Omit` is `Pick>`, and `keyof T` on a type + * carrying a string index signature is `string | number` — the literal member + * names are absorbed. `FlexSchema` inherits `BaseSchema`'s `[key: string]: any` + * (objectui#5155), so `Exclude` is still + * `string | number`, and the `Pick` reconstructed a type with the index + * signature and NONE of the named members. Measured against the emitted + * `dist/layout.d.ts`: `FlexSchema` declared 25 properties, `StackSchema` + * declared 1 (`type`) — `gap`, `children`, `align`, `justify`, `direction` and + * `wrap` were all absent, along with all 19 of `BaseSchema`'s other named + * members. + * + * Nothing errored, which is why it survived: the index signature made every + * absent key still assignable and still readable as `any`. What it cost was + * every tool that reads the declaration — editor completion on a `stack` node + * offered `type` and nothing else, and a docs-vs-type sweep read `stack.mdx` as + * documenting keys that do not exist (objectui#6143 flagged `gap`, `children` + * and `className` as divergences; the docs were right and the type was wrong). + * + * Extending FlexSchema directly instead is not available: an interface may + * narrow an inherited property only to a subtype, and `'stack'` is not a + * subtype of `FlexSchema`'s `type: 'flex'` — measured, TS2430 + * (`Interface 'StackSchema' incorrectly extends interface 'FlexSchema'. + * Types of property 'type' are incompatible.`). Lifting the shared members out + * of the inheritance path is what keeps them nameable from both sides. + * + * Pinned by `__tests__/stack-schema-emitted-members.test.ts`, which asserts + * against the EMITTED declaration rather than this source — a source-level + * assertion passes while the emitted declaration is empty, and that gap is + * exactly the defect. */ -export interface FlexSchema extends BaseSchema { - type: 'flex'; +export interface FlexLayoutProps { /** * Flex direction * @default 'row' @@ -220,10 +255,21 @@ export interface FlexSchema extends BaseSchema { children?: SchemaNode | SchemaNode[]; } +/** + * Flexbox layout component + */ +export interface FlexSchema extends BaseSchema, FlexLayoutProps { + type: 'flex'; +} + /** * Stack layout component (Vertical Flex shortcut) + * + * Declares the same members as {@link FlexSchema} — see {@link FlexLayoutProps} + * for why they are shared through a third interface rather than derived with an + * `Omit` (objectui#6151). */ -export interface StackSchema extends Omit { +export interface StackSchema extends BaseSchema, FlexLayoutProps { type: 'stack'; }