From 167a1e80f93a29a76c9820c82dc531a95a0256e6 Mon Sep 17 00:00:00 2001 From: Benoit Travers Date: Sat, 8 Aug 2026 01:23:01 +0200 Subject: [PATCH 1/4] fix: forget failed pairs in deepEqual, and freeze through union boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two measured correctness holes: - `deepEqual`'s cycle guard recorded every pair it entered and never forgot one that finished false, so `unorderedEqual`'s failed candidate matches poisoned later genuine comparisons — two Set fields with plainly different contents compared equal once their elements shared a subtree. The guard is now a stack of in-progress pairs, not a memo. - `deepFreeze`'s walk lost schema context at union, pipe and intersection boundaries, so a z.custom value nested inside one was frozen in place — mutating an object the caller still owns, the worse of the two errors by the module's own docstring. The walk now carries an "any of these branches" context that resolves ambiguity toward skipping. Also pins the documented-but-untested typed-array, ArrayBuffer, RegExp and tuple branches. Co-Authored-By: Claude Opus 5 (1M context) --- packages/entity/src/equal.spec.ts | 42 ++++++++++++++++++++ packages/entity/src/equal.ts | 32 ++++++++++----- packages/entity/src/freeze.spec.ts | 63 ++++++++++++++++++++++++++++++ packages/entity/src/freeze.ts | 43 ++++++++++++++++++-- 4 files changed, 168 insertions(+), 12 deletions(-) diff --git a/packages/entity/src/equal.spec.ts b/packages/entity/src/equal.spec.ts index c5ecede..83d50b8 100644 --- a/packages/entity/src/equal.spec.ts +++ b/packages/entity/src/equal.spec.ts @@ -1,6 +1,7 @@ import { expect, test } from "vitest"; import { z } from "zod"; +import { deepEqual } from "./equal.js"; import { Entity } from "./index.js"; const Id = z.uuid().brand("Id"); @@ -131,6 +132,47 @@ test("a cycle does not mask a difference reachable only through it", () => { expect(a.equals(b)).toBe(false); }); +test("a failed candidate match inside a Set does not poison a later comparison", () => { + type Item = { child: { v: number }; a: number }; + const ItemSchema = z.custom((v) => typeof v === "object" && v !== null); + const Items = z.set(ItemSchema).brand("Items"); + class Box extends Entity("Box")({ id: Id, items: Items }) {} + + const c = { v: 1 }; + const p: Item = { child: c, a: 1 }; + const r: Item = { child: c, a: 1 }; + const q: Item = { child: { v: 2 }, a: 1 }; + const s: Item = { child: { v: 1 }, a: 1 }; + + const left = Box.make({ id, items: new Set([p, r]) }).getOrThrow(); + const right = Box.make({ id, items: new Set([q, s]) }).getOrThrow(); + + // `left` holds two {v:1}-shaped items, `right` one {v:1} and one {v:2} — no + // matching exists, so the sets are unequal. The greedy walk first compares + // p against q (false), which recorded (c, q.child) in `seen`; r against q + // then hit that leftover pair and wrongly short-circuited to equal. + expect(left.equals(right)).toBe(false); +}); + +test("typed-array values compare bytewise", () => { + expect(deepEqual(new Uint8Array([1, 2]), new Uint8Array([1, 2]))).toBe(true); + expect(deepEqual(new Uint8Array([1, 2]), new Uint8Array([1, 3]))).toBe(false); + expect(deepEqual(new Uint8Array([1, 2]), new Uint8Array([1, 2, 3]))).toBe(false); + // different views over identical bytes are different kinds, never equal + expect(deepEqual(new Uint8Array([1]), new Int8Array([1]))).toBe(false); +}); + +test("ArrayBuffers compare bytewise", () => { + expect(deepEqual(new Uint8Array([1, 2]).buffer, new Uint8Array([1, 2]).buffer)).toBe(true); + expect(deepEqual(new Uint8Array([1, 2]).buffer, new Uint8Array([1, 3]).buffer)).toBe(false); +}); + +test("RegExps compare by source and flags", () => { + expect(deepEqual(/a/g, /a/g)).toBe(true); + expect(deepEqual(/a/g, /a/i)).toBe(false); + expect(deepEqual(/a/, /b/)).toBe(false); +}); + test("the same object compared against two different values is not confused", () => { const Bag = z.object({ n: z.number() }).brand("Bag"); class Holder extends Entity("Holder")({ id: Id, left: Bag, right: Bag }) {} diff --git a/packages/entity/src/equal.ts b/packages/entity/src/equal.ts index 6148b86..152d431 100644 --- a/packages/entity/src/equal.ts +++ b/packages/entity/src/equal.ts @@ -45,7 +45,7 @@ const keysOf = (value: object): readonly string[] => Object.keys(value); * a canonical form this module deliberately does not define. */ /** - * Pairs already being compared further up the stack. + * Pairs currently being compared further up the stack — a stack, not a memo. * * `deepFreeze` guards cycles with a `WeakSet` and its docstring names the * sources: "a `z.custom` field or a caller-supplied object can close a loop". @@ -54,13 +54,19 @@ const keysOf = (value: object): readonly string[] => Object.keys(value); * `RangeError: Maximum call stack size exceeded`, which is exactly the escaping * throw this module exists to remove. * - * Keyed by the left value, holding the right ones it is already being compared - * against, so the guard is per *pair*: `a` may legitimately be compared with - * several different values in one traversal. + * Keyed by the left value, holding the right ones it is currently being + * compared against, so the guard is per *pair*: `a` may legitimately be + * compared with several different values in one traversal. * * Assuming a revisited pair is equal is the standard co-inductive reading — * two structures are equal if assuming their cycles match leads to no - * contradiction elsewhere. + * contradiction elsewhere. That reading is only sound for pairs whose + * comparison is still *open*: a pair that already finished with `false` must be + * forgotten on the way out. `unorderedEqual`'s failed candidate matches do not + * abort the traversal, so a remembered failure would make a later genuine + * comparison of the same pair short-circuit to `true` — measured, two `Set` + * fields with plainly different contents compared equal once their elements + * shared a subtree. */ type Seen = WeakMap>; @@ -102,11 +108,19 @@ const equalWith = (a: unknown, b: unknown, seen: Seen): boolean => { const tag = tagOf(a); if (tag !== tagOf(b)) return false; - const against = seen.get(a); - if (against?.has(b) === true) return true; - if (against === undefined) seen.set(a, new WeakSet([b])); - else against.add(b); + const against = seen.get(a) ?? new WeakSet(); + if (against.has(b)) return true; + if (!seen.has(a)) seen.set(a, against); + against.add(b); + const result = compareObjects(a, b, tag, seen); + // the pair is only assumed-equal while its own comparison is open — see the + // `Seen` docstring for why a completed `false` must not stay recorded + if (!result) against.delete(b); + return result; +}; + +const compareObjects = (a: object, b: object, tag: string, seen: Seen): boolean => { const deepEqual = (l: unknown, r: unknown): boolean => equalWith(l, r, seen); switch (tag) { diff --git a/packages/entity/src/freeze.spec.ts b/packages/entity/src/freeze.spec.ts index 23c84c1..8d4b81d 100644 --- a/packages/entity/src/freeze.spec.ts +++ b/packages/entity/src/freeze.spec.ts @@ -171,3 +171,66 @@ test("a z.custom inside a record value stays writable", () => { Job.make({ id: jobId, cfgs: { a: caller } }).getOrThrow(); expect(Object.isFrozen(caller)).toBe(false); }); + +/** + * The walk must not lose schema context at a union boundary: `childSchema` had + * no `"union"` case, so a `z.custom` nested in an object *inside a branch* was + * walked with no context and frozen — the caller-owned mutation the module's + * own docstring calls the worse of the two errors. + */ +test("a z.custom nested in an object under a union branch stays writable", () => { + const Wrapped = z + .union([z.object({ cfg: CfgSchema, k: z.literal("a") }), z.literal("none")]) + .brand("Wrapped"); + class Job extends Entity("Job4")({ id: JobId, wrapper: Wrapped }) {} + + const caller = cfg(); + const job = Job.make({ id: jobId, wrapper: { cfg: caller, k: "a" } }).getOrThrow(); + + expect(Object.isFrozen(caller)).toBe(false); + caller.retries = 5; + expect(caller.retries).toBe(5); + // the branch's own object is decoded data, and is still frozen + expect(Object.isFrozen(job.wrapper)).toBe(true); +}); + +test("a z.custom behind a pipe is recognised through the out side", () => { + const schema = z.object({ cfg: z.unknown().pipe(CfgSchema) }); + const caller = cfg(); + const value = { cfg: caller }; + deepFreeze(value, undefined, schema); + expect(Object.isFrozen(value)).toBe(true); + expect(Object.isFrozen(caller)).toBe(false); +}); + +test("an intersection whose side is passthrough leaves the value alone", () => { + const schema = z.object({ + wrap: z.intersection( + CfgSchema, + z.custom(() => true), + ), + }); + const caller = cfg(); + deepFreeze({ wrap: caller }, undefined, schema); + expect(Object.isFrozen(caller)).toBe(false); +}); + +test("a z.custom inside one side of an intersection stays writable", () => { + const schema = z.intersection(z.object({ cfg: CfgSchema }), z.object({ n: z.number() })); + const caller = cfg(); + const value = { cfg: caller, n: 1 }; + deepFreeze(value, undefined, schema); + expect(Object.isFrozen(value)).toBe(true); + expect(Object.isFrozen(caller)).toBe(false); +}); + +test("a z.custom in a tuple slot is skipped while the other slots freeze", () => { + const schema = z.tuple([CfgSchema, z.object({ a: z.number() })]); + const caller = cfg(); + const other = { a: 1 }; + const value = [caller, other]; + deepFreeze(value, undefined, schema); + expect(Object.isFrozen(value)).toBe(true); + expect(Object.isFrozen(caller)).toBe(false); + expect(Object.isFrozen(other)).toBe(true); +}); diff --git a/packages/entity/src/freeze.ts b/packages/entity/src/freeze.ts index 9be55b2..905de71 100644 --- a/packages/entity/src/freeze.ts +++ b/packages/entity/src/freeze.ts @@ -68,6 +68,10 @@ const unwrap = (schema: Schema | undefined): Schema | undefined => { const getter = def["getter"]; return typeof getter === "function" ? unwrap(asSchema((getter as () => unknown)())) : schema; } + // the value being frozen is what parsing *produced*, and a pipe's output + // is described by its out side + case "pipe": + return unwrap(asSchema(def["out"])); default: return schema; } @@ -89,14 +93,34 @@ const isPassthrough = (schema: Schema | undefined): boolean => { const options = def["options"]; return Array.isArray(options) && options.some((o) => isPassthrough(asSchema(o))); } + // an intersection's value satisfied both sides, so if either side hands the + // caller's reference through, the value may still be caller-owned + if (def.type === "intersection") { + return isPassthrough(asSchema(def["left"])) || isPassthrough(asSchema(def["right"])); + } return false; }; +/** + * One schema context standing for several candidates, for the positions where + * the walk cannot know which branch produced the value. A synthetic `union` + * def, so `isPassthrough` treats it as passthrough when *any* candidate is — + * freezing an object the caller still owns is the worse of the two errors, and + * ambiguity resolves toward skipping. + */ +const anyOf = (candidates: readonly (Schema | undefined)[]): Schema | undefined => { + const children = candidates.filter((c): c is Schema => c !== undefined); + if (children.length === 0) return undefined; + if (children.length === 1) return children[0]; + return { _zod: { def: { type: "union", options: children } } }; +}; + /** * The schema describing `value[key]`, or `undefined` when the shape is one this - * cannot follow — an intersection, say. `undefined` means "no schema context", - * and the traversal then freezes as it always did, so an unhandled container is - * a missed skip rather than a crash. + * cannot follow. `undefined` means "no schema context", and the traversal then + * freezes as it always did — safe only because every container a passthrough + * value can legally sit under has a case here; an unhandled *leaf* is a missed + * skip rather than a crash. */ const childSchema = (schema: Schema | undefined, key: string | number): Schema | undefined => { const def = defOf(unwrap(schema)); @@ -118,6 +142,19 @@ const childSchema = (schema: Schema | undefined, key: string | number): Schema | const item = Array.isArray(items) ? items[Number(key)] : undefined; return asSchema(item ?? def["rest"]); } + // the walk cannot know which branch produced the value, so the child + // context is "any of these" — see `anyOf` + case "union": { + const options = def["options"]; + return Array.isArray(options) + ? anyOf(options.map((o) => childSchema(asSchema(o), key))) + : undefined; + } + case "intersection": + return anyOf([ + childSchema(asSchema(def["left"]), key), + childSchema(asSchema(def["right"]), key), + ]); default: return undefined; } From 5f1a39528f1a0d4085ecd707e06b6b4a819c5331 Mon Sep 17 00:00:00 2001 From: Benoit Travers Date: Sat, 8 Aug 2026 01:23:13 +0200 Subject: [PATCH 2/4] feat: readable InvalidEntity, honest toJSON typing, loud duplicate discriminants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `InvalidEntity.message` is rendered eagerly — entity name plus each issue's path and message — so a log line or failed assertion no longer prints a blank Error. New `Entity.renderIssue` and `Entity.keysOf` expose the same helpers for adapters building field-level responses. - `toJSON()` returns `DeepReadonly`: the projection is shallow, so nested containers are the instance's own frozen references, and the mutable type let `toJSON().tags.push(…)` compile and throw. - A duplicate union discriminant value across members is a declaration-time defect naming both members, instead of silently last-winning in `make` while zod threw lazily at the first parse. - The construction seal's property is `__useMakeOrFactoryInstead`, so the compile error on `new SomeEntity(…)` carries the fix. - Stale `consumer/` pointers updated to the emit-guards fixture; a byte-identical duplicated test in schema.spec.ts removed. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/heavy-buses-repair.md | 31 ++++++++++++++++++++++++++++ CLAUDE.md | 4 ++-- packages/entity/src/crud.spec.ts | 20 ++++++++++++++++++ packages/entity/src/entity.test-d.ts | 19 ++++++++++++----- packages/entity/src/entity.ts | 24 ++++++++++++++++----- packages/entity/src/errors.ts | 10 ++++++++- packages/entity/src/schema.spec.ts | 11 ---------- packages/entity/src/types.ts | 13 +++++++----- packages/entity/src/union.spec.ts | 10 +++++++++ packages/entity/src/union.ts | 23 ++++++++++++++++++--- 10 files changed, 133 insertions(+), 32 deletions(-) create mode 100644 .changeset/heavy-buses-repair.md diff --git a/.changeset/heavy-buses-repair.md b/.changeset/heavy-buses-repair.md new file mode 100644 index 0000000..acf7ec5 --- /dev/null +++ b/.changeset/heavy-buses-repair.md @@ -0,0 +1,31 @@ +--- +"@btravstack/entity": minor +--- + +Two correctness fixes, honest `toJSON` typing, and readable errors. + +- **Fix: `deepEqual` no longer remembers failed comparisons as equal.** The + cycle guard recorded every pair it entered and never forgot one that + finished `false`, so two `Set`/`Map` fields with plainly different contents + could compare equal once their elements shared a subtree. The guard is now a + stack of in-progress pairs, not a memo. +- **Fix: `deepFreeze` no longer freezes caller-owned values under a union + branch.** The schema walk lost context at `union`, `pipe` and + `intersection` boundaries, so a `z.custom(...)` value nested inside one was + frozen in place — mutating an object the caller still owns. The walk now + carries context through all three. +- **`toJSON()` returns `DeepReadonly`.** The projection is shallow: + the top-level object is fresh, but nested containers are the instance's own + frozen references, so the previous mutable type let + `toJSON().tags.push(…)` compile and throw at runtime. +- **`InvalidEntity.message` is populated** — `": : ; …"` — + so a log line or a failed assertion names the entity and the failing fields + instead of printing a blank `Error`. The structured `issues` are unchanged. +- **New `Entity.renderIssue` and `Entity.keysOf`** — the issue helpers an + adapter needs to turn an `InvalidEntity` into a response body, the same ones + the message is built from. +- **A duplicate union discriminant value is a declaration-time defect.** + `Entity.union` previously let the last member win while zod threw lazily at + the first parse; it now fails at the declaration, naming both members. +- **The construction seal's property is named `__useMakeOrFactoryInstead`**, so + the compile error on `new SomeEntity(…)` tells the reader what to do. diff --git a/CLAUDE.md b/CLAUDE.md index 1ae1145..3a7fa7b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -158,8 +158,8 @@ design — `contract.spec.ts` pins that both ways. `ConstructionKey` / `Sealed`, 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 - `TS4020`. That is measured, not assumed — `consumer/index.ts` names every - namespace member for exactly this reason, and an **unused** + `TS4020`. That is measured, not assumed — `examples/billing-domain/src/emit-guards.ts` + names every namespace member for exactly this reason, and an **unused** `@ts-expect-error` there is a failure signal, not noise. - **Entities are not subclassable.** One `extends` is the declaration form; `construct` defects on anything deeper. Behaviour goes in the entity's own diff --git a/packages/entity/src/crud.spec.ts b/packages/entity/src/crud.spec.ts index 12549e9..fcded80 100644 --- a/packages/entity/src/crud.spec.ts +++ b/packages/entity/src/crud.spec.ts @@ -88,6 +88,26 @@ test("update re-runs invariants", () => { expect(issues).toEqual(["trialEndsAt must be after createdAt"]); }); +test("an InvalidEntity carries a readable message, not a blank Error", () => { + const message = Organization.make({ id: "nope", slug: "" }).match({ + ok: () => "", + errCases: (m) => m.with(P.tag("InvalidEntity"), (e) => e.message), + defect: () => "DEFECT", + }); + // the structured `issues` stay the API; the message is their human spelling, + // so a bare `console.log(err)` or a failed assertion names the entity and + // the failing fields instead of printing an empty `Error` + expect(message).toContain("Organization"); + expect(message).toContain("id: "); +}); + +test("Entity.renderIssue and Entity.keysOf render and normalise one issue", () => { + // Standard Schema allows a bare key or a `{ key }` wrapper in a path + expect(Entity.keysOf({ path: ["a", { key: "b" }], message: "broken" })).toEqual(["a", "b"]); + expect(Entity.renderIssue({ path: ["a", { key: "b" }], message: "broken" })).toBe("a.b: broken"); + expect(Entity.renderIssue({ message: "spans the entity" })).toBe("spans the entity"); +}); + test("an entity with no generated or immutable options still exposes both schemas", () => { class Plain extends Entity("Plain")({ id: OrgId, slug: Slug }) {} expect(Object.keys(Plain.createInput.shape).toSorted()).toEqual(["id", "slug"]); diff --git a/packages/entity/src/entity.test-d.ts b/packages/entity/src/entity.test-d.ts index fd487dd..9e4b147 100644 --- a/packages/entity/src/entity.test-d.ts +++ b/packages/entity/src/entity.test-d.ts @@ -54,12 +54,21 @@ test("a branded field survives DeepReadonly with its brand intact", () => { void wrong; }); -test("toJSON() returns the plain, mutable decoded shape", () => { +test("toJSON() is typed readonly all the way down — the projection is shallow", () => { const bag = Bag.make({}).getOrThrow(); - // toJSON() builds a fresh object, so its own keys stay assignable — - // DeepReadonly applies to the instance's fields, not to this projection - const state: Entity.Output = bag.toJSON(); - state.tags = []; + const state = bag.toJSON(); + // Only the top-level object is fresh: nested containers are the instance's + // own frozen references, so `push` on one throws at runtime. Typing the + // return as the plain mutable shape let that compile — measured, an ORM + // mutating its argument hit `object is not extensible` where the types said + // it could not happen. + // @ts-expect-error a nested array is the frozen original + state.tags.push("x" as z.infer); + // @ts-expect-error a nested object's property is the frozen original's + state.address.city = "Paris"; + // reading is untouched + const city: string = state.address.city; + void city; }); test("toJSON() is the only projection — there is no second public spelling", () => { diff --git a/packages/entity/src/entity.ts b/packages/entity/src/entity.ts index f1fff58..cb756fe 100644 --- a/packages/entity/src/entity.ts +++ b/packages/entity/src/entity.ts @@ -7,7 +7,7 @@ import { deepEqual } from "./equal.js"; import { InvalidEntity } from "./errors.js"; import { deepFreeze } from "./freeze.js"; import { invariant, type Invariant } from "./invariant.js"; -import { renderIssue } from "./issues.js"; +import { keysOf, renderIssue } from "./issues.js"; import { attachSchema } from "./schema.js"; import { shape, type OnlyNominal } from "./shape.js"; import type { @@ -308,9 +308,17 @@ export function Entity(tag: Tag) { * a domain name. One that existed here was removed: two public spellings * of one projection is the alias CONTRIBUTING tells us to resist, and a * repository write reads perfectly well as `db.insert(org.toJSON())`. + * + * `DeepReadonly`, because the projection is shallow: the top-level object + * is fresh, but every nested container is the instance's own frozen + * reference. Typed as the plain mutable shape, `toJSON().tags.push(…)` + * compiled and threw `object is not extensible` at runtime — measured. */ - toJSON(): OutputShape { - return project(this); + toJSON(): DeepReadonly { + // through `unknown`: checking `OutputShape` against its own + // `DeepReadonly` while the shape is still generic makes TS build the + // full compatibility union and give up (TS2590) + return project(this) as unknown as DeepReadonly; } /** @@ -449,6 +457,12 @@ Entity.computed = computed; Entity.invariant = invariant; Entity.union = union; Entity.InvalidEntity = InvalidEntity; +// The issue helpers an adapter needs to turn an `InvalidEntity` into a +// response body: `keysOf` normalises a Standard Schema path (bare key or +// `{ key }` wrapper) to plain keys, `renderIssue` is the human spelling — +// the same one `InvalidEntity.message` is built from. +Entity.keysOf = keysOf; +Entity.renderIssue = renderIssue; /** * Source aliases for the namespace below. The `Src` suffix is load bearing. @@ -460,8 +474,8 @@ Entity.InvalidEntity = InvalidEntity; * loudly; the type just degenerates. Measured against tsdown + typescript * 7.0.2: spelling this member `import("./types.js").ConstructionKey` voided the * construction seal, and the only signal was the consumer fixture's - * `@ts-expect-error` on a forged key going *unused*. An unused directive under - * `consumer/` is a failure here, not noise. + * `@ts-expect-error` on a forged key going *unused*. An unused directive in + * `examples/billing-domain/src/emit-guards.ts` is a failure here, not noise. */ type ComputedFieldSrc = ComputedField; type InvariantSrc = Invariant; diff --git a/packages/entity/src/errors.ts b/packages/entity/src/errors.ts index 3b1e995..715ca92 100644 --- a/packages/entity/src/errors.ts +++ b/packages/entity/src/errors.ts @@ -1,6 +1,8 @@ import type { SchemaIssues } from "@unthrown/standard-schema"; import { TaggedError } from "unthrown"; +import { renderIssue } from "./issues.js"; + /** * `issues` stays structured (Standard Schema, as the validator produced it) so * a caller can key a field-level response off `path`. An `invariants` @@ -9,4 +11,10 @@ import { TaggedError } from "unthrown"; export class InvalidEntity extends TaggedError("InvalidEntity")<{ readonly entity: string; readonly issues: SchemaIssues; -}> {} +}> { + // Rendered eagerly, so a log line, a thrown `getOrThrow`, or a failed test + // assertion names the entity and the failing fields instead of printing a + // blank `Error`. The structured `issues` stay the API; the message is only + // their human spelling. + override message = `${this.entity}: ${this.issues.map(renderIssue).join("; ")}`; +} diff --git a/packages/entity/src/schema.spec.ts b/packages/entity/src/schema.spec.ts index 45356fc..96a46e1 100644 --- a/packages/entity/src/schema.spec.ts +++ b/packages/entity/src/schema.spec.ts @@ -29,17 +29,6 @@ test("the class nests inside a zod object and an array", () => { expect(many[0]).toBeInstanceOf(Organization); }); -test("a nested invariant failure names the failing member in the issue path", () => { - const result = z - .object({ owner: Organization }) - .safeParse({ owner: { ...raw, slug: "reserved" } }); - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.issues[0]?.path).toEqual(["owner"]); - expect(result.error.issues[0]?.message).toBe("slug must not be reserved"); - } -}); - test("the class is the Standard Schema entry point", () => { const parse = fromSchema(Organization); expect(parse(raw).getOrThrow()).toBeInstanceOf(Organization); diff --git a/packages/entity/src/types.ts b/packages/entity/src/types.ts index 218169f..06bd949 100644 --- a/packages/entity/src/types.ts +++ b/packages/entity/src/types.ts @@ -189,7 +189,10 @@ export declare class ConstructionKey { private constructor(); private readonly seal: never; } -export type Sealed = D & { readonly __constructionKey: ConstructionKey }; +// The property name is the error message: `new SomeEntity(…)` fails with +// "Property '__useMakeOrFactoryInstead' is missing…", which tells the reader +// what to do — the same trick `shape.ts` plays with its rejection type names. +export type Sealed = D & { readonly __useMakeOrFactoryInstead: ConstructionKey }; /** * The instance-side shape every entity's `Base` class structurally has: the @@ -211,7 +214,7 @@ export interface BaseInstance< A extends Fields, I extends readonly (keyof OutputOf)[], > { - toJSON(): OutputOf; + toJSON(): DeepReadonly>; equals(other: unknown): boolean; update(patch: PatchOf): Result; } @@ -228,9 +231,9 @@ export interface BaseInstance< * The data half is `DeepReadonly`, not `Readonly`: a shallow `Readonly` would * type an array field as a mutable `Tag[]`, so `entity.tags.push(…)` would * compile and — before the constructor started deep-freezing — mutate stored - * data, defeating an invariant that had already been checked. `toJSON()` keeps - * returning the plain `OutputOf` shape: it builds a fresh object, so its own - * keys really are assignable. + * data, defeating an invariant that had already been checked. `toJSON()` is + * `DeepReadonly` too: its top-level object is fresh, but the projection is + * shallow, so every nested container is one of these same frozen fields. */ type ConstructedInstance< Tag extends string, diff --git a/packages/entity/src/union.spec.ts b/packages/entity/src/union.spec.ts index b5e6796..f8e6292 100644 --- a/packages/entity/src/union.spec.ts +++ b/packages/entity/src/union.spec.ts @@ -137,6 +137,16 @@ test("a payload missing the discriminant is not misrouted to a member", () => { expect(message).toBe('Invalid discriminant undefined; expected one of "free", "silver", "gold"'); }); +test("two members claiming one discriminant value is a declaration-time defect", () => { + class DupA extends Entity("DupA")({ kind: z.literal("dup"), id: UserId }) {} + class DupB extends Entity("DupB")({ kind: z.literal("dup"), id: UserId }) {} + // A duplicate is a bug in the declaration, not bad caller input. Silently + // letting the last member win misrouted `make`, while zod's own + // discriminated union threw anyway — but lazily, at the first parse, far + // from the declaration that caused it. Failing here names both members. + expect(() => Entity.union("kind", [DupA, DupB])).toThrow(/DupA.+DupB.+"dup"/); +}); + test("a multi-value literal discriminant does not throw at construction", () => { class Wide extends Entity("Wide")({ plan: z.literal(["bronze", "tin"]), id: UserId }) {} const U = Entity.union("plan", [Member2Free, Wide]); diff --git a/packages/entity/src/union.ts b/packages/entity/src/union.ts index 7559331..246798f 100644 --- a/packages/entity/src/union.ts +++ b/packages/entity/src/union.ts @@ -111,9 +111,26 @@ export function union< members.map((m) => m.output) as unknown as Branches, ); - const byValue = new Map( - members.flatMap((m) => discriminantValues(m, discriminant).map((v) => [v, m] as const)), - ); + const byValue = new Map(); + for (const member of members) { + for (const value of discriminantValues(member, discriminant)) { + const taken = byValue.get(value); + if (taken !== undefined) { + // A defect, not an InvalidEntity: two members claiming one value is a + // bug in the declaration, not bad caller input. Left silent, the last + // member won and `make` misrouted; zod's own discriminated union threw + // `Duplicate discriminator value` anyway — but lazily, at the first + // parse, far from the declaration that caused it. Failing here names + // both members while the declaration is on the stack. + // oxlint-disable-next-line unthrown/no-throw + throw new Error( + `union(${JSON.stringify(discriminant)}): members "${taken.entityName}" and ` + + `"${member.entityName}" both claim discriminant value ${JSON.stringify(value)}`, + ); + } + byValue.set(value, member); + } + } const entity = members.map((m) => m.entityName).join(" | "); const known = [...byValue.keys()].map((k) => JSON.stringify(k)).join(", "); From c70928af73f0e222760d5c2975aae0cd5c911b42 Mon Sep 17 00:00:00 2001 From: Benoit Travers Date: Sat, 8 Aug 2026 01:23:23 +0200 Subject: [PATCH 3/4] docs: sync the reference pages, and argue branding and evolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The top-level type-export list is six, not three; `Entity.Static` is documented; `updateInput` is output minus immutable and computed; the field rules license `.optional()` and one array level; the zod `^4.3.0` floor reaches the site; `toJSON()` reads as DeepReadonly. - The http-contract guide maps issues with `Entity.keysOf` and shows `Entity.renderIssue`; errors.md covers the populated message, the union's invalid-discriminant issue and the duplicate-discriminant declaration defect. - Two new pages: explanation/branded-fields — the argument for the nominal rule and the two blessed brand-minting patterns — and how-to/evolve-an-entity — add, rename and retire fields against stored rows. Co-Authored-By: Claude Opus 5 (1M context) --- docs/.vitepress/config.ts | 2 + docs/api/index.md | 18 ++-- docs/explanation/branded-fields.md | 120 ++++++++++++++++++++++++ docs/explanation/peer-dependencies.md | 5 + docs/explanation/sealed-construction.md | 6 ++ docs/how-to/evolve-an-entity.md | 115 +++++++++++++++++++++++ docs/how-to/http-contract.md | 16 +++- docs/how-to/model-an-aggregate.md | 10 +- docs/how-to/persist-and-rehydrate.md | 12 ++- docs/how-to/test-domain-logic.md | 2 +- docs/reference/declaration.md | 20 +++- docs/reference/entry-points.md | 9 +- docs/reference/errors.md | 67 ++++++++++--- docs/reference/schemas.md | 2 +- docs/reference/types.md | 58 +++++++++--- docs/tutorial/getting-started.md | 13 ++- docs/typedoc.json | 13 ++- packages/entity/README.md | 14 +-- 18 files changed, 445 insertions(+), 57 deletions(-) create mode 100644 docs/explanation/branded-fields.md create mode 100644 docs/how-to/evolve-an-entity.md diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index 75a4b8e..9ce69da 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -20,6 +20,7 @@ const GUIDE_SIDEBAR = [ items: [ { text: "Expose an HTTP contract", link: "/how-to/http-contract" }, { text: "Persist and rehydrate", link: "/how-to/persist-and-rehydrate" }, + { text: "Evolve an entity", link: "/how-to/evolve-an-entity" }, { text: "Model an aggregate", link: "/how-to/model-an-aggregate" }, { text: "Test domain logic", link: "/how-to/test-domain-logic" }, ], @@ -39,6 +40,7 @@ const GUIDE_SIDEBAR = [ text: "Explanation", items: [ { text: "Why entity?", link: "/explanation/why-entity" }, + { text: "Branded fields", link: "/explanation/branded-fields" }, { text: "No I/O, by design", link: "/explanation/no-io" }, { text: "Sealed construction", link: "/explanation/sealed-construction" }, { text: "Immutability", link: "/explanation/immutability" }, diff --git a/docs/api/index.md b/docs/api/index.md index 509aaba..7affa95 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -4,8 +4,9 @@ Generated from the source with [TypeDoc](https://typedoc.org/) — every exporte symbol, with its signature and TSDoc. - **[`@btravstack/entity`](/api/entity/)** — `Entity`, the merged `Entity` - namespace, and the three seal names (`BaseInstance`, `ConstructionKey`, - `Sealed`) the published declarations force out. + namespace, and the six type names (`BaseInstance`, `ConstructionKey`, + `EntityStatic`, `EntityUnion`, `Sealed`, `UnionMember`) the published + declarations force out. ::: tip Looking for prose? The generated pages document _signatures_. For what each member is **for**, with @@ -22,9 +23,10 @@ _why_ the surface is shaped this way, read the import { Entity } from "@btravstack/entity"; ``` -`Entity.computed`, `Entity.invariant`, `Entity.union` and `Entity.InvalidEntity` -hang off it as values, and every public type lives in a merged -`declare namespace Entity`. A bare `computed` or `union` would be too generic to -take from a consumer's import scope, so nothing else is exported — with one -measured exception, [`BaseInstance` / `ConstructionKey` / -`Sealed`](/reference/types#the-seal-names). +`Entity.computed`, `Entity.invariant`, `Entity.union`, `Entity.InvalidEntity`, +`Entity.keysOf` and `Entity.renderIssue` hang off it as values, and every +public type lives in a merged `declare namespace Entity`. A bare `computed` or +`union` would be too generic to take from a consumer's import scope, so nothing +else is exported — with one measured exception, the +[six declaration-emit type names](/reference/types#the-declaration-emit-names) +a consumer's own `.d.ts` has to be able to write. diff --git a/docs/explanation/branded-fields.md b/docs/explanation/branded-fields.md new file mode 100644 index 0000000..5ee5b38 --- /dev/null +++ b/docs/explanation/branded-fields.md @@ -0,0 +1,120 @@ +--- +title: Branded fields +description: Why every field must be nominal, what counts as nominal, why the compile error is a type name, and the two blessed ways to mint a branded value. +--- + +# Branded fields + +Every field of an entity must be **nominal**: a branded schema, a narrow +literal union, a boolean, or another entity class. A bare `z.string()` is a +compile error naming `DomainFieldMustBeBrandedOrAnEntity`. This is the +package's most opinionated constraint, and it is the one place where it makes +your declaration longer rather than shorter — so it has to earn itself. + +## The bug the rule removes + +With plain primitives, a domain model is a bag of interchangeable strings. +`findOrg(slug, name)` type-checks with the arguments swapped; a repository +keyed by `userId: string` happily takes an `orgId`; a function returning a +"validated email" returns something the type system cannot tell from the raw +input it started with. Every one of these compiles, and every one of them is a +runtime bug waiting on the right call site — the class of defect the +literature calls _primitive obsession_. + +A brand makes the type nominal: + +```ts +const Slug = z.string().min(1).brand("Slug"); +const DisplayName = z.string().min(1).brand("DisplayName"); + +declare function findOrg( + slug: z.infer, + name: z.infer, +): void; + +findOrg(name, slug); // ✗ compile error — the arguments are swapped +``` + +At runtime a branded value is the plain primitive — the brand is a phantom +property that exists only in the type. It costs nothing to store, serialise or +compare; it only refuses to be confused with a different string. + +The rule is enforced rather than recommended because a brand only pays for +itself when it is unbroken: one bare `string` field is a hole every +unvalidated value in the program can flow through, and the field map is the +one place a library can check the whole perimeter at once. + +## What counts as nominal + +The check (`OnlyNominal`, applied to the field map) accepts a field whose +inferred type is already non-interchangeable: + +- a **branded schema** — `z.string().brand("Slug")`, `z.uuid().brand("OrgId")`, + a branded object, a branded number; +- a **narrow literal union** — `z.enum(["active", "inactive"])`, + `z.literal("user")`: the wide primitive is not assignable to it, so it + cannot be confused with an arbitrary string; +- a **boolean** — two values carry no identity worth branding; +- another **entity class** — an entity is nominal by construction, and the + class is itself a schema. + +The check looks through two wrappers — `.optional()` is stripped and one array +level is unwrapped — so `z.array(Customer)`, `Slug.optional()` and +`z.array(Slug).optional()` all pass; the rule applies to the element, not the +container. What it rejects is exactly the interchangeable core: bare +`z.string()`, bare `z.number()`, and any array or optional of those. + +## The error is a type name + +The rejection type is named `DomainFieldMustBeBrandedOrAnEntity` — a +deliberately sentence-shaped name, because the _name_ is the only part of a +type error guaranteed to survive. A rejection encoded as a tuple of message +strings prints as `& [...]` once TypeScript truncates a long diagnostic, +hiding the advice exactly when the field map is big enough to need it; a name +survives truncation and _is_ the message. The construction seal plays the same +trick: `new SomeEntity(...)` fails on a missing property called +`__useMakeOrFactoryInstead` +([Sealed construction](/explanation/sealed-construction)). + +## The cost, and the two blessed patterns + +The cost is ceremony: a branded type has no literal syntax, so somewhere a +plain value has to become a branded one. There are exactly two honest ways. + +**At a boundary, parse.** The schema is the brand's gatekeeper, so crossing +from untrusted to trusted goes through it — and for entity fields that +boundary already exists: `make` takes `unknown` and validates every field, so +a database row or request body never needs pre-branded values. + +```ts +const slug = Slug.parse(raw); // z.infer — or safeParse, handled +``` + +**Where the value is locally proven, cast.** Inside a generator or a +`computed` derivation the value is constructed in place and its validity is +visible in the same expression — and the package keeps the cast honest: +a factory's output goes through `make`'s validation, and a computed field's +output is checked against its own schema on every construction. + +```ts +const createOrg = Organization.factory({ + id: () => crypto.randomUUID() as z.infer, +}); + +computed: { + shout: Entity.computed(Upper, (d) => d.name.toUpperCase() as z.infer), +} +``` + +An `as` anywhere else — deep in application code, on a value that came from +outside — is not minting a brand, it is forging one: it silences the exact +check the rule exists to run. + +## Related + +- [Getting started, step 1](/tutorial/getting-started#_1-brand-your-fields) — + branding in practice. +- [Declaring an entity](/reference/declaration#fields) — the field rules as + reference, including the reserved names. +- [Why entity?](/explanation/why-entity) — the design this constraint belongs + to. diff --git a/docs/explanation/peer-dependencies.md b/docs/explanation/peer-dependencies.md index e6e1c08..dd2bdeb 100644 --- a/docs/explanation/peer-dependencies.md +++ b/docs/explanation/peer-dependencies.md @@ -18,3 +18,8 @@ So all four are installed together: ```sh pnpm add @btravstack/entity zod unthrown @unthrown/standard-schema ``` + +The declared zod range is `^4.3.0`, and the floor is measured rather than +guessed: the full surface typechecks, emits declarations and passes its runtime +assertions on 4.3.0. Nothing here needs a later minor, and monorepos commonly +pin one zod across every package, so the range is kept as wide as it is true. diff --git a/docs/explanation/sealed-construction.md b/docs/explanation/sealed-construction.md index cbce82e..5f49d46 100644 --- a/docs/explanation/sealed-construction.md +++ b/docs/explanation/sealed-construction.md @@ -13,6 +13,12 @@ run and the stored data is exactly what `output` describes. The seal is a type, not a runtime check, because a runtime guard would mean throwing — which this package exists to avoid. +The sealing property is named so the compile error carries the fix: +`new SomeEntity(...)` fails with +`Property '__useMakeOrFactoryInstead' is missing …` — the diagnostic tells the +reader what to do, the same trick the field rules play by making +`DomainFieldMustBeBrandedOrAnEntity` the rejection type's name. + Two alternatives were measured and rejected: - **`private constructor`** → `TS2675: Cannot extend a class 'Base'`. The diff --git a/docs/how-to/evolve-an-entity.md b/docs/how-to/evolve-an-entity.md new file mode 100644 index 0000000..e895978 --- /dev/null +++ b/docs/how-to/evolve-an-entity.md @@ -0,0 +1,115 @@ +--- +title: Evolve an entity +description: Add, default, rename and retire fields against stored rows — every read goes through make(), so evolution is about what old rows still validate. +--- + +# Evolve an entity + +**Problem:** the model needs a new field, a better name, or one field fewer — +and the database already holds rows in the old shape. Every read goes through +`make()`, which validates against `input`, so the question for each change is +the same: do the old rows still validate? + +> Snippets below assume these imports: +> +> ```ts +> import { z } from "zod"; +> import { Entity } from "@btravstack/entity"; +> ``` + +## Add an optional field + +The safe default. Old rows lack the key, `.optional()` accepts its absence, +and nothing else moves: + +```ts +class Organization extends Entity("Organization")({ + id: OrgId, + slug: Slug, + note: Note.optional(), // new — old rows simply don't have it +}) {} +``` + +The nominal-field check looks through `.optional()`, so the wrapper needs no +ceremony. ([Field rules](/reference/declaration#fields).) + +## Add a required field + +A required field rejects every old row, so something has to supply the value. +Two options, in order of preference: + +**Backfill, then require.** Migrate the stored rows first, then tighten the +declaration. The declaration stays honest — the field is required because +every row really has it — and a row that somehow escaped the backfill fails +loudly at `make` instead of silently carrying a filler value. + +**Default at the schema.** When there is one correct value for every old row, +put it on the field and skip the migration: + +```ts +class Organization extends Entity("Organization")({ + id: OrgId, + slug: Slug, + tier: z.enum(["free", "pro"]).default("free"), // old rows read as "free" +}) {} +``` + +`.default()` substitutes its value when the key is absent, **without** running +it through the schema; `.prefault()` parses the value like any other input — +prefer it when the field transforms or the default should face the same +validation. Either way the value is filled on read and present in `toJSON()`, +so rows heal as they are next written. The trade-off against backfilling: the +database keeps holding rows without the column, so anything querying the +column directly — SQL, an index, another service — does not see the default. +The schema heals reads through `make`; only a backfill heals the rows. + +## Rename a field + +`make` has no alias mechanism, deliberately — the declaration describes one +shape, not every shape the table has ever had. Renaming is a mapper concern, +at the repository edge: read both, write new. + +```ts +// was: shortName — now: slug +type StoredRow = Entity.Output; +type LegacyRow = Omit & { readonly shortName: string }; + +const fromRow = (row: StoredRow | LegacyRow) => + Organization.make("slug" in row ? row : { ...row, slug: row.shortName }); +``` + +Writes go through `toJSON()` and carry only the new name, so the old column +drains as rows are rewritten. Once a backfill (or time) has emptied it, delete +`LegacyRow` and the mapper's fallback — the mapper is the whole migration +surface, which is the point of routing reads through one. + +## Retire a field + +Remove it from the declaration. Nothing else is required: `make` ignores +unknown keys, so old rows still carrying the column validate untouched, and +`toJSON()` — which projects exactly `output`'s keys — stops writing it. Drop +the database column whenever convenient. + +Retiring is also what makes the **declaration-first** habit safe: a field the +model no longer names cannot be read, so any code still using it fails to +compile at the moment of the change, not in production. + +## Computed fields heal themselves + +A computed field needs no migration story at all: `make` validates the +declared fields and **re-derives** every computed one, so a row written before +a derivation changed — or before the computed field existed — reads back +correct. See +[Computed columns heal themselves](/how-to/persist-and-rehydrate#computed-columns-heal-themselves) +for the persistence half, and +[Why `computed` re-derives](/explanation/computed-fields) for the reasoning. + +## Decide what a failed read means + +Every evolution tightens or loosens what `make` accepts, and a row that stops +validating is a real signal, not noise. +[Decide what a read failure means](/how-to/persist-and-rehydrate#decide-what-a-read-failure-means) +covers handling it; while an evolution is rolling out, the +[`InvalidEntity.message`](/reference/errors#message) in the log names the +entity and the failing fields, which is usually enough to tell a missed +backfill from corruption. diff --git a/docs/how-to/http-contract.md b/docs/how-to/http-contract.md index a02af69..4121100 100644 --- a/docs/how-to/http-contract.md +++ b/docs/how-to/http-contract.md @@ -13,7 +13,7 @@ from the model. > > ```ts > import { z } from "zod"; -> import { match, P } from "unthrown"; +> import { P } from "unthrown"; > import { Entity } from "@btravstack/entity"; > ``` @@ -21,7 +21,7 @@ from the model. ```ts const CreateBody = Organization.createInput; // input minus generated -const UpdateBody = Organization.updateInput; // output minus immutable, partial +const UpdateBody = Organization.updateInput; // output minus immutable and computed, partial const ResponseBody = Organization.output; // stored state ``` @@ -76,7 +76,11 @@ const Listing = z.object({ ## Handle failures at the edge Issues are structured, so a field-keyed error response is a lookup rather than -a string parse: +a string parse. `Entity.keysOf` normalises an issue's path to plain keys — +Standard Schema permits a segment to be a bare key or a `{ key }` wrapper, and +the helper absorbs both — and `Entity.renderIssue` is the human spelling of one +issue, the same one +[`InvalidEntity.message`](/reference/errors#entity-invalidentity) is built from: ```ts const result = Organization.make(await request.json()); @@ -87,7 +91,7 @@ return result.match({ m.with(P.tag("InvalidEntity"), (e) => json(422, { errors: e.issues.map((i) => ({ - field: (i.path ?? []).join("."), // "" for a whole-entity rule + field: Entity.keysOf(i).join("."), // "" for a whole-entity rule message: i.message, })), }), @@ -99,6 +103,10 @@ return result.match({ }); ``` +When the response is a flat list of strings rather than field-keyed objects, +`e.issues.map(Entity.renderIssue)` is the whole mapping — `"slug: Too small: …"` +per issue, path prefix included. + An issue with an empty `path` came from `invariants` — a rule spanning the whole entity rather than one field. That distinction is what lets you decide whether to attach the message to a form field or to the form. diff --git a/docs/how-to/model-an-aggregate.md b/docs/how-to/model-an-aggregate.md index b0bfc8a..90f7e49 100644 --- a/docs/how-to/model-an-aggregate.md +++ b/docs/how-to/model-an-aggregate.md @@ -51,7 +51,7 @@ const order = Order.make(row).getOrThrow(); order.customer instanceof Customer; // true order.customer.shout; // its computed fields order.customer._tag; // its tag, for P.tag(...) matching -order.watchers[0].equals(other); // its behaviour +order.watchers.at(0)?.equals(other); // its behaviour ``` ## Invariants can span the boundary @@ -116,7 +116,13 @@ Member.make(row).getOrThrow(); // User | ServiceAccount — the real class The union dispatches on the discriminant rather than trying each branch, so a member whose own validation fails reports _its_ issues rather than every -branch's. +branch's. A payload whose discriminant matches no member fails as an +`InvalidEntity` whose one issue sits at `path: ["kind"]` and lists the values +the union knows. + +Two members claiming the same discriminant value is a bug in the declaration, +not bad input, so `Entity.union` throws at declaration time, naming both +members — left silent, the last member would win and `make` would misroute. The discriminant is a declared field, not `_tag`, because `_tag` is non-enumerable and absent after serialisation — a union built on it could not diff --git a/docs/how-to/persist-and-rehydrate.md b/docs/how-to/persist-and-rehydrate.md index e4a8852..13018d6 100644 --- a/docs/how-to/persist-and-rehydrate.md +++ b/docs/how-to/persist-and-rehydrate.md @@ -12,7 +12,7 @@ without the storage layer knowing about entity internals. > > ```ts > import { z } from "zod"; -> import { match, P } from "unthrown"; +> import { P } from "unthrown"; > import { Entity } from "@btravstack/entity"; > ``` @@ -38,6 +38,12 @@ org.toJSON(); // { id, slug } — cachedSummary is not there Do **not** use spread. `{ ...org }` copies own enumerable properties, which includes class-body fields — so it leaks exactly what `toJSON()` excludes. +The projection is typed `DeepReadonly`, and that is honest rather than +cautious: the top-level object is fresh, but nested containers are the +instance's own frozen references, so mutating one would throw. A driver that +insists on mutating its argument gets a structural clone +(`structuredClone(org.toJSON())`), not a cast. + ## Read with `make()` ```ts @@ -69,7 +75,9 @@ Person.make({ id, first: "Ada", last: "Lovelace", fullName: "stale value" }); That means a derivation change does not need a backfill migration to be _correct_ — only to make stored values match, for queries that read the column -directly. +directly. For every other kind of model change against stored rows — adding, +defaulting, renaming, retiring a field — see +[Evolve an entity](/how-to/evolve-an-entity). ## Map a repository diff --git a/docs/how-to/test-domain-logic.md b/docs/how-to/test-domain-logic.md index ae0b55d..e5958e4 100644 --- a/docs/how-to/test-domain-logic.md +++ b/docs/how-to/test-domain-logic.md @@ -12,7 +12,7 @@ tests without stubbing `Date.now` or `crypto.randomUUID`. > > ```ts > import { z } from "zod"; -> import { match, P } from "unthrown"; +> import { P } from "unthrown"; > import { Entity } from "@btravstack/entity"; > ``` diff --git a/docs/reference/declaration.md b/docs/reference/declaration.md index 06a6458..3fb4313 100644 --- a/docs/reference/declaration.md +++ b/docs/reference/declaration.md @@ -14,7 +14,6 @@ helpers that go inside them. For _why_ it is shaped this way, see > > ```ts > import { z } from "zod"; -> import { match, P } from "unthrown"; > import { Entity } from "@btravstack/entity"; > ``` @@ -31,6 +30,12 @@ class Organization extends Entity("Organization")(fields, options) {} A map of field name to schema. Every field must be **nominal** — a branded schema, a narrow literal union, a boolean, or another entity class. A bare `z.string()` is a compile error naming `DomainFieldMustBeBrandedOrAnEntity`. +([Why](/explanation/branded-fields).) + +The check looks through two wrappers: `.optional()` is stripped, and one array +level is unwrapped. So `z.array(Customer)`, `z.optional(Slug)` and even +`z.array(Slug).optional()` are all accepted — the rule applies to the element, +not the container. Four names are reserved, because an entity installs them on every instance: `_tag`, `equals`, `toJSON`, `update`. Using one is a compile error naming @@ -152,5 +157,18 @@ Member.discriminant; // "kind" on it rather than trying each branch, so a failing member reports its own issues. The union is a schema too, so it nests as a field. +A payload whose discriminant matches no member fails as an `InvalidEntity` +whose one issue carries `path: [discriminant]` — see +[Errors](/reference/errors#which-channel-a-failure-takes). Two members claiming +the same discriminant value is a **declaration-time defect**: `Entity.union` +throws, naming both members, rather than letting the last one silently win the +dispatch table. + +```ts +Entity.union("kind", [User, AlsoUser]); +// throws: union("kind"): members "User" and "AlsoUser" +// both claim discriminant value "user" +``` + `_tag` cannot serve as the discriminant here, and that is not an oversight — [it never reaches the wire](/explanation/tags-and-identity). diff --git a/docs/reference/entry-points.md b/docs/reference/entry-points.md index e5efcca..b11efe2 100644 --- a/docs/reference/entry-points.md +++ b/docs/reference/entry-points.md @@ -57,11 +57,18 @@ Returns a **new** entity. Re-runs the invariants and re-derives the computed fields. `immutable` and `computed` fields are absent from the patch type and dropped at runtime. -## `entity.toJSON()` → the stored shape +## `entity.toJSON()` → `DeepReadonly` Projects exactly `output`'s keys. Excludes `_tag` and any class-body fields. Called implicitly by `JSON.stringify`. +The return type is `DeepReadonly` because the projection is shallow: the +top-level object is fresh, but every nested container is the instance's own +frozen reference. Typed as the plain mutable shape, +`org.toJSON().tags.push(…)` compiled and threw `object is not extensible` at +runtime — the readonly type makes the freeze visible at compile time. Need a +mutable copy? Clone: `structuredClone(org.toJSON())`. + ## `entity.equals(other)` → `boolean` True when both are the same entity and their stored data is deep-equal. diff --git a/docs/reference/errors.md b/docs/reference/errors.md index 6fc1af1..cee3ce8 100644 --- a/docs/reference/errors.md +++ b/docs/reference/errors.md @@ -1,6 +1,6 @@ --- title: Errors -description: Entity.InvalidEntity, its structured issues, and the table of which failure goes down which channel. +description: Entity.InvalidEntity, its structured issues and rendered message, the Entity.keysOf / Entity.renderIssue helpers, and the table of which failure goes down which channel. --- # Errors @@ -11,7 +11,7 @@ modelled as a value; a bug in domain code goes down the separate defect channel. > Snippets on this page assume these imports: > > ```ts -> import { match, P } from "unthrown"; +> import { P } from "unthrown"; > import { Entity } from "@btravstack/entity"; > ``` @@ -21,7 +21,9 @@ modelled as a value; a bug in domain code goes down the separate defect channel. class InvalidEntity extends TaggedError("InvalidEntity")<{ readonly entity: string; readonly issues: SchemaIssues; // readonly StandardSchemaV1.Issue[] -}> {} +}> { + override message: string; // ": : ; …" — rendered eagerly +} ``` Reachable as both a value and a type — `e instanceof Entity.InvalidEntity` and @@ -33,16 +35,59 @@ is how you spell it. Matching by tag needs no import at all: Schema failures carry the failing field's `path`; an `invariants` violation has none — that absence distinguishes a whole-entity rule from a field complaint. +### `message` + +`issues` stays the structured API; `message` is only its human spelling — +rendered eagerly, so a log line, a thrown `getOrThrow()` or a failed test +assertion names the entity and the failing fields instead of printing a blank +`Error`: + +```ts +Organization.make({ id: "not-a-uuid", slug: "" }).getOrThrow(); +// throws: Organization: id: Invalid UUID; slug: Too small: expected string to have >=1 characters +``` + +Each issue renders as `path: message`, path segments joined with `.`; an +issue with no path — an invariant — renders as its message alone. + +## `Entity.keysOf(issue)` / `Entity.renderIssue(issue)` + +The two helpers an adapter needs to turn an `InvalidEntity` into a response +body, working on one element of `issues`: + +```ts +Entity.keysOf(issue); // PropertyKey[] — ["customer", "name"] +Entity.renderIssue(issue); // string — "customer.name: Too small: …" +``` + +`keysOf` normalises the issue's path to plain keys. Standard Schema permits a +path segment to be a bare `PropertyKey` **or** a `{ key }` wrapper — zod emits +the bare form, but code written against `issue.path` directly breaks on the +wrapped one, which is why the helper exists. `renderIssue` is the spelling +`message` is built from, so a hand-assembled error list and a logged message +never disagree. [Expose an HTTP contract](/how-to/http-contract#handle-failures-at-the-edge) +uses both. + ## Which channel a failure takes -| Failure | Channel | -| ---------------------------------------- | ------------------------------------ | -| a field fails its own schema | `InvalidEntity`, issue has a `path` | -| a broken `invariants` rule | `InvalidEntity`, issue has no `path` | -| `computed` output failing its own schema | **defect** | -| a `computed` function throwing | **defect** | -| an async generator rejecting | **defect** | -| subclassing an entity | **defect** | +| Failure | Channel | +| ------------------------------------------------- | ---------------------------------------------- | +| a field fails its own schema | `InvalidEntity`, issue has a `path` | +| a broken `invariants` rule | `InvalidEntity`, issue has no `path` | +| a union payload's discriminant matches nobody | `InvalidEntity`, one issue at `[discriminant]` | +| `computed` output failing its own schema | **defect** | +| a `computed` function throwing | **defect** | +| an async generator rejecting | **defect** | +| subclassing an entity | **defect** | +| two union members claiming one discriminant value | **defect**, thrown at declaration time | + +The union's "Invalid discriminant" issue lists the values it knows — +`Invalid discriminant "robot"; expected one of "user", "service_account"` — +and sits at the discriminant's own path, so it keys a field-level response +like any schema failure. The duplicate-value defect is different in kind: it +is a bug in the _declaration_, so `Entity.union` throws while the declaration +is on the stack, naming both members, instead of letting the last one silently +win the dispatch table. The line between the two columns is argued in [Errors are values, and defects are separate](/explanation/errors-are-values). diff --git a/docs/reference/schemas.md b/docs/reference/schemas.md index a6c9e88..7c12f61 100644 --- a/docs/reference/schemas.md +++ b/docs/reference/schemas.md @@ -18,7 +18,7 @@ Every entity carries four plain `ZodObject`s as statics, plus the class itself. Organization.input; // ZodObject — everything make() accepts Organization.output; // ZodObject — stored state and response body Organization.createInput; // ZodObject — input minus generated -Organization.updateInput; // ZodObject — output minus immutable, partial +Organization.updateInput; // ZodObject — output minus immutable and computed, partial Organization.entityName; // the tag, as a literal type Organization; // …is itself a zod schema, parsing to an instance ``` diff --git a/docs/reference/types.md b/docs/reference/types.md index 2bff596..e85f202 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 — and the three seal names exported at the top level. +description: Entity.Input, Entity.Output, Entity.CreateInput, Entity.Patch — and the six declaration-emit names exported at the top level. --- # Helper types @@ -17,22 +17,56 @@ type OrgCreate = Entity.CreateInput; // what a factory acce type OrgPatch = Entity.Patch; // what update() accepts ``` -Also `Entity.ComputedField` and `Entity.Union`, the shapes `Entity.computed` and -`Entity.union` return. +Also `Entity.ComputedField` and `Entity.Invariant`, the shapes `Entity.computed` +and `Entity.invariant` return; `Entity.Union`, what `Entity.union` returns; and +`Entity.Static`, the full static surface `Entity(tag)(fields, options)` returns +— the type of the anonymous class the declaration form extends. You rarely name +any of them: the declaration helpers infer their parameters from the +surrounding declaration. -## The seal names +## The declaration-emit names -`BaseInstance`, `ConstructionKey` and `Sealed` are the one exception to the -single-import rule: they are exported at the top level **as well as** under -`Entity`, because a downstream library compiling with `declaration: true` emits -the underlying name rather than the namespace path that aliases it, and would -otherwise fail with `TS4020`. They are not part of the API you write against. +Six types are exported at the top level: `BaseInstance`, `ConstructionKey`, +`EntityStatic`, `EntityUnion`, `Sealed`, `UnionMember`. They are the one +exception to the single-import rule, and none of them is part of the API you +write against. Five also have namespace aliases for anyone annotating by hand — +`Entity.BaseInstance`, `Entity.ConstructionKey`, `Entity.Sealed`, +`Entity.Static`, `Entity.Union` — but a consumer's _emitted declarations_ use +the top-level names. ```ts -import type { BaseInstance, ConstructionKey, Sealed } from "@btravstack/entity"; +import type { + BaseInstance, + ConstructionKey, + EntityStatic, + EntityUnion, + Sealed, + UnionMember, +} from "@btravstack/entity"; ``` -That is measured, not assumed: a fixture in CI compiles a consumer with -declaration emit against the built types, so it cannot regress. See +The exception exists for one reason: a downstream library compiling with +`declaration: true` emits the **underlying** type name, not the namespace path +that aliases it, so every type its declarations can reach must have a +top-level name. What each one buys was measured, not assumed: + +- **`BaseInstance`, `ConstructionKey`, `Sealed`** — the construction seal. + Kept module-private, a consumer's emitted `extends` clause fails with + `TS4020: … has or is using private name`. Exported, the emitted `.d.ts` + references `import("@btravstack/entity").Sealed<…>` and compiles. +- **`EntityStatic`** — what the whole builder returns. With no name to write, + TypeScript serialises the entire static surface structurally into every + consumer's `.d.ts`: a one-field entity emitted a 274,048-byte declaration + (240 bytes with the name), a realistic enum crossed the serialisation + ceiling (`TS7056`, issue #31), and a branded object field expanded until + zod's module-private `$brand` symbol could not be named (`TS4020`, #32). +- **`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 + `TS4023: Exported variable … cannot be named`. `UnionMember` travels with + `EntityUnion` because it is that type's own constraint. + +A fixture in CI compiles a consumer with declaration emit against the built +types, so none of this can regress. See [Sealed construction](/explanation/sealed-construction) for what the seal buys and what the two rejected alternatives cost. diff --git a/docs/tutorial/getting-started.md b/docs/tutorial/getting-started.md index 34e9efe..88a4ee5 100644 --- a/docs/tutorial/getting-started.md +++ b/docs/tutorial/getting-started.md @@ -22,7 +22,8 @@ pnpm add @btravstack/entity zod unthrown @unthrown/standard-schema All four, because `zod`, `unthrown` and `@unthrown/standard-schema` are **peer** dependencies — the package hands you back _your_ copies of them rather than its -own. ([Why](/explanation/peer-dependencies).) +own. ([Why](/explanation/peer-dependencies).) Any zod `^4.3.0` works; the floor +is measured, not guessed. ## 1. Brand your fields @@ -41,7 +42,7 @@ const Instant = z.iso.datetime().brand("Instant"); The reason is the one every domain modeller already knows: with plain strings, `findOrg(slug, name)` type-checks with the arguments swapped. Branded, it does -not. +not. ([The full argument](/explanation/branded-fields).) ## 2. Declare the entity @@ -66,8 +67,10 @@ Organization.createInput; // ZodObject — the create request Organization.updateInput; // ZodObject — the update request, partial ``` -Right now `createInput` equals `input` and `updateInput` is just `output` made -partial. The next step is what makes them differ. +Right now `createInput` has the same shape as `input`, and `updateInput` is +just `output` made partial — though each is its own object, so a registry keyed +by schema identity keeps all four. The next step is what makes their shapes +differ. ## 3. Say which fields the domain owns @@ -285,6 +288,8 @@ no output representation. end to end. - [Persist and rehydrate](/how-to/persist-and-rehydrate) — repositories, and why computed columns heal themselves. +- [Evolve an entity](/how-to/evolve-an-entity) — changing the model once rows + are stored. - [Model an aggregate](/how-to/model-an-aggregate) — entities nested in entities, and `Entity.union`. - [Test domain logic](/how-to/test-domain-logic) — deterministic tests with no diff --git a/docs/typedoc.json b/docs/typedoc.json index 0954900..012b7be 100644 --- a/docs/typedoc.json +++ b/docs/typedoc.json @@ -5,17 +5,24 @@ "out": "api/entity", "categoryOrder": ["Facade", "Declaration", "Entry points", "Types", "Errors", "*"], "intentionallyNotExported": [ + "AsyncEntityFactory", + "AsyncGenerators", "BaseInstanceSrc", "ComputedField", "ComputedFieldSrc", "ComputedOf", + "ConstructedInstance", "ConstructionKeySrc", + "CreateInputOf", + "DeepReadonly", "DomainFieldMustBeBrandedOrAnEntity", - "EntityStatic", - "EntityUnion", + "EntityFactory", + "EntityStaticSrc", "EntityUnionSrc", "Fields", + "Generators", "InputOf", + "InstanceOf", "InvalidEntity", "Invariant", "InvariantSrc", @@ -24,6 +31,6 @@ "OutputOf", "PatchOf", "SealedSrc", - "UnionMember" + "UpdateInputShapeOf" ] } diff --git a/packages/entity/README.md b/packages/entity/README.md index 2ecffb1..e6fc9a3 100644 --- a/packages/entity/README.md +++ b/packages/entity/README.md @@ -58,13 +58,13 @@ const loaded = Organization.make(row).getOrThrow(); // rows, imports, event fold const renamed = loaded.update({ name: next }).getOrThrow(); // a NEW entity ``` -| Schema member | For | -| ------------- | ---------------------------------------------------- | -| `input` | everything `make()` accepts | -| `output` | stored state and response body | -| `createInput` | create request — `input` minus `generated` | -| `updateInput` | update request — `output` minus `immutable`, partial | -| _the class_ | parses to an instance; valid as a field | +| Schema member | For | +| ------------- | ------------------------------------------------------------------- | +| `input` | everything `make()` accepts | +| `output` | stored state and response body | +| `createInput` | create request — `input` minus `generated` | +| `updateInput` | update request — `output` minus `immutable` and `computed`, partial | +| _the class_ | parses to an instance; valid as a field | Also `Entity.union(...)` for a union that is itself entity-like, and `SomeEntity.extend(tag)(fields)` to build a new entity from an existing one. From 2f808298554d11c5273b8d6cda4efbb5163df8d9 Mon Sep 17 00:00:00 2001 From: Benoit Travers Date: Sat, 8 Aug 2026 01:31:10 +0200 Subject: [PATCH 4/4] fix: forget every completed pair in deepEqual, not only the failed ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A completed `true` may have relied on an enclosing pair that was still only provisionally assumed equal; when that assumption then fails, the remembered `true` wrongly matches the nested pair in a later genuine trial. Both directions are now pinned, and the guard removes each pair on exit regardless of outcome — pure stack semantics, matching the `Seen` docstring. Co-Authored-By: Claude Opus 5 (1M context) --- packages/entity/src/equal.spec.ts | 25 +++++++++++++++++++++++++ packages/entity/src/equal.ts | 17 +++++++++-------- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/entity/src/equal.spec.ts b/packages/entity/src/equal.spec.ts index 83d50b8..5065c37 100644 --- a/packages/entity/src/equal.spec.ts +++ b/packages/entity/src/equal.spec.ts @@ -154,6 +154,31 @@ test("a failed candidate match inside a Set does not poison a later comparison", expect(left.equals(right)).toBe(false); }); +test("a provisional match inside a failed comparison is not remembered either", () => { + // x and y compare equal only under the assumption that their cyclic peers + // a and b match — an assumption that then fails (v differs). The completed + // `true` for (x, y) must be forgotten with it: remembered, it wrongly + // matches x to y in a later genuine trial, where their peers differ. + const pair = (v: number) => { + const node: { child?: unknown; v: number } = { child: undefined, v }; + const link = { peer: node, w: 1 }; + node.child = link; + return [node, link] as const; + }; + const [a, x] = pair(1); + const [b, y] = pair(2); + const [a2] = pair(1); + const [b2] = pair(2); + const p = { ref: a, extra: 1 }; + const q = { ref: b, extra: 1 }; + const p2 = { ref: a2, extra: 1 }; + const r = { ref: b2, extra: 1 }; + + // trial p-vs-q fails but seeds (x, y) as equal; x then has no genuine + // match on the right, so the sets are unequal + expect(deepEqual(new Set([p, x, r]), new Set([q, p2, y]))).toBe(false); +}); + test("typed-array values compare bytewise", () => { expect(deepEqual(new Uint8Array([1, 2]), new Uint8Array([1, 2]))).toBe(true); expect(deepEqual(new Uint8Array([1, 2]), new Uint8Array([1, 3]))).toBe(false); diff --git a/packages/entity/src/equal.ts b/packages/entity/src/equal.ts index 152d431..eeab652 100644 --- a/packages/entity/src/equal.ts +++ b/packages/entity/src/equal.ts @@ -61,12 +61,13 @@ const keysOf = (value: object): readonly string[] => Object.keys(value); * Assuming a revisited pair is equal is the standard co-inductive reading — * two structures are equal if assuming their cycles match leads to no * contradiction elsewhere. That reading is only sound for pairs whose - * comparison is still *open*: a pair that already finished with `false` must be - * forgotten on the way out. `unorderedEqual`'s failed candidate matches do not - * abort the traversal, so a remembered failure would make a later genuine - * comparison of the same pair short-circuit to `true` — measured, two `Set` - * fields with plainly different contents compared equal once their elements - * shared a subtree. + * comparison is still *open*: every completed pair must be forgotten on the + * way out, whatever it concluded. A remembered `false` poisons a later genuine + * comparison directly — measured, two `Set` fields with plainly different + * contents compared equal once their elements shared a subtree. A remembered + * `true` is subtler but as wrong: it may have relied on an enclosing pair that + * was still only provisionally assumed equal, and that assumption can then + * fail — both are pinned in `equal.spec.ts`. */ type Seen = WeakMap>; @@ -115,8 +116,8 @@ const equalWith = (a: unknown, b: unknown, seen: Seen): boolean => { const result = compareObjects(a, b, tag, seen); // the pair is only assumed-equal while its own comparison is open — see the - // `Seen` docstring for why a completed `false` must not stay recorded - if (!result) against.delete(b); + // `Seen` docstring for why no completed pair may stay recorded + against.delete(b); return result; };