Skip to content

fix: make add-produced fields immutable - #4

Merged
btravers merged 1 commit into
mainfrom
fix/add-fields-immutable
Aug 6, 2026
Merged

fix: make add-produced fields immutable#4
btravers merged 1 commit into
mainfrom
fix/add-fields-immutable

Conversation

@btravers

@btravers btravers commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

decoded.add declares computed fields. Two defects let a caller desynchronise
one from the source it is derived from. Both were reproduced against main
before the fix.

Fixture:

class Org extends Entity("Org")(
  { id: OrgId, slug: Slug },
  {
    decoded: {
      add: add({ slugUpper: Upper })((e) => ({
        slugUpper: e.slug.toUpperCase() as z.infer<typeof Upper>,
      })),
    },
  },
) {}

const org = Org.decode({ id, slug: "acme" }).getOrThrow();

A. Computed fields went stale on update

update() calls this.encode() then Ctor.make(applied); addSpec.from was
never re-run.

slug slugUpper
before "beta" "ACME" (stale)
after "beta" "ACME" (pinned, by design — see below)

B. Computed fields were directly patchable

updateInput was built from decoded, which includes the added fields, so
Object.keys(Org.updateInput.shape) was ['id', 'slug', 'slugUpper'] and the
public API accepted a patch that contradicted the source outright.

org.update({ slugUpper: "LIES" }):

slug slugUpper
before "acme" "LIES"
after "acme" "ACME" (patch key dropped)

Object.keys(Org.updateInput.shape) is now ['id', 'slug'], and patching an
added field is a compile error.

Why immutability rather than recomputation

Recomputation was considered and rejected: it cannot be implemented correctly.
add's function takes the encoded object, and update() only ever holds
the decoded one — which, for the motivating case, no longer carries the source
field at all. In the package's own ApiKey example, fingerprint is computed
from secret, and secret is decoded.omit-ed; by update() time it is
gone. This is the same encoded/decoded asymmetry the README already documents
under "decode(x.encode()) does not round-trip".

So a recomputing update() would either fail for every entity whose computed
field reads an omitted source, or silently recompute from a partial input and
produce a different wrong answer. Given a value that drifts out of step with
its source versus one a caller can contradict, the package now offers neither:
an add-produced field is implicitly immutable, whether or not immutable
names it, and a derived value changes only by decoding a fresh encoded payload.

Changes

  • types.tsPatchOf and UpdateInputShapeOf exclude keyof A alongside
    I[number].
  • entity.ts — a single frozenKeys list (declared immutable plus add's
    keys) drives both the updateInput omit mask and update()'s runtime
    drop-list, mirroring how the latter already defended against a smuggled
    immutable field.
  • decoded.spec.ts — three tests: an added field is absent from
    updateInput.shape; update on a source field leaves the entity
    consistent; a smuggled added field is dropped at runtime.
  • entity.test-d.ts@ts-expect-error pinning that patching an added field
    and reading it off updateInput.shape are both compile errors.
  • types.test-d.tsPatchOf drops an added field not named in immutable.
  • contract.spec.ts — the ApiKey fixture gains a mutable label field so
    updateInput still exercises a non-empty update request schema now that
    fingerprint has left it.
  • Both READMEs document the rule and the reason.

Compatibility

Type-level narrowing of Patch/updateInput — code that patched an added
field stops compiling (it was already producing inconsistent data). Released as
a minor.

Copilot AI lite review requested due to automatic review settings August 6, 2026 17:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

packages/entity/src/entity.ts:78

  • The comment says freezing added fields is "the only answer that keeps a computed field consistent with its source", but this PR’s intended behavior is to pin added fields (not recompute them) — so when a source field is updated, the added field may become stale relative to it. Reword to avoid implying post-update consistency with the current source value.
     * `add` contributed. An added field is *implicitly* immutable — `add`
     * reads the **encoded** object, and `update` only ever holds the decoded
     * one, which no longer carries an omitted source field like `secret`, so
     * there is nothing to recompute from. Freezing them is the only answer
     * that keeps a computed field consistent with its source; see `PatchOf`.

packages/entity/src/decoded.spec.ts:76

  • This comment claims the slug/slugUpper pair is "never internally contradictory", but the test intentionally asserts slug === "beta" while slugUpper === "ACME". Since the added field is pinned (not recomputed), it can become stale when its source field is updated; the comment should reflect that behavior.
  // `slugUpper` is carried over, not recomputed — the encoded object `add`
  // reads from is gone by `update` time. It stays pinned to the value the
  // entity was decoded with, so the pair is never internally contradictory in
  // the way a silently stale recomputation would be.

`decoded.add` fields were part of `updateInput` and of the `Patch` type, so a
caller could patch a derived field to a value its own source contradicts, and
`update()` never re-ran `addSpec.from`, so a patch to a source field left the
computed field stale.

Recomputation is not available: `add` reads the encoded object, and by
`update()` time only the decoded one remains, without the omitted source field.
Added keys are therefore excluded from `UpdateInputShapeOf` and `PatchOf`, from
the runtime `updateInput`, and from `update()`'s applied patch.
@btravers
btravers force-pushed the fix/add-fields-immutable branch from 002b4b9 to 0b9a39a Compare August 6, 2026 20:33
@btravers
btravers merged commit d41b53b into main Aug 6, 2026
@btravers
btravers deleted the fix/add-fields-immutable branch August 6, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants