fix: bind instance and ~standard to the class they are read from

.changeset/plain-subclass-instance-identity.md

@@ -0,0 +1,5 @@
+---
+"@btravstack/entity": patch
+---
+
+`instance` and `~standard` now memoise per class, so a plain `class Y extends X {}` decodes to a `Y` regardless of read order.

README.md

@@ -548,6 +548,16 @@ org.cachedSummary = "computed"; // ✓ still writable — it isn't declared data
 org.toJSON(); // does NOT include cachedSummary — toJSON() projects only the declared schema's keys
 ```
 
+Subclassing an entity works, but **prefer not to**. Put the behaviour in the
+entity's own class body instead: that is what the body is for, and it keeps one
+name for one concept. A subclass buys nothing the body does not, and it adds a
+second place to look for an entity's methods. Where it is genuinely useful — a
+transient cache, a per-request decoration — keep it to fields and methods, and
+never redeclare a data field. Two things stop you: TypeScript reports TS4114
+(`must have an 'override' modifier`), and if you write `override` anyway the
+constructor installed that field non-writable and non-configurable, so
+construction fails with `TypeError: Cannot redefine property`.
+
 ## Helper types
 
 Four generic type-level helpers name each schema by reading it off an entity

packages/entity/README.md

@@ -80,6 +80,15 @@ carries a transform, so it has no _output_ representation —
 `z.toJSONSchema(SomeEntity.instance, { io: "output" })` throws by design, and a
 test in `contract.spec.ts` pins that.
 
+`instance` is bound to the class it is read from, including a plain
+`class Special extends SomeEntity {}` that never calls `Entity(...)` itself:
+`Special.instance.parse(raw)` yields a `Special`, `Special.instance` is not
+`SomeEntity.instance`, and neither read order changes that. Each class's
+schema is built once and reused, so `Special.instance === Special.instance`.
+The _type_ of `instance` is always the base entity's, though — TypeScript
+cannot repolymorphize a static property per subclass — so narrow with
+`instanceof` after parsing if you need the subclass's own members back.
+
 ## Instance members
 
 - the declared data fields, **deeply** read-only: each is installed

packages/entity/src/entity.spec.ts

@@ -271,3 +271,10 @@ test("update cannot smuggle a mutation in through the patch it was handed", () =
   asMutableArray(patch.tags).push("y");
   expect(updated.toJSON().tags).toEqual(["x"]);
 });
+
+test("a subclass redeclaring a data field fails at construction", () => {
+  class Hijack extends Organization {
+    override slug = "hijacked" as z.infer<typeof Slug>;
+  }
+  expect(() => Hijack.decode(raw).getOrThrow()).toThrow(/Cannot redefine property/);
+});

packages/entity/src/instance.spec.ts

@@ -52,6 +52,71 @@ test("instance and ~standard are built once and reused", () => {
   expect(Organization["~standard"]).toBe(Organization["~standard"]);
 });
 
+test("a bare subclass decodes to itself when the parent was read first", () => {
+  class Parent extends Entity("ParentReadFirst")({ id: OrgId, slug: Slug }) {}
+  class Sub extends Parent {}
+
+  expect(Parent.instance.parse(raw)).toBeInstanceOf(Parent);
+  expect(Sub.instance.parse(raw)).toBeInstanceOf(Sub);
+});
+
+test("a bare subclass decodes to itself when the subclass was read first", () => {
+  class Parent extends Entity("SubclassReadFirst")({ id: OrgId, slug: Slug }) {}
+  class Sub extends Parent {}
+
+  expect(Sub.instance.parse(raw)).toBeInstanceOf(Sub);
+  const built = Parent.instance.parse(raw);
+  expect(built).toBeInstanceOf(Parent);
+  expect(built).not.toBeInstanceOf(Sub);
+});
+
+test("a bare subclass gets its own instance and ~standard, each still stable", () => {
+  class Parent extends Entity("DistinctFromParent")({ id: OrgId, slug: Slug }) {}
+  class Sub extends Parent {}
+
+  expect(Sub.instance).not.toBe(Parent.instance);
+  expect(Parent.instance).toBe(Parent.instance);
+  expect(Sub.instance).toBe(Sub.instance);
+  expect(Sub["~standard"]).not.toBe(Parent["~standard"]);
+  expect(Parent["~standard"]).toBe(Parent["~standard"]);
+  expect(Sub["~standard"]).toBe(Sub["~standard"]);
+});
+
+test("instance follows a two-level subclass chain", () => {
+  class Root extends Entity("SubclassChainRoot")({ id: OrgId, slug: Slug }) {}
+  class Mid extends Root {}
+  class Leaf extends Mid {}
+
+  expect(Root.instance.parse(raw)).toBeInstanceOf(Root);
+  expect(Mid.instance.parse(raw)).toBeInstanceOf(Mid);
+  const leaf = Leaf.instance.parse(raw);
+  expect(leaf).toBeInstanceOf(Leaf);
+  expect(leaf).toBeInstanceOf(Root);
+  expect(new Set([Root.instance, Mid.instance, Leaf.instance]).size).toBe(3);
+});
+
+test("neither instance nor ~standard becomes an own enumerable key", () => {
+  class Parent extends Entity("NotEnumerable")({ id: OrgId, slug: Slug }) {}
+  class Sub extends Parent {}
+
+  // read both on both classes: nothing may be materialised as a plain key
+  void Parent.instance;
+  void Parent["~standard"];
+  void Sub.instance;
+  void Sub["~standard"];
+
+  for (const C of [Parent, Sub]) {
+    expect(Object.keys(C)).not.toContain("instance");
+    expect(Object.keys(C)).not.toContain("~standard");
+  }
+  // the accessor lives on the `Entity(...)` base and stays there: reading it
+  // must not stamp a value onto `Parent` (which would then reach `Sub`)
+  expect(Object.hasOwn(Parent, "instance")).toBe(false);
+  expect(Object.hasOwn(Sub, "instance")).toBe(false);
+  const base = Object.getPrototypeOf(Parent) as object;
+  expect(Object.getOwnPropertyDescriptor(base, "instance")?.get).toBeTypeOf("function");
+});
+
 test("a defect during decode propagates instead of becoming a validation issue", () => {
   class Buggy extends Entity("Buggy")(
     { id: OrgId },

packages/entity/src/instance.ts

@@ -42,51 +42,69 @@ function instanceSchema<T>(
 }
 
 /**
- * Attaches `instance` and `~standard` to an entity class as lazily
- * computed, self-overwriting accessor properties.
+ * The one schema each class gets, keyed by the class that asked for it.
+ * Keys are constructors, so a discarded class stays collectable.
+ */
+const schemas = new WeakMap<object, z.ZodType>();
+
+/**
+ * Attaches `instance` and `~standard` to an entity class as lazily computed
+ * accessor properties, memoised per receiver.
  *
  * The class is only ever consumed through a subclass (`class X extends
  * Entity(tag)(fields) {}`), which does not exist yet when the entity builder
  * runs. A plain value would close over the literal base constructor, so
- * `X.instance.parse(...)` would build a base instance — not an `X` —
- * failing `instanceof X`. A getter instead reads `this` from the access
- * site (`X.instance`), which JS's prototype-based static inheritance sets
- * to the actual receiver, so it binds to whichever subclass it was read
- * from — but a bare getter would rebuild the schema (and its `~standard`)
- * on every access, so `X.instance !== X.instance` and every `validate()`
- * call would reconstruct the whole transform chain. Each getter therefore
- * overwrites itself with a plain, non-enumerable data property on the same
- * receiver the first time it runs, so later reads are free and identity is
- * stable.
+ * `X.instance.parse(...)` would build a base instance — not an `X` — failing
+ * `instanceof X`. A getter instead reads `this` from the access site
+ * (`X.instance`), which JS's prototype-based static inheritance sets to the
+ * actual receiver, so it binds to whichever subclass it was read from. That
+ * receiver-reading is the whole point of the accessor and is why it cannot
+ * be replaced by a value.
+ *
+ * A bare getter would rebuild the schema (and its `~standard`) on every
+ * access, so `X.instance !== X.instance` and every `validate()` call would
+ * reconstruct the whole transform chain. The obvious cure — letting each
+ * getter overwrite itself with a plain data property on the receiver — was
+ * tried and abandoned, because a data property defined on `X` is *inherited*
+ * by every `class Y extends X {}` that has no `Entity(...)` call of its own.
+ * Reading `X.instance` first replaced the getter before `Y` ever saw it, and
+ * `Y.instance` then resolved to `X`'s property: `Y.instance.parse(...)`
+ * silently built an `X`, not a `Y`, with no error. Read order must not
+ * decide which class comes out.
+ *
+ * So the accessor stays in place permanently and the built schema is cached
+ * against the receiver in `schemas` instead. Every read re-enters the getter
+ * with `this` bound to the class actually read, and gets that class's own
+ * entry: `X.instance === X.instance` (built once, stable identity),
+ * `Y.instance !== X.instance`, and `Y.instance.parse(...)` yields a `Y`
+ * whichever of the two was read first.
  *
- * Caveat: the self-overwrite is first-read-wins *per receiver*, not per
- * class. `attachInstance` runs once per `Entity(...)` call, so every entity
- * built that way defines its own getter and is unaffected. But a bare `class
- * Y extends X {}` — a plain JS subclass with no `Entity(...)` call of its
- * own — has no getter of its own; it inherits `X`'s. If `X.instance` is read
- * first, the getter on `X` is replaced by `X`'s own data property before `Y`
- * ever reads it, and `Y.instance` then resolves to that inherited property:
- * `Y.instance.parse(...)` silently builds an `X`, not a `Y`, with no error.
+ * `~standard` reads through `this.instance` rather than caching separately,
+ * so the two can never disagree about which class they decode to; zod hangs
+ * `~standard` off the schema at construction, so a memoised `instance` makes
+ * it stable for free. Both properties stay non-enumerable — absent from
+ * `Object.keys`, spread and `JSON.stringify` — and configurable, so a
+ * consumer can still redefine them on a class of their own.
  */
 export function attachInstance<T>(Base: object, encoded: z.ZodType): void {
   Object.defineProperty(Base, "instance", {
     configurable: true,
     enumerable: false,
     get(this: object) {
+      const cached = schemas.get(this);
+      if (cached !== undefined) return cached;
       const built = instanceSchema<T>(encoded, (d) =>
         (this as unknown as { decode: (raw: unknown) => Result<T, InvalidEntity> }).decode(d),
       );
-      Object.defineProperty(this, "instance", { value: built, enumerable: false });
+      schemas.set(this, built);
       return built;
     },
   });
   Object.defineProperty(Base, "~standard", {
     configurable: true,
     enumerable: false,
     get(this: { instance: z.ZodType<T> }) {
-      const standard = (this.instance as unknown as { "~standard": unknown })["~standard"];
-      Object.defineProperty(this, "~standard", { value: standard, enumerable: false });
-      return standard;
+      return (this.instance as unknown as { "~standard": unknown })["~standard"];
     },
   });
 }