HarperFast / HarperFast/harper

Collapse the ImmediateTransaction / RELEASED_TRANSACTION surface — two objects for one 'no owning scope' concept

Open
#2,241 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

### Summary

Two objects in `resources/DatabaseTransaction.ts` now encode the same idea — "this context has no owning transaction scope; reads see the latest committed state; writes do not belong to me" — with different shapes:

| | `ImmediateTransaction` | `RELEASED_TRANSACTION` |
| --- | --- | --- |
| `open` | 1 (OPEN) | 0 (CLOSED) |
| bound to a store | yes, `constructor(db)` sets `this.db` | no, `db` is undefined |
| lifetime | one per context, per store | one per process |
| frozen | no — `txnForContext` claims and mutates it | yes |

`ImmediateTransaction` is what `txnForContext` mints when a context has no transaction at all. `RELEASED_TRANSACTION` (added in #2230) is what a completed transaction leaves behind when it releases its context back-reference. A store-less, immutable `ImmediateTransaction` would *be* the released marker, so the surface is redundant.

### Why they are not already one thing

Three properties differ, and each is load-bearing rather than incidental:

1. **`open`.** A `txnForContext`-minted `ImmediateTransaction` must read as OPEN: the dispatchers join it and its `save()` override commits the write immediately. A released marker must read as CLOSED, or `resources/Resource.ts`'s dispatcher and `transaction()` treat the slot as a live scope, skip the `transaction()` wrapper, and nothing commits the staged write — the #1591 write-loss shape.
2. **Store binding.** `this.db` makes an instance specific to one store. A shared instance carrying a `db` would either serve the wrong store or have `txnForContext` walk and create `.next` links on the object every context points at.
3. **Mutability.** One-per-context means claimable and mutable is fine. One-per-process means any mutation writes through to every other context, so it has to be refused.

A single class can express both — store-bound + OPEN + mutable, or store-less + CLOSED + frozen — but it needs to be built for that rather than one inheriting from the other.

### What was already tried

The released marker started as a frozen `DatabaseTransaction` subclass (#2230, review rounds 1–2). It was replaced because inheriting a mutable working class to build an immutable marker kept leaking the working class's surface:

- `Object.freeze` is shallow, so `RELEASED_TRANSACTION.writes.push(op)` still mutated the shared instance.
- Freeze does not cover `#private` fields, so `setContext()` could rebind the shared context.
- Every inherited mutator that was not explicitly overridden threw an opaque `TypeError` from inside the base class rather than anything a caller could act on.

So the direction that failed is "released marker extends the working class". The direction worth trying is the inverse: give the store-less immutable form its own construction path and let `txnForContext` keep building store-bound instances from the same class.

### Scope

Pure refactor — no behavior change intended. `Context.transaction`'s documented contract stays as #2230 leaves it: after a transaction completes, the slot holds something whose `commit()`/`abort()` are no-ops and whose reads see the latest committed state.

Worth doing when the transaction lifecycle is next opened up rather than as a patch: `resources/Resource.ts`'s argument normalizer, `Resource.create`, `transaction()` and `txnForContext` all recognize the released marker by identity today, and collapsing the surface touches every one of those call sites.

Contributor guide

Open the contributing guide

Research direction

Start in resources/DatabaseTransaction.ts, then trace the released-marker identity checks in resources/Resource.ts, including the argument normalizer, Resource.create, transaction(), and txnForContext. Preserve the documented post-transaction no-op and latest-committed-state behavior while collapsing the redundant transaction surface; done means the lifecycle still behaves unchanged at every listed call site.

Written by the indexing model from the issue text.

Assessment

Tech stack
nodejs, typescript
Domain
backend, databases
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.