diff --git a/.changeset/forbid-entity-subclassing.md b/.changeset/forbid-entity-subclassing.md new file mode 100644 index 0000000..ac4c812 --- /dev/null +++ b/.changeset/forbid-entity-subclassing.md @@ -0,0 +1,17 @@ +--- +"@btravstack/entity": minor +--- + +**BREAKING**: subclassing an entity class is no longer supported. + +`class Sub extends SomeEntity {}` now fails at construction with a `Defect` — +a bug in domain code, not bad caller input, so it is not an `InvalidEntity`. +One `extends` is the declaration form and is unaffected; so is using the +builder's return directly without `extends`. + +Behaviour belongs in the entity's own class body, which is unchanged: extra +fields stay writable and are still absent from `toJSON()`. + +The prohibition is runtime-only. TypeScript has no `final`, so +`class Sub extends SomeEntity {}` still compiles and reports on first +construction. diff --git a/CLAUDE.md b/CLAUDE.md index 368b4b1..785dcb4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,6 +98,11 @@ design — `contract.spec.ts` pins that both ways. updating the matching `@ts-expect-error` assertion. - **One concept, one name.** The surface is meant to stay small enough that the library can be "done". Resist convenience aliases. +- **Entities are not subclassable.** One `extends` is the declaration form; + `construct` defects on anything deeper. Behaviour goes in the entity's own + class body. This is runtime-only — TypeScript has no `final`, and + `private`/`protected` constructors were measured to break the declaration + form (TS2675) and the statics (TS2684) respectively. - **No I/O.** The package reads no clock and generates no id — `create` takes domain-generated values as its second argument, by design. - `zod`, `unthrown` and `@unthrown/standard-schema` are peer dependencies to diff --git a/README.md b/README.md index 5669923..b6080ce 100644 --- a/README.md +++ b/README.md @@ -535,12 +535,12 @@ field whose schema yields a live mutable object is outside the guarantee. so assigning to _its_ keys is fine and mappers keep working unchanged (the values inside it are the entity's own, and stay frozen). -`Object.freeze(this)` is **not** used, and cannot be: a subclass's field +`Object.freeze(this)` is **not** used, and cannot be: a class body's field initialisers run after `super()` returns, so the instance itself has to stay extensible. Freezing the field values individually leaves that intact: ```ts -class OrgWithCache extends Organization { +class OrgWithCache extends Entity("OrgWithCache")({ id: OrgId, slug: Slug }) { cachedSummary = ""; } const org = OrgWithCache.decode(raw).getOrThrow(); @@ -548,6 +548,38 @@ org.cachedSummary = "computed"; // ✓ still writable — it isn't declared data org.toJSON(); // does NOT include cachedSummary — toJSON() projects only the declared schema's keys ``` +## Entities are not subclassable + +One `extends` is the declaration form. Subclassing the result is not +supported, and fails at construction with a `Defect`: + +```ts +class Sub extends Organization {} +Sub.decode(raw); // Defect — not an InvalidEntity: this is a bug in domain code +``` + +Put the behaviour in the entity's own class body, which is what it is for: + +```ts +class Organization extends Entity("Organization")({ ...fields }) { + get greeting(): string { + return `Welcome, ${this.name}`; + } +} +``` + +A subclass buys nothing the body does not, and it adds a second place to look +for an entity's methods. Redeclaring a data field is doubly blocked — the +compiler reports TS4114 (`must have an 'override' modifier`), and the field is +installed non-writable and non-configurable, so construction fails with +`TypeError: Cannot redefine property`. + +The prohibition is a **runtime** one: TypeScript has no `final`, and a +`private`/`protected` constructor cannot express "extendable once" — measured, +`TS2675` for `private` (the declaration form stops compiling) and `TS2684` for +`protected` (the statics stop returning the subclass). So `class Sub extends +Organization {}` compiles, and reports on first construction. + ## Helper types Four generic type-level helpers name each schema by reading it off an entity diff --git a/packages/entity/README.md b/packages/entity/README.md index 78c2812..570e462 100644 --- a/packages/entity/README.md +++ b/packages/entity/README.md @@ -98,16 +98,14 @@ test in `contract.spec.ts` pins that. re-running the invariants; immutable fields are dropped even if smuggled in at runtime past the type check - `toJSON()` — the stored data, projected to exactly the `decoded` schema's - keys, even from a subclass with extra fields. This is the **only** public + keys, even when the class body declares extra fields. This is the **only** public projection: it is the hook `JSON.stringify` looks for, so it has to exist, and a second method returning the same value under a domain name would be the alias this package resists. A repository write is `db.insert(org.toJSON())` - `equals(other)` — true when both are the same entity type and their - stored data is deep-equal. "Same entity" means the entity the class was - built from, not the class itself: two subclasses of a single `Entity(...)` - call compare equal when their stored data matches, while two separate - `Entity(...)` calls never do, even with identical fields + stored data is deep-equal. Two separate `Entity(...)` calls never compare + equal, even with identical fields ## Computed fields: `add` diff --git a/packages/entity/src/entity.spec.ts b/packages/entity/src/entity.spec.ts index d02f9a0..544d497 100644 --- a/packages/entity/src/entity.spec.ts +++ b/packages/entity/src/entity.spec.ts @@ -131,23 +131,44 @@ test("data fields are locked against mutation at runtime", () => { expect(org.slug).toBe("acme"); }); -test("locking data fields leaves subclass instance fields writable", () => { - // Object.freeze(this) would break this: subclass field initialisers run +// The entity's own class body — the supported place for behaviour and extra +// fields, now that subclassing an entity is not. +class OrgWithCache extends Entity("OrgWithCache")({ + id: OrgId, + slug: Slug, + name: DisplayName, +}) { + cachedSummary = ""; +} + +test("locking data fields leaves class-body instance fields writable", () => { + // Object.freeze(this) would break this: class-body field initialisers run // after super() returns, so the object must stay extensible. - class OrgWithCache extends Organization { - cachedSummary = ""; - } const org = OrgWithCache.decode(raw).getOrThrow(); org.cachedSummary = "computed"; expect(org.cachedSummary).toBe("computed"); expect(org.slug).toBe("acme"); }); -test("toJSON does not leak subclass instance fields", () => { - class OrgWithCache extends Organization { - cachedSummary = "leak me"; - } - expect(OrgWithCache.decode(raw).getOrThrow().toJSON()).not.toHaveProperty("cachedSummary"); +test("toJSON does not leak class-body instance fields", () => { + const org = OrgWithCache.decode(raw).getOrThrow(); + org.cachedSummary = "leak me"; + expect(org.toJSON()).not.toHaveProperty("cachedSummary"); +}); + +test("subclassing an entity is a defect, not a silent success", () => { + class Sub extends Organization {} + const outcome = Sub.decode(raw).match({ + ok: () => "WRONGLY ACCEPTED", + errCases: (m) => m.with(P.tag("InvalidEntity"), () => "invalid"), + defect: () => "defect", + }); + expect(outcome).toBe("defect"); +}); + +test("using the builder's return directly, without extends, still works", () => { + const Anon = Entity("Anon")({ id: OrgId, slug: Slug, name: DisplayName }); + expect(Anon.decode(raw).getOrThrow().slug).toBe("acme"); }); const Instant = z.iso.datetime().brand("Instant"); diff --git a/packages/entity/src/entity.ts b/packages/entity/src/entity.ts index 42b152b..113940c 100644 --- a/packages/entity/src/entity.ts +++ b/packages/entity/src/entity.ts @@ -1,5 +1,5 @@ import { fromSchema, type SchemaIssues } from "@unthrown/standard-schema"; -import { Err, Ok, P, type Result } from "unthrown"; +import { Err, Ok, P, fromThrowable, type Result } from "unthrown"; import type { z } from "zod"; import type { AddSpec } from "./add.js"; @@ -147,9 +147,27 @@ export function Entity(tag: Tag) { ): Result => { const broken = invariants?.(d) ?? []; // no `path` — an invariant spans the entity, not one field - return broken.length > 0 - ? Err(new InvalidEntity({ entity: tag, issues: broken.map((message) => ({ message })) })) - : Ok(new Ctor(d as Sealed)); + if (broken.length > 0) { + return Err( + new InvalidEntity({ entity: tag, issues: broken.map((message) => ({ message })) }), + ); + } + // A defect, not an `InvalidEntity`: subclassing is a bug in domain code, + // not bad caller input. `fromThrowable` is what keeps it inside the + // Result channel — a bare throw would escape `decode()` entirely. + return fromThrowable( + () => { + const ctor = Ctor as unknown as object; + if (ctor !== Base && (Object.getPrototypeOf(ctor) as unknown) !== Base) { + // oxlint-disable-next-line unthrown/no-throw + throw new Error( + `${tag}: subclassing an entity class is not supported — put the behaviour in the entity's own class body.`, + ); + } + return new Ctor(d as Sealed); + }, + (cause, defect) => defect(cause), + )() as Result; }; class Base { diff --git a/packages/entity/src/equality.spec.ts b/packages/entity/src/equality.spec.ts index ab6d129..1c06dce 100644 --- a/packages/entity/src/equality.spec.ts +++ b/packages/entity/src/equality.spec.ts @@ -41,14 +41,6 @@ test("different entity types with identical data are unequal", () => { expect(Organization.decode(raw).getOrThrow().equals(Team.decode(raw).getOrThrow())).toBe(false); }); -test("sibling subclasses of one entity with the same data are equal", () => { - // Identity is the entity a class was built from, not the class itself: both - // sides are still an `Organization`, so equal stored data means equal entity. - class Vendor extends Organization {} - class Customer extends Organization {} - expect(Vendor.decode(raw).getOrThrow().equals(Customer.decode(raw).getOrThrow())).toBe(true); -}); - test("comparing against a non-entity is false, not a throw", () => { const org = Organization.decode(raw).getOrThrow(); expect(org.equals(raw)).toBe(false);