From a28031749108eaa281c1ba3d94fe60916b2f570e Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Sun, 9 Aug 2026 11:43:00 +0200 Subject: [PATCH] fix: type merged fields as child-wins, matching the runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `extend` merges fields with `{ ...parent.fields, ...nextFields }`, so a variant redeclaring an inherited field wins. The types said `S & S2`, which typed that key as both brands while the schema held is the child's alone — the lie already retired for the computed map. Adds `MergedFields = Omit & S2` and uses it at `extend`'s return type and at its `computed` / `invariants` input positions, so a rule's `d` reads a redeclared field honestly too. Named and exported, as `MergedComputed` had to be: inline, the 5.9.3 emitter copies the type parameter through unsubstituted (`TS2304`). Measured against the deferral reason on record: the emitter writes the alias by reference, so the billing fixture's `index.d.ts` grew 84 bytes across two variants and all four consumer typecheck steps stay clean. Also corrects two comments in `base.test-d.ts` — including the pre-existing computed one this change copied — that claimed a plain intersection would break the positive assertion. Measured: an intersection is assignable to either constituent, so both lines compile and the regression surfaces as the `@ts-expect-error` going unused (`TS2578`). Documents the fields merge in `declaration.md`'s table and its honest-surface caveat, and updates the declaration-emit name count in `types.md` and `CLAUDE.md`. --- .changeset/merged-fields.md | 26 +++++++++++ CLAUDE.md | 5 ++- docs/reference/declaration.md | 52 +++++++++++----------- docs/reference/types.md | 25 +++++++---- docs/typedoc.json | 1 + examples/billing-domain/src/emit-guards.ts | 3 ++ packages/entity/src/base.spec.ts | 18 ++++++++ packages/entity/src/base.test-d.ts | 29 +++++++++++- packages/entity/src/entity.ts | 4 ++ packages/entity/src/index.ts | 5 +++ packages/entity/src/types.ts | 46 ++++++++++++++----- 11 files changed, 166 insertions(+), 48 deletions(-) create mode 100644 .changeset/merged-fields.md diff --git a/.changeset/merged-fields.md b/.changeset/merged-fields.md new file mode 100644 index 0000000..902c156 --- /dev/null +++ b/.changeset/merged-fields.md @@ -0,0 +1,26 @@ +--- +"@btravstack/entity": minor +--- + +Type a root's merged field map as child-wins, matching the runtime. + +`Root.extend(tag)(fields)` merges fields with `{ ...parent.fields, ...nextFields }`, so +a variant redeclaring an inherited field wins. The types said `S & S2`, which typed +that key as both brands at once while the schema held was the child's alone — +the same lie already fixed for the `computed` map. The merge is now +`MergedFields` — `Omit & S2` — at `extend`'s return type and at +its `computed` and `invariants` input positions, so a rule's `d` reads a redeclared +field honestly too. + +Nothing changes at runtime, and no entity _declaration_ that compiled stops +compiling — the change is confined to what `extend` reports for a redeclared key. +Code **consuming** such a key is what may break: an assignment relying on the +_root's_ brand there was always unsound, since the value never carried that brand, +and it now fails to compile instead of passing silently. As with `computed`, the honest +surfaces are `Entity.Output`, `toJSON()` and `output.shape` — an _instance_ still +reads as the intersection, because a root's instance type reaches a variant +unmapped (`TS2425`). + +`MergedFields` is exported at the top level, and as `Entity.MergedFields`, for the +reason `MergedComputed` is: written inline, the 5.9.3 emitter copies the type +parameter through unsubstituted and a consumer's declarations fail with `TS2304`. diff --git a/CLAUDE.md b/CLAUDE.md index 6cee9ca..1a03232 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -213,9 +213,10 @@ design — `contract.spec.ts` pins that both ways. library can be "done". Resist convenience aliases. - **`index.ts` exports `Entity`, and nothing else you write against.** A bare `computed` or `union` is too generic to take from a consumer's import scope, - so everything hangs off the builder. The sole exception is the seven + so everything hangs off the builder. The sole exception is the nine declaration-emit names — `AbstractEntity`, `BaseInstance`, `ConstructionKey`, - `EntityStatic`, `EntityUnion`, `Sealed`, `UnionMember` — exported at the top + `EntityStatic`, `EntityUnion`, `MergedComputed`, `MergedFields`, `Sealed`, + `UnionMember` — exported at the top level as well: a downstream library compiling with `declaration: true` emits the _underlying_ name, not the namespace path aliasing it, so hiding them fails the consumer pass with diff --git a/docs/reference/declaration.md b/docs/reference/declaration.md index f6b04d0..966a097 100644 --- a/docs/reference/declaration.md +++ b/docs/reference/declaration.md @@ -257,15 +257,17 @@ class Personal extends AccountBase.extend("Personal")({ } ``` -Options **accumulate**, root-then-variant. A variant adds to what it inherits -and cannot shed it, so it is never quietly laxer than its root. - -| Option | How a variant's declaration meets the root's | -| ------------ | ------------------------------------------------------------- | -| `generated` | concatenated, root-then-variant | -| `immutable` | concatenated, root-then-variant | -| `invariants` | concatenated, root-then-variant | -| `computed` | merged **per key** — a repeated key takes the variant's entry | +A variant's declaration **accumulates** onto the root's, root-then-variant. It +adds to what it inherits and cannot shed it, so it is never quietly laxer than +its root. + +| Declaration part | How a variant's declaration meets the root's | +| ---------------- | -------------------------------------------------------------- | +| `fields` | merged **per key** — a repeated key takes the variant's schema | +| `generated` | concatenated, root-then-variant | +| `immutable` | concatenated, root-then-variant | +| `invariants` | concatenated, root-then-variant | +| `computed` | merged **per key** — a repeated key takes the variant's entry | A variant names only what it adds. `Personal` above declares no options and inherits everything `AccountBase` declared; a variant declaring @@ -280,22 +282,22 @@ The key lists are not deduplicated, and do not need to be. Each is turned into a keyed lookup before it reaches a schema or a patch check, so naming a key the root already declared is harmless. -`computed` merges per key rather than concatenating, because it is a map. A -variant can add a derived field beside the root's, and can **redefine** one the -root declared — its schema and its derivation replace that entry alone — but -cannot drop one. - -Redefining an inherited computed key has one edge, measured. The variant's -derivation is what runs, and every surface read off the declaration agrees with -it: `Variant.output.shape`, `toJSON()` and `Entity.Output` all -carry the variant's schema. The **instance property** does not — it keeps the -root's type intersected in, so a key the root branded `Upper` and the variant -rebranded `Label` reads as `Upper & Label` on an instance, and is still -assignable where the root's brand is expected. The root's instance type is -intersected into every variant **unmapped**, and subtracting a key from it is -exactly what `TS2425` forbids: any mapped form turns the root's methods into -function-typed properties and breaks every variant implementing an `abstract` -member. There is no fix pending; read the field off +`fields` and `computed` merge per key rather than concatenating, because both +are maps. A variant can add to either beside what the root declared, and can +**redeclare** an entry the root declared — its schema, and for `computed` its +derivation, replace that entry alone — but cannot drop one. + +Redeclaring an inherited key, in either map, has one edge, measured. The +variant's schema is what validates and its derivation is what runs, and every +surface read off the declaration agrees: `Variant.output.shape`, `toJSON()` and +`Entity.Output` all carry the variant's schema. The **instance +property** does not — it keeps the root's type intersected in, so a key the root +branded `Upper` and the variant rebranded `Label` reads as `Upper & Label` on an +instance, and is still assignable where the root's brand is expected. The root's +instance type is intersected into every variant **unmapped**, and subtracting a +key from it is exactly what `TS2425` forbids: any mapped form turns the root's +methods into function-typed properties and breaks every variant implementing an +`abstract` member. There is no fix pending; read the key off `Entity.Output` where its exact type matters. `extend` lives only on a root. The entity it returns is final. diff --git a/docs/reference/types.md b/docs/reference/types.md index 49dba7a..1e08761 100644 --- a/docs/reference/types.md +++ b/docs/reference/types.md @@ -1,6 +1,6 @@ --- title: Helper types -description: Entity.Input, Entity.Output, Entity.CreateInput, Entity.Patch, Entity.Instance — and the seven declaration-emit names exported at the top level. +description: Entity.Input, Entity.Output, Entity.CreateInput, Entity.Patch, Entity.Instance — and the nine declaration-emit names exported at the top level. --- # Helper types @@ -47,14 +47,14 @@ surrounding declaration. ## The declaration-emit names -Eight types are exported at the top level: `AbstractEntity`, `BaseInstance`, -`ConstructionKey`, `EntityStatic`, `EntityUnion`, `MergedComputed`, `Sealed`, -`UnionMember`. They are the one exception to the single-import rule, and none of -them is part of the API you write against. Seven also have namespace aliases for -anyone annotating by hand — `Entity.Abstract`, `Entity.BaseInstance`, -`Entity.ConstructionKey`, `Entity.MergedComputed`, `Entity.Sealed`, -`Entity.Static`, `Entity.Union` — but a consumer's _emitted declarations_ use the -top-level names. +Nine types are exported at the top level: `AbstractEntity`, `BaseInstance`, +`ConstructionKey`, `EntityStatic`, `EntityUnion`, `MergedComputed`, +`MergedFields`, `Sealed`, `UnionMember`. They are the one exception to the +single-import rule, and none of them is part of the API you write against. Eight +also have namespace aliases for anyone annotating by hand — `Entity.Abstract`, +`Entity.BaseInstance`, `Entity.ConstructionKey`, `Entity.MergedComputed`, +`Entity.MergedFields`, `Entity.Sealed`, `Entity.Static`, `Entity.Union` — but a +consumer's _emitted declarations_ use the top-level names. ```ts import type { @@ -64,6 +64,7 @@ import type { EntityStatic, EntityUnion, MergedComputed, + MergedFields, Sealed, UnionMember, } from "@btravstack/entity"; @@ -97,6 +98,12 @@ top-level name. What each one buys was measured, not assumed: position correctly, so only downstream builds saw it. Naming it is half the fix and exporting it is the other half: unexported, the emitter expands the alias structurally again and the identical dangling `A2` comes back. +- **`MergedFields`** — the same merge for the _field_ map, which `extend` hands + `EntityStatic` as its `S`. The runtime spreads parent-then-child, so a variant + redeclaring an inherited field wins; typed as `S & S2` that key read as both + brands at once. Named and exported from the start rather than measured into + existence a second time — inline, it carries `MergedComputed`'s hazard with + `S2` in place of `A2`. - **`EntityUnion`, `UnionMember`** — the same story for `Entity.union(...)` assigned to an exported `const`: without a top-level name the members expand structurally and reach `$brand`, failing with diff --git a/docs/typedoc.json b/docs/typedoc.json index c45a87b..18426fe 100644 --- a/docs/typedoc.json +++ b/docs/typedoc.json @@ -30,6 +30,7 @@ "InvariantSrc", "IsNominalField", "MergedComputedSrc", + "MergedFieldsSrc", "OnlyNominal", "OutputOf", "PatchOf", diff --git a/examples/billing-domain/src/emit-guards.ts b/examples/billing-domain/src/emit-guards.ts index 27dec29..2b9d262 100644 --- a/examples/billing-domain/src/emit-guards.ts +++ b/examples/billing-domain/src/emit-guards.ts @@ -103,6 +103,9 @@ export type Root = Entity.Abstract< never >; export type Merged = Entity.MergedComputed<{ label: typeof DisplayLabel }, Record>; +// The `Record` second argument is the shape that found the +// dangling `A2` (`TS2304`): it is what the *omitted* side collapses to. +export type MergedFieldMap = Entity.MergedFields<{ total: typeof Money }, Record>; /** The error is reachable as both a value and a type. */ export const isInvalid = (error: unknown): error is Entity.InvalidEntity => diff --git a/packages/entity/src/base.spec.ts b/packages/entity/src/base.spec.ts index 2236354..835e529 100644 --- a/packages/entity/src/base.spec.ts +++ b/packages/entity/src/base.spec.ts @@ -284,6 +284,24 @@ test("generated accumulates, so a variant cannot make a root's key caller-suppli expect(Object.keys(Doc.createInput.shape).toSorted()).toEqual(["note"]); }); +test("a variant redeclaring a field replaces the root's schema for that key", () => { + // The two schemas accept overlapping but neither-contains-the-other sets, so + // all three candidate merges are told apart rather than only two of them. + const Code5 = z.string().length(5).brand("Code5"); + const Digits = z.string().regex(/^\d+$/).brand("Digits"); + abstract class Coded extends Entity.abstract("Coded")({ id: AccountId, code: Code5 }) {} + class Numbered extends Coded.extend("Numbered")({ code: Digits }) {} + + // accepted by the root, rejected by the variant — rules out parent-wins + expect(Numbered.make({ id, code: "abcde" }).isErr()).toBe(true); + // rejected by the root, accepted by the variant — rules out an intersection, + // which is the merge this key's *type* used to claim + expect(Numbered.make({ id, code: "42" }).getOrThrow().code).toBe("42"); + // accepted by both, so the key is not simply dropped + expect(Numbered.make({ id, code: "12345" }).getOrThrow().code).toBe("12345"); + expect(Object.keys(Numbered.output.shape).toSorted()).toEqual(["code", "id"]); +}); + test("a variant redefining one computed key overrides that entry only", () => { class Louder extends AccountBase.extend("Louder")( { note: Label }, diff --git a/packages/entity/src/base.test-d.ts b/packages/entity/src/base.test-d.ts index ce8603f..566fa45 100644 --- a/packages/entity/src/base.test-d.ts +++ b/packages/entity/src/base.test-d.ts @@ -134,8 +134,11 @@ test("a redefined computed key takes the variant's type, not an intersection", ( // data is what TS2425 forbids (see `BehaviourOf` in `types.ts`). Every surface // reading `A` on its own — `__output`, `toJSON()`, `output.shape` — is clean. type Out = Entity.Output; - // the root typed `shout` as Upper; the variant retypes it as Label. Under a - // plain `A & A2` this would be `Upper & Label` and neither line would compile. + // the root typed `shout` as Upper; the variant retypes it as Label. The + // negative below is the whole guard: under a plain `A & A2` this key would be + // `Upper & Label`, an intersection is assignable to *either* constituent, so + // both lines would still compile and the failure would surface as the + // directive going **unused** (`TS2578`) — not as the positive line breaking. const asLabel: z.infer = null as unknown as Out["shout"]; void asLabel; // @ts-expect-error the root's `Upper` brand is gone, not intersected in @@ -152,4 +155,26 @@ test("a redefined computed key takes the variant's type, not an intersection", ( void instanceAsUpper; }); +test("a redeclared field takes the variant's brand, not an intersection", () => { + class Retyped extends AccountBase.extend("Retyped")({ label: Upper }) { + override describe(): string { + return "retyped"; + } + } + // `Entity.Output` for the same reason as `Louder` above: an instance is that + // intersected with `BehaviourOf`, which carries the root's `label` + // unmapped, so only the surfaces reading `S` alone are honest. + type Out = Entity.Output; + // the root typed `label` as Label; the variant redeclares it as Upper. As with + // `Louder` above, the negative is the whole guard: under a plain `S & S2` this + // key would be `Label & Upper`, assignable to either constituent, so both + // lines would still compile and the regression would show up as the directive + // going **unused** (`TS2578`) rather than as a type error here. + const asUpper: z.infer = null as unknown as Out["label"]; + void asUpper; + // @ts-expect-error the root's `Label` brand is gone, not intersected in + const asLabel: z.infer = null as unknown as Out["label"]; + void asLabel; +}); + void Business; diff --git a/packages/entity/src/entity.ts b/packages/entity/src/entity.ts index 6789e0d..d0b6f32 100644 --- a/packages/entity/src/entity.ts +++ b/packages/entity/src/entity.ts @@ -26,6 +26,7 @@ import type { EntityStatic, Fields, MergedComputed, + MergedFields, PatchOf, Sealed, UpdateInputShapeOf, @@ -484,6 +485,7 @@ type AbstractEntitySrc< I extends PropertyKey, > = AbstractEntity; type MergedComputedSrc = MergedComputed; +type MergedFieldsSrc = MergedFields; type EntityStaticSrc< Tag extends string, S extends Fields, @@ -531,6 +533,8 @@ export declare namespace Entity { export type Sealed = SealedSrc; /** A root's computed map merged with a variant's — what `extend` hands `Static` as its `A`. */ export type MergedComputed = MergedComputedSrc; + /** A root's field map merged with a variant's — what `extend` hands `Static` as its `S`. */ + export type MergedFields = MergedFieldsSrc; /** * What `Entity(tag)(fields, options)` returns — the static surface itself. diff --git a/packages/entity/src/index.ts b/packages/entity/src/index.ts index bb4c990..2e07195 100644 --- a/packages/entity/src/index.ts +++ b/packages/entity/src/index.ts @@ -33,11 +33,16 @@ export { Entity } from "./entity.js"; // against a root declaring no `computed`: the emitter expands the alias // structurally again and the identical dangling `A2` comes back, `TS2304` and // all. Naming it without exporting it fixes nothing. Do not un-export it. +// +// `MergedFields` is that alias for the *field* map — the second type argument — +// and carries the identical hazard with `S2` in place of `A2`. It was named and +// exported from the start rather than measured into existence a second time. export type { BaseInstance, ConstructionKey, EntityStatic, MergedComputed, + MergedFields, Sealed, } from "./types.js"; diff --git a/packages/entity/src/types.ts b/packages/entity/src/types.ts index 8f104fe..3937fb5 100644 --- a/packages/entity/src/types.ts +++ b/packages/entity/src/types.ts @@ -270,14 +270,38 @@ export type BehaviourOf = This extends abstract new (...args: never[]) => * expand the alias structurally and bring the identical dangling `A2` straight * back. See the export list there, and do not un-export it. * - * The *fields* half of the same merge is `S & S2`, not this shape, and carries - * the same lie: the runtime spread is child-wins there too, so a redefined field - * types as `Parent & Child`. Left as is deliberately — the `Omit` form costs - * serialised characters against the `TS7056` budget on **every** entity, where - * this one is only paid by an entity that declares `computed`. + * The *fields* half of the same merge is `MergedFields`, below. */ export type MergedComputed = Omit & A2; +/** + * A root's field map merged with a variant's — what `extend` hands + * `EntityStatic` as its `S`. + * + * `Omit & S2`, never `S & S2`, for `MergedComputed`'s reason one + * map over: the runtime spread is `{ ...parent.fields, ...nextFields }`, so a + * variant redeclaring an inherited field wins, and a plain intersection would + * type that key as `ZodBranded & ZodBranded` while the schema + * held is the child's alone. + * + * Named and exported from the start for the reason **measured** on + * `MergedComputed`: written inline, the 5.9.3 emitter copied that alias's `A2` + * through unsubstituted and consumers failed with `TS2304`. This is the same + * alias in the same position, so it was never written inline here and the + * `S2` spelling of that failure has not been observed — it is inferred from the + * `A2` one, not a second measurement. See the export list in `index.ts`, and do + * not un-export it. + * + * Serialised width was the recorded reason this half was deferred, since every + * variant pays it where `MergedComputed` is paid only by one declaring + * `computed`. Measured rather than assumed: the emitter writes the alias **by + * reference**, so the billing fixture's `index.d.ts` grew 10,061 → 10,145 bytes + * — 42 per variant, against the 274,048 an unnamed type expands to — and all + * four `typecheck` steps stayed clean on both compilers. The `TS7056` budget is + * serialised characters, and a named alias barely spends it. + */ +export type MergedFields = Omit & S2; + /** * What `Entity.abstract(name)(fields, options?)` returns. * @@ -312,19 +336,21 @@ export type AbstractEntity< ): < S2 extends Fields, A2 extends Fields = Record, - const G2 extends readonly (keyof (S & S2))[] = [], - const I2 extends readonly (keyof OutputOf>)[] = [], + const G2 extends readonly (keyof MergedFields)[] = [], + const I2 extends readonly (keyof OutputOf, MergedComputed>)[] = [], >( fields: S2 & OnlyNominal, options?: { readonly generated?: G2; readonly immutable?: I2; - readonly computed?: { [K in keyof A2]: ComputedFieldOf> }; - readonly invariants?: readonly InvariantOf>[]; + readonly computed?: { + [K in keyof A2]: ComputedFieldOf>>; + }; + readonly invariants?: readonly InvariantOf>>[]; }, ) => EntityStatic< Tag2, - S & S2, + MergedFields, MergedComputed, G | G2[number], I | I2[number],