From a18618772e95ba80ef6ff484c6ffab11ff2e6285 Mon Sep 17 00:00:00 2001 From: Marc MacLeod Date: Mon, 27 Jul 2026 15:30:27 -0400 Subject: [PATCH] fix(db): compose optimistic updates from changes instead of whole-row snapshots --- .changeset/soft-donkeys-shave.md | 9 + packages/db/src/collection/state.ts | 40 +++- .../db/tests/optimistic-composition.test.ts | 224 ++++++++++++++++++ 3 files changed, 271 insertions(+), 2 deletions(-) create mode 100644 .changeset/soft-donkeys-shave.md create mode 100644 packages/db/tests/optimistic-composition.test.ts diff --git a/.changeset/soft-donkeys-shave.md b/.changeset/soft-donkeys-shave.md new file mode 100644 index 000000000..85c8c1dc2 --- /dev/null +++ b/.changeset/soft-donkeys-shave.md @@ -0,0 +1,9 @@ +--- +'@tanstack/db': patch +--- + +Fix concurrent optimistic updates to the same row overwriting each other's fields. + +Each pending update was stored as a whole-row snapshot taken at `mutate()` time, so whichever transaction applied last set every field — including fields it never touched. The clearest symptom: rolling back one of several in-flight transactions left its change visible, restored from a sibling transaction's snapshot. + +Updates now apply only the top-level fields they actually changed. Inserts are unaffected — they still apply the whole row, because an insert's `changes` omit schema defaults. diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index 0f7b3b868..185b71d06 100644 --- a/packages/db/src/collection/state.ts +++ b/packages/db/src/collection/state.ts @@ -13,6 +13,7 @@ import type { ChangeMessage, CollectionConfig, OptimisticChangeMessage, + PendingMutation, } from '../types' import type { CollectionImpl } from './index.js' import type { CollectionLifecycleManager } from './lifecycle' @@ -103,6 +104,11 @@ export class CollectionStateManager< public pendingLocalChanges = new Set() public pendingLocalOrigins = new Set() + private composedUpserts = new WeakMap< + object, + { base: TOutput; composed: TOutput } + >() + private virtualPropsCache = new WeakMap< object, { @@ -623,7 +629,7 @@ export class CollectionStateManager< case `update`: this.optimisticUpserts.set( mutation.key, - mutation.modified as TOutput, + this.resolveOptimisticUpsert(mutation), ) this.optimisticDeletes.delete(mutation.key) break @@ -798,6 +804,36 @@ export class CollectionStateManager< } } + /** + * Resolve the value an optimistic upsert contributes to the overlay. Updates + * compose `changes` over the value they were layered on so a mutation only + * owns the top-level fields it changed; inserts keep `modified` because their + * `changes` omit schema defaults. Composed rows are cached per `changes` + * object so an unchanged overlay keeps its reference and no-op events stay + * suppressed. + */ + private resolveOptimisticUpsert(mutation: PendingMutation): TOutput { + if (mutation.type !== `update`) { + return mutation.modified as TOutput + } + + const key = mutation.key as TKey + const base = this.optimisticUpserts.get(key) ?? this.syncedData.get(key) + if (base === undefined) { + return mutation.modified as TOutput + } + + const changes = mutation.changes as object + const cached = this.composedUpserts.get(changes) + if (cached !== undefined && cached.base === base) { + return cached.composed + } + + const composed = { ...base, ...mutation.changes } as TOutput + this.composedUpserts.set(changes, { base, composed }) + return composed + } + /** * Get the previous value for a key given previous optimistic state */ @@ -1174,7 +1210,7 @@ export class CollectionStateManager< case `update`: this.optimisticUpserts.set( mutation.key, - mutation.modified as TOutput, + this.resolveOptimisticUpsert(mutation), ) this.optimisticDeletes.delete(mutation.key) break diff --git a/packages/db/tests/optimistic-composition.test.ts b/packages/db/tests/optimistic-composition.test.ts new file mode 100644 index 000000000..466469eb3 --- /dev/null +++ b/packages/db/tests/optimistic-composition.test.ts @@ -0,0 +1,224 @@ +import { describe, expect, it } from 'vitest' +import { z } from 'zod' +import { createCollection } from '../src/collection/index.js' +import { createTransaction } from '../src/transactions' +import { mockSyncCollectionOptions, stripVirtualProps } from './utils' + +type Row = { id: number; a: string; b: string; c: string } + +const inFlight = () => new Promise(() => {}) + +function createRowCollection() { + return createCollection( + mockSyncCollectionOptions({ + id: `optimistic-composition`, + getKey: (row) => row.id, + initialData: [{ id: 1, a: `a0`, b: `b0`, c: `c0` }], + }), + ) +} + +function inFlightTransaction() { + const transaction = createTransaction({ + mutationFn: inFlight, + autoCommit: false, + }) + transaction.isPersisted.promise.catch(() => {}) + return transaction +} + +describe(`optimistic overlay composition`, () => { + it(`keeps a field owned by a transaction that mutated after a later-sorting one`, () => { + const collection = createRowCollection() + const t1 = inFlightTransaction() + const t2 = inFlightTransaction() + + t2.mutate(() => + collection.update(1, (draft) => { + draft.b = `b2` + }), + ) + t1.mutate(() => + collection.update(1, (draft) => { + draft.a = `a1` + }), + ) + t1.commit() + t2.commit() + + expect(stripVirtualProps(collection.get(1))).toEqual({ + id: 1, + a: `a1`, + b: `b2`, + c: `c0`, + }) + }) + + it(`keeps a settled transaction's field when an unrelated key syncs`, async () => { + const collection = createCollection( + mockSyncCollectionOptions({ + id: `optimistic-composition-unrelated-sync`, + getKey: (row) => row.id, + initialData: [ + { id: 1, a: `a0`, b: `b0`, c: `c0` }, + { id: 2, a: `x`, b: `y`, c: `z` }, + ], + }), + ) + + const pending = inFlightTransaction() + pending.mutate(() => + collection.update(1, (draft) => { + draft.a = `a1` + }), + ) + pending.commit() + + const settled = collection.update(1, (draft) => { + draft.b = `b2` + }) + collection.utils.resolveSync() + await settled.isPersisted.promise + + collection.utils.begin() + collection.utils.write({ + type: `update`, + value: { id: 2, a: `x2`, b: `y`, c: `z` }, + }) + collection.utils.commit() + + expect(stripVirtualProps(collection.get(1))).toEqual({ + id: 1, + a: `a1`, + b: `b2`, + c: `c0`, + }) + }) + + it(`rolling back one transaction only reverts its own field`, () => { + const collection = createRowCollection() + const t1 = inFlightTransaction() + const t2 = inFlightTransaction() + const t3 = inFlightTransaction() + + t1.mutate(() => + collection.update(1, (draft) => { + draft.a = `a1` + }), + ) + t2.mutate(() => + collection.update(1, (draft) => { + draft.b = `b2` + }), + ) + t3.mutate(() => + collection.update(1, (draft) => { + draft.c = `c3` + }), + ) + t1.commit() + t2.commit() + t3.commit() + + t2.rollback() + + expect(stripVirtualProps(collection.get(1))).toEqual({ + id: 1, + a: `a1`, + b: `b0`, + c: `c3`, + }) + }) + + it(`surfaces a synced update to a field no uncommitted mutation touched`, () => { + const collection = createRowCollection() + const transaction = inFlightTransaction() + + transaction.mutate(() => + collection.update(1, (draft) => { + draft.a = `a1` + }), + ) + + collection.utils.begin() + collection.utils.write({ + type: `update`, + value: { id: 1, a: `a0`, b: `remote`, c: `c0` }, + }) + collection.utils.commit() + + expect(stripVirtualProps(collection.get(1))).toEqual({ + id: 1, + a: `a1`, + b: `remote`, + c: `c0`, + }) + }) + + it(`preserves schema defaults from an optimistic insert when a later transaction updates the row`, () => { + const schema = z.object({ + id: z.number(), + title: z.string(), + completed: z.boolean().default(false), + priority: z.number().default(3), + }) + + const collection = createCollection({ + id: `optimistic-composition-defaults`, + getKey: (row) => row.id, + sync: { + sync: ({ begin, commit }) => { + begin() + commit() + }, + }, + schema, + }) + + const insertTransaction = inFlightTransaction() + insertTransaction.mutate(() => collection.insert({ id: 1, title: `first` })) + insertTransaction.commit() + + const updateTransaction = inFlightTransaction() + updateTransaction.mutate(() => + collection.update(1, (draft) => { + draft.title = `second` + }), + ) + updateTransaction.commit() + + expect(stripVirtualProps(collection.get(1))).toEqual({ + id: 1, + title: `second`, + completed: false, + priority: 3, + }) + }) + + it(`does not emit redundant update events when recomputing an unchanged overlay`, () => { + const collection = createRowCollection() + const transaction = inFlightTransaction() + + transaction.mutate(() => + collection.update(1, (draft) => { + draft.a = `a1` + }), + ) + transaction.commit() + + const seen: Array = [] + collection.subscribeChanges((changes) => { + for (const change of changes) { + if (change.key === 1) { + seen.push(change) + } + } + }) + + const unrelated = inFlightTransaction() + unrelated.mutate(() => collection.insert({ id: 2, a: `x`, b: `y`, c: `z` })) + unrelated.commit() + + expect(seen).toEqual([]) + }) +})