Skip to content

Commit 7a2cacd

Browse files
KyleAMathewsclaudegithub-actions[bot]kevin-dp
authored
Fix syncedData not updating on manual writes in mutationFn (#1130)
* fix(db): ensure manual writes update syncedData during persisting transactions Manual write operations (writeInsert, writeUpdate, writeDelete, writeUpsert) were not updating syncedData when called from within a mutation handler (e.g., onUpdate with refetch: false). This caused an "off by one" bug where the cache would show stale data until the next sync operation. Root cause: commitPendingTransactions() skipped processing sync transactions when a persisting user transaction was active, but manual writes need to update syncedData synchronously. Fix: Add an `immediate` flag to sync transactions. When begin() is called with { immediate: true }, the transaction bypasses the persisting transaction check and is processed immediately. Manual write operations now use this flag. Changes: - Add `immediate?: boolean` to PendingSyncedTransaction interface - Update begin() to accept optional { immediate?: boolean } parameter - Modify commitPendingTransactions() to process immediate transactions regardless of persisting transaction state - Update performWriteOperations() to use begin({ immediate: true }) - Add regression test for writeUpsert in onUpdate with refetch: false * refactor(test): use .some() for cleaner transaction state check Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: add changeset for syncedData fix Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: clarify why all committed sync txs are processed together When hasImmediateSync or hasTruncateSync is true, we process ALL committed sync transactions, not just the immediate ones. This preserves causal ordering - if we only processed the immediate transaction, earlier non-immediate ones would apply later and could overwrite newer state. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: simplify boolean checks in state.ts Co-authored-by: Kevin <kevin-dp@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Kevin <kevin-dp@users.noreply.github.com>
1 parent 56f9f76 commit 7a2cacd

6 files changed

Lines changed: 139 additions & 6 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@tanstack/db': patch
3+
'@tanstack/query-db-collection': patch
4+
---
5+
6+
Fix syncedData not updating when manual write operations (writeUpsert, writeInsert, etc.) are called after async operations in mutation handlers. Previously, the sync transaction would be blocked by the persisting user transaction, leaving syncedData stale until the next sync cycle.

packages/db/src/collection/state.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ interface PendingSyncedTransaction<
2525
upserts: Map<TKey, T>
2626
deletes: Set<TKey>
2727
}
28+
/**
29+
* When true, this transaction should be processed immediately even if there
30+
* are persisting user transactions. Used by manual write operations (writeInsert,
31+
* writeUpdate, writeDelete, writeUpsert) which need synchronous updates to syncedData.
32+
*/
33+
immediate?: boolean
2834
}
2935

3036
export class CollectionStateManager<
@@ -437,13 +443,17 @@ export class CollectionStateManager<
437443
committedSyncedTransactions,
438444
uncommittedSyncedTransactions,
439445
hasTruncateSync,
446+
hasImmediateSync,
440447
} = this.pendingSyncedTransactions.reduce(
441448
(acc, t) => {
442449
if (t.committed) {
443450
acc.committedSyncedTransactions.push(t)
444-
if (t.truncate === true) {
451+
if (t.truncate) {
445452
acc.hasTruncateSync = true
446453
}
454+
if (t.immediate) {
455+
acc.hasImmediateSync = true
456+
}
447457
} else {
448458
acc.uncommittedSyncedTransactions.push(t)
449459
}
@@ -457,10 +467,21 @@ export class CollectionStateManager<
457467
PendingSyncedTransaction<TOutput, TKey>
458468
>,
459469
hasTruncateSync: false,
470+
hasImmediateSync: false,
460471
},
461472
)
462473

463-
if (!hasPersistingTransaction || hasTruncateSync) {
474+
// Process committed transactions if:
475+
// 1. No persisting user transaction (normal sync flow), OR
476+
// 2. There's a truncate operation (must be processed immediately), OR
477+
// 3. There's an immediate transaction (manual writes must be processed synchronously)
478+
//
479+
// Note: When hasImmediateSync or hasTruncateSync is true, we process ALL committed
480+
// sync transactions (not just the immediate/truncate ones). This is intentional for
481+
// ordering correctness: if we only processed the immediate transaction, earlier
482+
// non-immediate transactions would be applied later and could overwrite newer state.
483+
// Processing all committed transactions together preserves causal ordering.
484+
if (!hasPersistingTransaction || hasTruncateSync || hasImmediateSync) {
464485
// Set flag to prevent redundant optimistic state recalculations
465486
this.isCommittingSyncTransactions = true
466487

packages/db/src/collection/sync.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,12 @@ export class CollectionSyncManager<
8888
const syncRes = normalizeSyncFnResult(
8989
this.config.sync.sync({
9090
collection: this.collection,
91-
begin: () => {
91+
begin: (options?: { immediate?: boolean }) => {
9292
this.state.pendingSyncedTransactions.push({
9393
committed: false,
9494
operations: [],
9595
deletedKeys: new Set(),
96+
immediate: options?.immediate,
9697
})
9798
},
9899
write: (

packages/db/src/types.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,12 @@ export interface SyncConfig<
328328
> {
329329
sync: (params: {
330330
collection: Collection<T, TKey, any, any, any>
331-
begin: () => void
331+
/**
332+
* Begin a new sync transaction.
333+
* @param options.immediate - When true, the transaction will be processed immediately
334+
* even if there are persisting user transactions. Used by manual write operations.
335+
*/
336+
begin: (options?: { immediate?: boolean }) => void
332337
write: (message: ChangeMessageOrDeleteKeyMessage<T, TKey>) => void
333338
commit: () => void
334339
markReady: () => void

packages/query-db-collection/src/manual-sync.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ export interface SyncContext<
3535
queryClient: QueryClient
3636
queryKey: Array<unknown>
3737
getKey: (item: TRow) => TKey
38-
begin: () => void
38+
/**
39+
* Begin a new sync transaction.
40+
* @param options.immediate - When true, the transaction will be processed immediately
41+
* even if there are persisting user transactions. Used by manual write operations.
42+
*/
43+
begin: (options?: { immediate?: boolean }) => void
3944
write: (message: Omit<ChangeMessage<TRow>, `key`>) => void
4045
commit: () => void
4146
/**
@@ -144,7 +149,9 @@ export function performWriteOperations<
144149
const normalized = normalizeOperations(operations, ctx)
145150
validateOperations(normalized, ctx)
146151

147-
ctx.begin()
152+
// Use immediate: true to ensure syncedData is updated synchronously,
153+
// even when called from within a mutationFn with an active persisting transaction
154+
ctx.begin({ immediate: true })
148155

149156
for (const op of normalized) {
150157
switch (op.type) {

packages/query-db-collection/tests/query.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2286,6 +2286,99 @@ describe(`QueryCollection`, () => {
22862286
expect(todo?.id).not.toBe(clientId)
22872287
})
22882288

2289+
it(`should update syncedData immediately when writeUpsert is called after async API in onUpdate handler`, async () => {
2290+
// Reproduces bug where syncedData shows stale values when writeUpsert is called
2291+
// AFTER an async API call in a mutation handler. The async await causes the
2292+
// transaction to be added to state.transactions before writeUpsert runs,
2293+
// which means commitPendingTransactions() sees hasPersistingTransaction=true
2294+
// and would skip processing the sync transaction without the immediate flag.
2295+
const queryKey = [`writeUpsert-after-api-test`]
2296+
2297+
type Brand = {
2298+
id: string
2299+
brandName: string
2300+
}
2301+
2302+
const serverBrands: Array<Brand> = [{ id: `123`, brandName: `A` }]
2303+
2304+
const queryFn = vi.fn().mockImplementation(async () => {
2305+
return [...serverBrands]
2306+
})
2307+
2308+
// Track syncedData state immediately after writeUpsert
2309+
let syncedDataAfterWriteUpsert: Brand | undefined
2310+
let hasPersistingTransactionDuringWrite = false
2311+
2312+
const collection = createCollection(
2313+
queryCollectionOptions<Brand>({
2314+
id: `writeUpsert-after-api-test`,
2315+
queryKey,
2316+
queryFn,
2317+
queryClient,
2318+
getKey: (item: Brand) => item.id,
2319+
startSync: true,
2320+
onUpdate: async ({ transaction }) => {
2321+
const updates = transaction.mutations.map((m) => m.modified)
2322+
2323+
// Simulate async API call - THIS IS KEY!
2324+
// After this await, the transaction will be in state.transactions
2325+
await new Promise((resolve) => setTimeout(resolve, 10))
2326+
2327+
// Check if there's now a persisting transaction
2328+
hasPersistingTransactionDuringWrite = Array.from(
2329+
collection._state.transactions.values(),
2330+
).some((tx) => tx.state === `persisting`)
2331+
2332+
// Update server state
2333+
for (const update of updates) {
2334+
const idx = serverBrands.findIndex((b) => b.id === update.id)
2335+
if (idx !== -1) {
2336+
serverBrands[idx] = { ...serverBrands[idx], ...update }
2337+
}
2338+
}
2339+
2340+
// Write the server response back to syncedData
2341+
// Without the immediate flag, this would be blocked by the persisting transaction
2342+
collection.utils.writeBatch(() => {
2343+
for (const update of updates) {
2344+
collection.utils.writeUpsert(update)
2345+
}
2346+
})
2347+
2348+
// Check syncedData IMMEDIATELY after writeUpsert
2349+
syncedDataAfterWriteUpsert = collection._state.syncedData.get(`123`)
2350+
2351+
return { refetch: false }
2352+
},
2353+
}),
2354+
)
2355+
2356+
await vi.waitFor(() => {
2357+
expect(collection.status).toBe(`ready`)
2358+
})
2359+
2360+
// Verify initial state
2361+
expect(collection._state.syncedData.get(`123`)?.brandName).toBe(`A`)
2362+
2363+
// Update brandName from A to B
2364+
collection.update(`123`, (draft) => {
2365+
draft.brandName = `B`
2366+
})
2367+
2368+
// Wait for mutation to complete
2369+
await flushPromises()
2370+
await new Promise((resolve) => setTimeout(resolve, 50))
2371+
2372+
// Verify we had a persisting transaction during the write
2373+
expect(hasPersistingTransactionDuringWrite).toBe(true)
2374+
2375+
// The CRITICAL assertion: syncedData should have been updated IMMEDIATELY after writeUpsert
2376+
// Without the fix, this would fail because commitPendingTransactions() would skip
2377+
// processing due to hasPersistingTransaction being true
2378+
expect(syncedDataAfterWriteUpsert).toBeDefined()
2379+
expect(syncedDataAfterWriteUpsert?.brandName).toBe(`B`)
2380+
})
2381+
22892382
it(`should not rollback object field updates after server response with refetch: false`, async () => {
22902383
const queryKey = [`object-field-update-test`]
22912384

0 commit comments

Comments
 (0)