Skip to content

Commit 2bebdbc

Browse files
seefeldbclaude
andauthored
refactor(runner): Use single TransactionWrapper class and optimize Cell.sink() (commontoolsinc#2176)
refactor(runner): Use single TransactionWrapper class with configurable options Refactors the transaction wrapper to use a single configurable class instead of multiple wrapper classes. Also uses this to optimize Cell.sink() by removing the duplicate validateAndTransform call - now we wrap the transaction to capture dependencies while child cells use a separate transaction. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <[email protected]>
1 parent 02f501d commit 2bebdbc

File tree

3 files changed

+84
-30
lines changed

3 files changed

+84
-30
lines changed

packages/runner/src/cell.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ import type {
6969
IExtendedStorageTransaction,
7070
IReadOptions,
7171
} from "./storage/interface.ts";
72-
import { NonReactiveTransaction } from "./storage/extended-storage-transaction.ts";
72+
import {
73+
createChildCellTransaction,
74+
createNonReactiveTransaction,
75+
} from "./storage/extended-storage-transaction.ts";
7376
import { fromURI } from "./uri-utils.ts";
7477
import { ContextualFlowControl } from "./cfc.ts";
7578

@@ -532,11 +535,11 @@ export class CellImpl<T> implements ICell<T>, IStreamable<T> {
532535
sample(): Readonly<T> {
533536
if (!this.synced) this.sync(); // No await, just kicking this off
534537

535-
// Wrap the transaction with NonReactiveTransaction to make all reads
536-
// non-reactive. Child cells created during validateAndTransform will
537-
// use the original transaction (via getTransactionForChildCells).
538+
// Wrap the transaction to make all reads non-reactive. Child cells created
539+
// during validateAndTransform will use the original transaction (via
540+
// getTransactionForChildCells).
538541
const readTx = this.runtime.readTx(this.tx);
539-
const nonReactiveTx = new NonReactiveTransaction(readTx);
542+
const nonReactiveTx = createNonReactiveTransaction(readTx);
540543

541544
return validateAndTransform(
542545
this.runtime,
@@ -1287,17 +1290,14 @@ function subscribeToReferencedDocs<T>(
12871290
const cancel = runtime.scheduler.subscribe((tx) => {
12881291
if (isCancel(cleanup)) cleanup();
12891292

1290-
// Run once with tx to capture _this_ cell's read dependencies.
1291-
validateAndTransform(runtime, tx, link, true);
1292-
1293-
// Using a new transaction for the callback, as we're only interested in
1293+
// Using a new transaction for child cells, as we're only interested in
12941294
// dependencies for the initial get, not further cells the callback might
12951295
// read. The callback is responsible for calling sink on those cells if it
12961296
// wants to stay updated.
1297-
12981297
const extraTx = runtime.edit();
1298+
const wrappedTx = createChildCellTransaction(tx, extraTx);
12991299

1300-
const newValue = validateAndTransform(runtime, extraTx, link, true);
1300+
const newValue = validateAndTransform(runtime, wrappedTx, link, true);
13011301
cleanup = callback(newValue);
13021302

13031303
// no async await here, but that also means no retry. TODO(seefeld): Should

packages/runner/src/storage/extended-storage-transaction.ts

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -216,21 +216,44 @@ export class ExtendedStorageTransaction implements IExtendedStorageTransaction {
216216
}
217217

218218
/**
219-
* A wrapper around an IExtendedStorageTransaction that adds ignoreReadForScheduling
220-
* meta to all read operations. This makes reads non-reactive - they won't trigger
221-
* re-execution of handlers when the read values change.
219+
* Options for configuring a TransactionWrapper.
220+
*/
221+
export interface TransactionWrapperOptions {
222+
/**
223+
* If true, adds ignoreReadForScheduling meta to all reads, making them
224+
* non-reactive.
225+
*/
226+
nonReactive?: boolean;
227+
228+
/**
229+
* Transaction to use for creating child cells. If not provided, uses the
230+
* wrapped transaction.
231+
*/
232+
childCellTx?: IExtendedStorageTransaction;
233+
}
234+
235+
/**
236+
* A configurable wrapper around an IExtendedStorageTransaction.
222237
*
223-
* Used by Cell.sample() to read values without subscribing to changes.
238+
* Supports two modes that can be combined:
239+
* - nonReactive: Adds ignoreReadForScheduling meta to all reads
240+
* - childCellTx: Uses a different transaction for child cells
241+
*
242+
* Used by:
243+
* - Cell.sample(): nonReactive=true, childCellTx=wrapped (child cells reactive)
244+
* - Cell.sink(): nonReactive=false, childCellTx=extraTx (child cells on separate tx)
224245
*/
225-
export class NonReactiveTransaction implements IExtendedStorageTransaction {
226-
constructor(private wrapped: IExtendedStorageTransaction) {}
246+
export class TransactionWrapper implements IExtendedStorageTransaction {
247+
constructor(
248+
private wrapped: IExtendedStorageTransaction,
249+
private options: TransactionWrapperOptions = {},
250+
) {}
227251

228252
/**
229253
* Get the transaction to use for creating child cells.
230-
* Child cells should be reactive, so this returns the unwrapped transaction.
231254
*/
232255
getTransactionForChildCells(): IExtendedStorageTransaction {
233-
return this.wrapped;
256+
return this.options.childCellTx ?? this.wrapped;
234257
}
235258

236259
get tx(): IStorageTransaction {
@@ -249,7 +272,10 @@ export class NonReactiveTransaction implements IExtendedStorageTransaction {
249272
return this.wrapped.reader(space);
250273
}
251274

252-
private addNonReactiveMeta(options?: IReadOptions): IReadOptions {
275+
private transformReadOptions(options?: IReadOptions): IReadOptions {
276+
if (!this.options.nonReactive) {
277+
return options ?? {};
278+
}
253279
return {
254280
...options,
255281
meta: { ...options?.meta, ...ignoreReadForScheduling },
@@ -260,14 +286,17 @@ export class NonReactiveTransaction implements IExtendedStorageTransaction {
260286
address: IMemorySpaceAddress,
261287
options?: IReadOptions,
262288
): Result<IAttestation, ReadError> {
263-
return this.wrapped.read(address, this.addNonReactiveMeta(options));
289+
return this.wrapped.read(address, this.transformReadOptions(options));
264290
}
265291

266292
readOrThrow(
267293
address: IMemorySpaceAddress,
268294
options?: IReadOptions,
269295
): JSONValue | undefined {
270-
return this.wrapped.readOrThrow(address, this.addNonReactiveMeta(options));
296+
return this.wrapped.readOrThrow(
297+
address,
298+
this.transformReadOptions(options),
299+
);
271300
}
272301

273302
readValueOrThrow(
@@ -276,7 +305,7 @@ export class NonReactiveTransaction implements IExtendedStorageTransaction {
276305
): JSONValue | undefined {
277306
return this.wrapped.readValueOrThrow(
278307
address,
279-
this.addNonReactiveMeta(options),
308+
this.transformReadOptions(options),
280309
);
281310
}
282311

@@ -318,18 +347,39 @@ export class NonReactiveTransaction implements IExtendedStorageTransaction {
318347
}
319348
}
320349

350+
/**
351+
* Create a non-reactive transaction wrapper for Cell.sample().
352+
* Reads won't trigger re-execution, but child cells will be reactive.
353+
*/
354+
export function createNonReactiveTransaction(
355+
tx: IExtendedStorageTransaction,
356+
): TransactionWrapper {
357+
return new TransactionWrapper(tx, { nonReactive: true, childCellTx: tx });
358+
}
359+
360+
/**
361+
* Create a transaction wrapper for Cell.sink() that uses a separate transaction
362+
* for child cells.
363+
*/
364+
export function createChildCellTransaction(
365+
tx: IExtendedStorageTransaction,
366+
childCellTx: IExtendedStorageTransaction,
367+
): TransactionWrapper {
368+
return new TransactionWrapper(tx, { childCellTx });
369+
}
370+
321371
/**
322372
* Helper function to get the transaction to use for creating child cells from a
323-
* potentially wrapped NonReactiveTransaction. If the transaction is not wrapped,
324-
* returns it as-is.
373+
* potentially wrapped transaction. If the transaction is not wrapped, returns
374+
* it as-is.
325375
*
326-
* Used when creating child cells that should be reactive even when the parent
327-
* read was non-reactive (e.g., in Cell.sample()).
376+
* Used when creating child cells that should use a different transaction than
377+
* the parent read (e.g., in Cell.sample() or Cell.sink()).
328378
*/
329379
export function getTransactionForChildCells(
330380
tx: IExtendedStorageTransaction | undefined,
331381
): IExtendedStorageTransaction | undefined {
332-
if (tx instanceof NonReactiveTransaction) {
382+
if (tx instanceof TransactionWrapper) {
333383
return tx.getTransactionForChildCells();
334384
}
335385
return tx;

packages/runner/src/storage/interface.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -922,5 +922,9 @@ export interface IAttestation {
922922
readonly value?: JSONValue;
923923
}
924924

925-
// Re-export NonReactiveTransaction from implementation
926-
export { NonReactiveTransaction } from "./extended-storage-transaction.ts";
925+
// Re-export transaction wrapper utilities from implementation
926+
export {
927+
createChildCellTransaction,
928+
createNonReactiveTransaction,
929+
TransactionWrapper,
930+
} from "./extended-storage-transaction.ts";

0 commit comments

Comments
 (0)