From afc98ef007936fe35b3e7f1a772b6fa9ebb2a887 Mon Sep 17 00:00:00 2001 From: Freddy Breitenstein Date: Sun, 12 Apr 2026 15:49:38 +0300 Subject: [PATCH] fix(db): item disappears during optimistic-to-synced transition (#1017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit collection.insert() called commit() (which synchronously enters mutationFn/onInsert) before recomputeOptimisticState(true), so the item was not in optimisticUpserts when onInsert ran. Any sync delivery during onInsert (e.g., Electric's txid handshake) would not find the item. Fix: move transactions.set(), scheduleTransactionCleanup(), and recomputeOptimisticState(true) before commit() so the optimistic entry exists when mutationFn executes. Regression test verifies collection.has(key) is true at every checkpoint during the optimistic → synced transition. Fixes #1017 --- .changeset/fix-1017-reconciliation-gap.md | 13 ++ packages/db/src/collection/mutations.ts | 9 +- packages/db/tests/reconciliation-gap.test.ts | 130 +++++++++++++++++++ 3 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-1017-reconciliation-gap.md create mode 100644 packages/db/tests/reconciliation-gap.test.ts diff --git a/.changeset/fix-1017-reconciliation-gap.md b/.changeset/fix-1017-reconciliation-gap.md new file mode 100644 index 000000000..8a22fc40e --- /dev/null +++ b/.changeset/fix-1017-reconciliation-gap.md @@ -0,0 +1,13 @@ +--- +"@tanstack/db": patch +--- + +fix: item disappears during optimistic-to-synced transition (#1017) + +`collection.insert()` calls `commit()` (which synchronously enters `mutationFn`/`onInsert`) +before `recomputeOptimisticState(true)` sets up the optimistic entry. This means `collection.has(key)` +returns `false` inside `onInsert`, and any sync data delivered during `onInsert` (e.g., Electric's +txid handshake) cannot find the item. + +Fix: move `transactions.set()`, `scheduleTransactionCleanup()`, and `recomputeOptimisticState(true)` +before `commit()` so the item is in `optimisticUpserts` when `onInsert` runs. diff --git a/packages/db/src/collection/mutations.ts b/packages/db/src/collection/mutations.ts index 963110801..7688ea92e 100644 --- a/packages/db/src/collection/mutations.ts +++ b/packages/db/src/collection/mutations.ts @@ -250,14 +250,17 @@ export class CollectionMutationsManager< // Apply mutations to the new transaction directOpTransaction.applyMutations(mutations) this.markPendingLocalOrigins(mutations) - // Errors still reject tx.isPersisted.promise; this catch only prevents global unhandled rejections - directOpTransaction.commit().catch(() => undefined) - // Add the transaction to the collection's transactions store + // Add the transaction and recompute optimistic state BEFORE commit. + // commit() synchronously enters mutationFn (onInsert) which may check + // collection.has(key). The item must be visible at that point (#1017). state.transactions.set(directOpTransaction.id, directOpTransaction) state.scheduleTransactionCleanup(directOpTransaction) state.recomputeOptimisticState(true) + // Errors still reject tx.isPersisted.promise; this catch only prevents global unhandled rejections + directOpTransaction.commit().catch(() => undefined) + return directOpTransaction } } diff --git a/packages/db/tests/reconciliation-gap.test.ts b/packages/db/tests/reconciliation-gap.test.ts new file mode 100644 index 000000000..485748858 --- /dev/null +++ b/packages/db/tests/reconciliation-gap.test.ts @@ -0,0 +1,130 @@ +/** + * Regression test for TanStack/db#1017 + * + * When a direct insert's onInsert handler syncs data back (e.g., Electric's + * txid handshake), the item must never disappear from the collection during + * the optimistic → synced transition. + */ +import { describe, expect, it } from 'vitest' +import { createCollection } from '../src/collection/index.js' + +interface Item { + id: string + title: string +} + +describe(`Reconciliation gap (#1017)`, () => { + it(`item should not disappear when onInsert syncs data back`, async () => { + let syncBegin: (() => void) | undefined + let syncWrite: + | ((msg: { type: `insert`; value: Item }) => void) + | undefined + let syncCommit: (() => void) | undefined + let syncMarkReady: (() => void) | undefined + + const collection = createCollection({ + id: `reconciliation-test`, + getKey: (item) => item.id, + startSync: true, + sync: { + sync: (params) => { + syncBegin = params.begin + syncWrite = params.write + syncCommit = params.commit + syncMarkReady = params.markReady + }, + }, + onInsert: async ({ transaction }) => { + const item = transaction.mutations[0].modified + + // Simulate Electric's onInsert flow: + // 1. Server accepts the write (REST API call) + // 2. Electric streams the committed row back via WAL + // 3. Sync delivers the row to the collection + syncBegin!() + syncWrite!({ type: `insert`, value: item }) + syncCommit!() + + // 4. Return (like Electric's awaitTxId resolving) + return {} + }, + }) + + syncMarkReady!() + await collection.stateWhenReady() + + // Insert — triggers optimistic insert + onInsert sync cycle. + collection.insert({ id: `item-1`, title: `Test item` }) + + // The item must ALWAYS be visible — never undefined. + // Before the fix, touchCollection() called onTransactionStateChange() + // (clearing optimistic state) before commitPendingTransactions() + // (writing synced data), creating a gap. + expect(collection.get(`item-1`)).toBeDefined() + expect(collection.has(`item-1`)).toBe(true) + + // Allow async settlement. + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Still visible after full settlement. + expect(collection.get(`item-1`)).toBeDefined() + }) + + it(`item visibility should have no gap during transition`, async () => { + let syncBegin: (() => void) | undefined + let syncWrite: + | ((msg: { type: `insert`; value: Item }) => void) + | undefined + let syncCommit: (() => void) | undefined + let syncMarkReady: (() => void) | undefined + const visibility: Array = [] + + const collection = createCollection({ + id: `visibility-gap-test`, + getKey: (item) => item.id, + startSync: true, + sync: { + sync: (params) => { + syncBegin = params.begin + syncWrite = params.write + syncCommit = params.commit + syncMarkReady = params.markReady + }, + }, + onInsert: async ({ transaction }) => { + const item = transaction.mutations[0].modified + + // Capture visibility before sync delivery. + visibility.push(collection.has(`item-1`)) + + syncBegin!() + syncWrite!({ type: `insert`, value: item }) + syncCommit!() + + // Capture visibility after sync delivery. + visibility.push(collection.has(`item-1`)) + return {} + }, + }) + + syncMarkReady!() + await collection.stateWhenReady() + + // Before insert: not visible. + visibility.push(collection.has(`item-1`)) + + collection.insert({ id: `item-1`, title: `Visibility test` }) + + // After insert returns: must be visible. + visibility.push(collection.has(`item-1`)) + + await new Promise((resolve) => setTimeout(resolve, 50)) + + // After settlement: still visible. + visibility.push(collection.has(`item-1`)) + + // Expected: [false, true, true, true, true] + // The item should be visible at every checkpoint after creation. + expect(visibility).toEqual([false, true, true, true, true]) + }) +})