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/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. 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/equal.spec.ts b/packages/entity/src/equal.spec.ts index c5ecede..5065c37 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,72 @@ 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("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); + 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..eeab652 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,20 @@ 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*: 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>; @@ -102,11 +109,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 no completed pair may stay recorded + 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/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/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; } 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(", ");