HarperFast / HarperFast/harper
Collapse the ImmediateTransaction / RELEASED_TRANSACTION surface — two objects for one 'no owning scope' concept
- 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
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