From c02be10d2d5598eacf6d2a70f18839bbea08070e Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Wed, 12 Aug 2026 20:54:05 +0200 Subject: [PATCH] refactor: cut the hand-rolled deepEqual and dead scaffolding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `equals` delegates to `node:util`'s `isDeepStrictEqual` instead of a 172-line traversal. Every case `equal.spec.ts` pins passes against it unchanged — bigint, Set/Map by contents, nested key order, typed arrays and ArrayBuffer bytewise, RegExp, Date, and both cyclic-value tests — with one difference: `+0`/`-0` now compare unequal (Object.is) where SameValueZero made them equal. No test pinned that. The import makes the package Node-only. Also removed, all verified against the full gate: - two of the three JSON-LD blocks in the docs head; `WebSite` and `Organization` restated title/url/logo VitePress already emits - the `test:types` script/turbo task/script triple — CI runs format, lint, typecheck, test, knip, build, and `typecheck` already runs the `.test-d.ts` pass - `IsAtLeastAsWideAs`, inlined at its two uses in `IsNarrowLiteral` - `RootInstance`, now `ConstructedInstance` rather than a second spelling of the same intersection — measured 47 bytes SMALLER in the emitted `index.d.mts`, with zero `GeneratedKeys<`/`ImmutableKeys<` in the consumer emit set - `schemaOf`, inlined at its one call site in `shape()` - the `Array.isArray(values)` branch in `discriminantValues`: zod v4 gives a Set for `z.literal(...).values` and `undefined` for `z.enum(...).values`, so no schema reached it A shared vitest config was tried and reverted: knip discovers spec files through each package's own config, so consolidating reported all 16 spec files and 5 devDependencies as unused. --- .changeset/lazy-pans-brake.md | 13 +++ CLAUDE.md | 14 +-- docs/.vitepress/config.ts | 27 ----- package.json | 1 - packages/entity/package.json | 1 - packages/entity/src/entity.ts | 9 +- packages/entity/src/equal.spec.ts | 3 +- packages/entity/src/equal.ts | 172 ------------------------------ packages/entity/src/field.ts | 2 - packages/entity/src/shape.ts | 20 ++-- packages/entity/src/types.ts | 3 +- packages/entity/src/union.ts | 1 - turbo.json | 3 - 13 files changed, 38 insertions(+), 231 deletions(-) create mode 100644 .changeset/lazy-pans-brake.md delete mode 100644 packages/entity/src/equal.ts diff --git a/.changeset/lazy-pans-brake.md b/.changeset/lazy-pans-brake.md new file mode 100644 index 0000000..4ff0f47 --- /dev/null +++ b/.changeset/lazy-pans-brake.md @@ -0,0 +1,13 @@ +--- +"@btravstack/entity": minor +--- + +`equals` now delegates to `node:util`'s `isDeepStrictEqual` instead of a +hand-rolled traversal. Behaviour is unchanged for every case the suite pins — +`bigint`, `Set`/`Map` by contents, nested key order, typed arrays and +`ArrayBuffer` bytewise, `RegExp`, `Date`, and cyclic values — with one +difference: `+0` and `-0` now compare **unequal** (`Object.is` semantics) where +the previous implementation treated them as equal (SameValueZero). + +This makes the package **Node-only**: it now imports a `node:` builtin, so a +browser or edge bundle without a `node:util` shim will fail to resolve it. diff --git a/CLAUDE.md b/CLAUDE.md index d04e4b2..492ddde 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,7 +85,7 @@ cannot run against it. Measured — the reason is inline in ## Architecture -Thirteen source modules under `packages/entity/src` besides `index.ts`, split by +Twelve source modules under `packages/entity/src` besides `index.ts`, split by what they own: - **`entity.ts`** — the builder. `Entity(tag)(fields, options)` derives the @@ -130,11 +130,13 @@ what they own: keys and the tag — a bare-schema redeclaration used to drop the root's flags silently. Built against a loosened `BuildEntity` passed in from `entity.ts`, so this module imports no builder and there is no cycle. -- **`equal.ts`** — `deepEqual`, the primitive behind `equals`. Not - `JSON.stringify`: that **threw** on a `bigint` field, compared `Set`/`Map`/ - typed-array fields with different contents as **equal**, and reported a - nested record as changed when only its key order differed. All three were - measured; `equal.spec.ts` pins them. +- **Equality** — `equals` is `node:util`'s **`isDeepStrictEqual`**, not `JSON.stringify` and + not a hand-rolled walk: serialising **threw** on a `bigint` field, compared + `Set`/`Map`/typed-array fields with different contents as **equal**, and + reported a nested record as changed when only its key order differed. All + three were measured, as was the cyclic-field stack overflow; + `equal.spec.ts` pins every one against the stdlib function. This import is + what makes the package **Node-only**. - **`freeze.ts`** — `deepFreeze`, the runtime half of immutability. Freezes and recurses into arrays and plain objects, freezes `Date` as a leaf, and deliberately leaves `Map`/`Set`/class instances alone. Which _fields_ to skip diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index 7ea3e3f..7556e2c 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -281,32 +281,5 @@ export default defineConfig({ keywords: "TypeScript, zod, domain-driven design, entity, immutability, Result", }), ], - // WebSite JSON-LD for proper site name display in Google search - [ - "script", - { type: "application/ld+json" }, - JSON.stringify({ - "@context": "https://schema.org", - "@type": "WebSite", - name: "entity", - url: SITE_URL, - }), - ], - // Organization JSON-LD for logo display in Google search - [ - "script", - { type: "application/ld+json" }, - JSON.stringify({ - "@context": "https://schema.org", - "@type": "Organization", - name: "entity", - url: SITE_URL, - logo: { - "@type": "ImageObject", - url: `${SITE_URL}logo.svg`, - }, - sameAs: ["https://github.com/btravstack/entity"], - }), - ], ], }); diff --git a/package.json b/package.json index e31ead0..37c61cb 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,6 @@ "prepare": "lefthook install", "release": "pnpm build && changeset publish", "test": "turbo run test", - "test:types": "turbo run test:types", "typecheck": "turbo run typecheck", "version": "changeset version" }, diff --git a/packages/entity/package.json b/packages/entity/package.json index fdd5542..c0f4626 100644 --- a/packages/entity/package.json +++ b/packages/entity/package.json @@ -48,7 +48,6 @@ "build": "tsdown src/index.ts --format cjs,esm --dts --clean", "dev": "tsdown src/index.ts --format cjs,esm --dts --watch", "test": "vitest run", - "test:types": "tsc --noEmit -p tsconfig.test-d.json", "typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.test-d.json" }, "devDependencies": { diff --git a/packages/entity/src/entity.ts b/packages/entity/src/entity.ts index 1bece42..1abad8e 100644 --- a/packages/entity/src/entity.ts +++ b/packages/entity/src/entity.ts @@ -1,3 +1,5 @@ +import { isDeepStrictEqual } from "node:util"; + import { fromSchema, type SchemaIssues } from "@unthrown/standard-schema"; import { Err, Ok, P, all, fromPromise, fromThrowable, type Result } from "unthrown"; import type { z } from "zod"; @@ -5,7 +7,6 @@ import type { z } from "zod"; import type { BuildEntity } from "./base.js"; import { createBase, record } from "./base.js"; import { computed, type ComputedField } from "./computed.js"; -import { deepEqual } from "./equal.js"; import { InvalidEntity } from "./errors.js"; import { field, isFieldSpec, type FieldSpec, type Flags } from "./field.js"; import { deepFreeze } from "./freeze.js"; @@ -355,11 +356,13 @@ export function Entity(tag: Tag) { * Compares the projected data structurally, not by `JSON.stringify`: * serialising threw on a `bigint` field, equated `Set`/`Map`/typed-array * fields with different contents, and reported a nested record as changed - * when only its key order differed. See `equal.ts`. + * when only its key order differed. `node:util`'s `isDeepStrictEqual` + * handles all three, plus cycles — every case is pinned in + * `equal.spec.ts`. It is what makes this package Node-only. */ equals(other: unknown): boolean { if (!(other instanceof Base)) return false; - return deepEqual(project(this), project(other)); + return isDeepStrictEqual(project(this), project(other)); } /** diff --git a/packages/entity/src/equal.spec.ts b/packages/entity/src/equal.spec.ts index 5065c37..49ac1f0 100644 --- a/packages/entity/src/equal.spec.ts +++ b/packages/entity/src/equal.spec.ts @@ -1,7 +1,8 @@ +import { isDeepStrictEqual as deepEqual } from "node:util"; + 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"); diff --git a/packages/entity/src/equal.ts b/packages/entity/src/equal.ts deleted file mode 100644 index eeab652..0000000 --- a/packages/entity/src/equal.ts +++ /dev/null @@ -1,172 +0,0 @@ -/** - * Structural equality for stored entity data. - * - * This exists because `JSON.stringify(a) === JSON.stringify(b)` — the previous - * implementation — is wrong in three measured ways for field types this package - * accepts. `OnlyNominal` admits any branded schema, and `freeze.ts` explicitly - * contemplates `Map`/`Set` fields, so none of these are exotic declarations: - * - * - A `z.bigint().brand(...)` field made `equals` **throw** - * `TypeError: Do not know how to serialize a BigInt`. `equals` returns a bare - * `boolean` with no Result channel, so that escaped as an uncaught exception. - * - A `Set`, `Map` or typed-array field serialises to `{}`, so two entities - * with entirely different contents compared **equal** — a false positive on - * identity, the worst direction for an equality check. - * - Only top-level key order is normalised, by `project`. A nested - * `z.record(...)` holding `{a,b}` versus `{b,a}` — identical contents — - * compared **unequal**, so a row read back with a different key order looked - * like a change. - * - * The traversal mirrors `freeze.ts`'s: the two walk the same shapes, and a - * type handled there should be handled here. - */ - -/** `Object.is`, but treating `+0` and `-0` as equal, per SameValueZero. */ -const sameValueZero = (a: unknown, b: unknown): boolean => - a === b || (Number.isNaN(a) && Number.isNaN(b)); - -const isObject = (value: unknown): value is object => typeof value === "object" && value !== null; - -const tagOf = (value: object): string => Object.prototype.toString.call(value); - -/** - * Own enumerable string keys, which is exactly what `toJSON` projects and what - * a class-body field would add. `_tag` is non-enumerable and so never compared, - * which is why two entities of different types are ruled out by the - * `instanceof` check in `equals` rather than by their tags. - */ -const keysOf = (value: object): readonly string[] => Object.keys(value); - -/** - * Set and Map hold their entries in an internal slot, so there is no key order - * to normalise and no way to index into them. Matching each entry against an - * unconsumed counterpart is quadratic, which is acceptable: these are domain - * collections, and the alternative — hashing arbitrary deep values — would need - * a canonical form this module deliberately does not define. - */ -/** - * 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". - * That applies here too, and more so now that a `z.custom` value is no longer - * frozen — measured, an unguarded compare of two cyclic values died with - * `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 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. 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>; - -const unorderedEqual = ( - left: readonly unknown[], - right: readonly unknown[], - equal: (a: unknown, b: unknown) => boolean, -): boolean => { - if (left.length !== right.length) return false; - const taken: boolean[] = Array.from({ length: right.length }, () => false); - return left.every((l) => - right.some((r, i) => { - if (taken[i] === true || !equal(l, r)) return false; - taken[i] = true; - return true; - }), - ); -}; - -/** - * Deep structural equality. - * - * Handles what an entity can actually store: primitives (including `bigint`, - * and `NaN` as equal to itself), `Date` by timestamp, arrays elementwise, - * `Set`/`Map` by contents rather than order, typed arrays and `ArrayBuffer` - * bytewise, `RegExp` by source and flags, and plain objects and class instances - * by their own enumerable keys, order-independently. - * - * Two values of different runtime kinds are never equal, so a `Map` never - * compares equal to the plain object with the same entries. - */ -export const deepEqual = (a: unknown, b: unknown): boolean => - equalWith(a, b, new WeakMap>()); - -const equalWith = (a: unknown, b: unknown, seen: Seen): boolean => { - if (sameValueZero(a, b)) return true; - if (!isObject(a) || !isObject(b)) return false; - - const tag = tagOf(a); - if (tag !== tagOf(b)) return false; - - 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) { - case "[object Date]": - return (a as Date).getTime() === (b as Date).getTime(); - case "[object RegExp]": - return String(a) === String(b); - case "[object Set]": { - const left = [...(a as Set)]; - const right = [...(b as Set)]; - return unorderedEqual(left, right, deepEqual); - } - case "[object Map]": { - const left = [...(a as Map)]; - const right = [...(b as Map)]; - return unorderedEqual(left, right, (l, r) => { - const [lk, lv] = l as readonly [unknown, unknown]; - const [rk, rv] = r as readonly [unknown, unknown]; - return deepEqual(lk, rk) && deepEqual(lv, rv); - }); - } - case "[object ArrayBuffer]": { - const left = new Uint8Array(a as ArrayBuffer); - const right = new Uint8Array(b as ArrayBuffer); - return left.length === right.length && left.every((byte, i) => byte === right[i]); - } - default: - break; - } - - if (ArrayBuffer.isView(a) && ArrayBuffer.isView(b)) { - const left = new Uint8Array(a.buffer, a.byteOffset, a.byteLength); - const right = new Uint8Array(b.buffer, b.byteOffset, b.byteLength); - return left.length === right.length && left.every((byte, i) => byte === right[i]); - } - - if (Array.isArray(a) || Array.isArray(b)) { - if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false; - return a.every((item, i) => deepEqual(item, b[i])); - } - - const aKeys = keysOf(a); - const bKeys = keysOf(b); - if (aKeys.length !== bKeys.length) return false; - const bRecord = b as Record; - const aRecord = a as Record; - return aKeys.every((key) => Object.hasOwn(bRecord, key) && deepEqual(aRecord[key], bRecord[key])); -}; diff --git a/packages/entity/src/field.ts b/packages/entity/src/field.ts index 0bc511d..1f2d0e6 100644 --- a/packages/entity/src/field.ts +++ b/packages/entity/src/field.ts @@ -90,5 +90,3 @@ export const isFieldSpec = (v: unknown): v is FieldSpec Object.hasOwn(v, "schema") && Object.hasOwn(v, "flags") && !("_zod" in v); - -export const schemaOf = (entry: unknown): unknown => (isFieldSpec(entry) ? entry.schema : entry); diff --git a/packages/entity/src/shape.ts b/packages/entity/src/shape.ts index 86770c6..9bdabb5 100644 --- a/packages/entity/src/shape.ts +++ b/packages/entity/src/shape.ts @@ -1,29 +1,23 @@ import { z } from "zod"; -import { schemaOf } from "./field.js"; +import { isFieldSpec } from "./field.js"; import type { Fields, SchemaOf, SchemasOf } from "./types.js"; /** A field is nominal if its inferred type is branded, or is already non-interchangeable. */ type Nominal = z.core.$brand | boolean; -/** - * True when `Wide` is assignable to `Candidate` — i.e. `Candidate` is at - * least as wide as `Wide`. Wrapped in a tuple so union `Candidate`s are - * compared as a whole rather than distributed member-by-member. - */ -type IsAtLeastAsWideAs = [Wide] extends [Candidate] ? true : false; - /** * A string/number literal union (e.g. a `z.enum(...)`) is narrow: the wide * primitive it's drawn from is not assignable to it. Bare `string`/`number` - * are the wide primitives themselves and are rejected. + * are the wide primitives themselves and are rejected. Each test is + * tuple-wrapped so a union `T` is compared as a whole rather than distributed. */ type IsNarrowLiteral = T extends string - ? IsAtLeastAsWideAs extends true + ? [string] extends [T] ? false : true : T extends number - ? IsAtLeastAsWideAs extends true + ? [number] extends [T] ? false : true : false; @@ -100,7 +94,9 @@ type OnlyNominal = { /** The only sanctioned way to declare a domain shape. */ export function shape(fields: T & OnlyNominal): z.ZodObject> { - const unwrapped = Object.fromEntries(Object.entries(fields).map(([k, v]) => [k, schemaOf(v)])); + const unwrapped = Object.fromEntries( + Object.entries(fields).map(([k, v]) => [k, isFieldSpec(v) ? v.schema : v]), + ); return z.object(unwrapped as SchemasOf); } diff --git a/packages/entity/src/types.ts b/packages/entity/src/types.ts index 85b1819..c370020 100644 --- a/packages/entity/src/types.ts +++ b/packages/entity/src/types.ts @@ -266,8 +266,7 @@ type ConstructedInstance = BaseInstance & - DeepReadonly> & { readonly _tag: string }; +export type RootInstance = ConstructedInstance; /** * Whatever the receiver's own class body added, carried **unmapped**. diff --git a/packages/entity/src/union.ts b/packages/entity/src/union.ts index 639bb2d..bf9604a 100644 --- a/packages/entity/src/union.ts +++ b/packages/entity/src/union.ts @@ -81,7 +81,6 @@ const discriminantValues = (member: UnionMember, discriminant: string): readonly // form that makes the singular `.value` getter throw. const values: unknown = schema?.values; if (values instanceof Set) return [...values]; - if (Array.isArray(values)) return values; // `z.enum([...])` — `.options` is the declared member list. const options: unknown = schema?.options; diff --git a/turbo.json b/turbo.json index 3a641c3..36880a8 100644 --- a/turbo.json +++ b/turbo.json @@ -9,9 +9,6 @@ "typecheck": { "dependsOn": ["build"] }, - "test:types": { - "dependsOn": ["^build"] - }, "test": { "dependsOn": ["^build"], "cache": false