fix: prevent user actions from being blocked by event filtering optimizations (#308)

Co-authored-by: Claude <[email protected]>

Author: KyleAMathews

.changeset/open-zebras-remain.md

@@ -0,0 +1,21 @@
+---
+"@tanstack/db": patch
+---
+
+Fix UI responsiveness issue with rapid user interactions in collections
+
+Fixed a critical issue where rapid user interactions (like clicking multiple checkboxes quickly) would cause the UI to become unresponsive when using collections with slow backend responses. The problem occurred when optimistic updates would back up and the UI would stop reflecting user actions.
+
+**Root Causes:**
+
+- Event filtering logic was blocking ALL events for keys with recent sync operations, including user-initiated actions
+- Event batching was queuing user actions instead of immediately updating the UI during high-frequency operations
+
+**Solution:**
+
+- Added `triggeredByUserAction` parameter to `recomputeOptimisticState()` to distinguish user actions from sync operations
+- Modified event filtering to allow user-initiated actions to bypass sync status checks
+- Enhanced `emitEvents()` with `forceEmit` parameter to skip batching for immediate user action feedback
+- Updated all user action code paths to properly identify themselves as user-triggered
+
+This ensures the UI remains responsive during rapid user interactions while maintaining the performance benefits of event batching and duplicate event filtering for sync operations.

packages/db/src/collection.ts

@@ -687,7 +687,9 @@ export class CollectionImpl<
   /**
    * Recompute optimistic state from active transactions
    */
-  private recomputeOptimisticState(): void {
+  private recomputeOptimisticState(
+    triggeredByUserAction: boolean = false
+  ): void {
     // Skip redundant recalculations when we're in the middle of committing sync transactions
     if (this.isCommittingSyncTransactions) {
       return
@@ -738,13 +740,26 @@ export class CollectionImpl<
     this.collectOptimisticChanges(previousState, previousDeletes, events)
 
     // Filter out events for recently synced keys to prevent duplicates
-    const filteredEventsBySyncStatus = events.filter(
-      (event) => !this.recentlySyncedKeys.has(event.key)
-    )
+    // BUT: Only filter out events that are actually from sync operations
+    // New user transactions should NOT be filtered even if the key was recently synced
+    const filteredEventsBySyncStatus = events.filter((event) => {
+      if (!this.recentlySyncedKeys.has(event.key)) {
+        return true // Key not recently synced, allow event through
+      }
+
+      // Key was recently synced - allow if this is a user-triggered action
+      if (triggeredByUserAction) {
+        return true
+      }
+
+      // Otherwise filter out duplicate sync events
+      return false
+    })
 
     // Filter out redundant delete events if there are pending sync transactions
     // that will immediately restore the same data, but only for completed transactions
-    if (this.pendingSyncedTransactions.length > 0) {
+    // IMPORTANT: Skip complex filtering for user-triggered actions to prevent UI blocking
+    if (this.pendingSyncedTransactions.length > 0 && !triggeredByUserAction) {
       const pendingSyncKeys = new Set<TKey>()
       const completedTransactionMutations = new Set<string>()
 
@@ -788,14 +803,14 @@ export class CollectionImpl<
       if (filteredEvents.length > 0) {
         this.updateIndexes(filteredEvents)
       }
-      this.emitEvents(filteredEvents)
+      this.emitEvents(filteredEvents, triggeredByUserAction)
     } else {
       // Update indexes for all events
       if (filteredEventsBySyncStatus.length > 0) {
         this.updateIndexes(filteredEventsBySyncStatus)
       }
       // Emit all events if no pending sync transactions
-      this.emitEvents(filteredEventsBySyncStatus)
+      this.emitEvents(filteredEventsBySyncStatus, triggeredByUserAction)
     }
   }
 
@@ -878,22 +893,21 @@ export class CollectionImpl<
    */
   private emitEvents(
     changes: Array<ChangeMessage<T, TKey>>,
-    endBatching = false
+    forceEmit = false
   ): void {
-    if (this.shouldBatchEvents && !endBatching) {
+    // Skip batching for user actions (forceEmit=true) to keep UI responsive
+    if (this.shouldBatchEvents && !forceEmit) {
       // Add events to the batch
       this.batchedEvents.push(...changes)
       return
     }
 
-    // Either we're not batching, or we're ending the batching cycle
+    // Either we're not batching, or we're forcing emission (user action or ending batch cycle)
     let eventsToEmit = changes
 
-    if (endBatching) {
-      // End batching: combine any batched events with new events and clean up state
-      if (this.batchedEvents.length > 0) {
-        eventsToEmit = [...this.batchedEvents, ...changes]
-      }
+    // If we have batched events and this is a forced emit, combine them
+    if (this.batchedEvents.length > 0 && forceEmit) {
+      eventsToEmit = [...this.batchedEvents, ...changes]
       this.batchedEvents = []
       this.shouldBatchEvents = false
     }
@@ -1625,7 +1639,7 @@ export class CollectionImpl<
       ambientTransaction.applyMutations(mutations)
 
       this.transactions.set(ambientTransaction.id, ambientTransaction)
-      this.recomputeOptimisticState()
+      this.recomputeOptimisticState(true)
 
       return ambientTransaction
     } else {
@@ -1650,7 +1664,7 @@ export class CollectionImpl<
 
       // Add the transaction to the collection's transactions store
       this.transactions.set(directOpTransaction.id, directOpTransaction)
-      this.recomputeOptimisticState()
+      this.recomputeOptimisticState(true)
 
       return directOpTransaction
     }
@@ -1847,7 +1861,7 @@ export class CollectionImpl<
       ambientTransaction.applyMutations(mutations)
 
       this.transactions.set(ambientTransaction.id, ambientTransaction)
-      this.recomputeOptimisticState()
+      this.recomputeOptimisticState(true)
 
       return ambientTransaction
     }
@@ -1876,7 +1890,7 @@ export class CollectionImpl<
     // Add the transaction to the collection's transactions store
 
     this.transactions.set(directOpTransaction.id, directOpTransaction)
-    this.recomputeOptimisticState()
+    this.recomputeOptimisticState(true)
 
     return directOpTransaction
   }
@@ -1963,7 +1977,7 @@ export class CollectionImpl<
       ambientTransaction.applyMutations(mutations)
 
       this.transactions.set(ambientTransaction.id, ambientTransaction)
-      this.recomputeOptimisticState()
+      this.recomputeOptimisticState(true)
 
       return ambientTransaction
     }
@@ -1989,7 +2003,7 @@ export class CollectionImpl<
     directOpTransaction.commit()
 
     this.transactions.set(directOpTransaction.id, directOpTransaction)
-    this.recomputeOptimisticState()
+    this.recomputeOptimisticState(true)
 
     return directOpTransaction
   }
@@ -2251,6 +2265,6 @@ export class CollectionImpl<
     // CRITICAL: Capture visible state BEFORE clearing optimistic state
     this.capturePreSyncVisibleState()
 
-    this.recomputeOptimisticState()
+    this.recomputeOptimisticState(false)
   }
 }

packages/db/tests/collection.test.ts

@@ -1248,4 +1248,120 @@ describe(`Collection with schema validation`, () => {
     expect(thirdItem!.updatedAt).toEqual(new Date(`2023-01-01T00:00:00Z`))
     expect(thirdItem!.id).toBe(`task-id-3`)
   })
+
+  it(`should not block user actions when keys are recently synced`, async () => {
+    // This test reproduces the ACTUAL issue where rapid user actions get blocked
+    // when optimistic updates back up with slow sync responses
+    const txResolvers: Array<() => void> = []
+    const emitter = mitt()
+    const changeEvents: Array<any> = []
+
+    const mutationFn = vi.fn().mockImplementation(async ({ transaction }) => {
+      // Simulate SLOW server operation - this is key to reproducing the issue
+      return new Promise((resolve) => {
+        txResolvers.push(() => {
+          emitter.emit(`sync`, transaction.mutations)
+          resolve(null)
+        })
+      })
+    })
+
+    const collection = createCollection<{ id: number; checked: boolean }>({
+      id: `user-action-blocking-test`,
+      getKey: (item) => item.id,
+      startSync: true,
+      sync: {
+        sync: ({ begin, write, commit, markReady }) => {
+          // Initialize with checkboxes
+          begin()
+          for (let i = 1; i <= 3; i++) {
+            write({
+              type: `insert`,
+              value: { id: i, checked: false },
+            })
+          }
+          commit()
+          markReady()
+
+          // Listen for sync events - this triggers the problematic batching
+          // @ts-expect-error don't trust mitt's typing
+          emitter.on(`*`, (_, changes: Array<PendingMutation>) => {
+            begin()
+            changes.forEach((change) => {
+              write({
+                type: change.type,
+                // @ts-expect-error TODO type changes
+                value: change.modified,
+              })
+            })
+            commit()
+          })
+        },
+      },
+      onUpdate: mutationFn,
+    })
+
+    // Listen to change events to verify they're emitted (this was the actual problem)
+    collection.subscribeChanges((changes) => {
+      changeEvents.push(...changes)
+    })
+
+    await collection.stateWhenReady()
+
+    // CRITICAL: Simulate rapid clicking WITHOUT waiting for transactions to complete
+    // This is what actually triggers the bug - multiple pending transactions
+
+    // Step 1: First click
+    const tx1 = collection.update(1, (draft) => {
+      draft.checked = true
+    })
+    expect(collection.state.get(1)?.checked).toBe(true)
+    const initialEventCount = changeEvents.length
+
+    // Step 2: Second click immediately (before first completes)
+    const tx2 = collection.update(1, (draft) => {
+      draft.checked = false
+    })
+    expect(collection.state.get(1)?.checked).toBe(false)
+
+    // Step 3: Third click immediately (before others complete)
+    const tx3 = collection.update(1, (draft) => {
+      draft.checked = true
+    })
+    expect(collection.state.get(1)?.checked).toBe(true)
+
+    // CRITICAL TEST: Verify events are still being emitted for rapid user actions
+    // Before the fix, these would be batched and UI would freeze
+    expect(changeEvents.length).toBeGreaterThan(initialEventCount)
+    expect(mutationFn).toHaveBeenCalledTimes(3)
+
+    // Now complete the first transaction to trigger sync and batching
+    txResolvers[0]?.()
+    await tx1.isPersisted.promise
+
+    // Step 4: More rapid clicks after sync starts (this is where the bug occurred)
+    const eventCountBeforeRapidClicks = changeEvents.length
+
+    const tx4 = collection.update(1, (draft) => {
+      draft.checked = false
+    })
+    const tx5 = collection.update(1, (draft) => {
+      draft.checked = true
+    })
+
+    // CRITICAL: Verify that even after sync/batching starts, user actions still emit events
+    expect(changeEvents.length).toBeGreaterThan(eventCountBeforeRapidClicks)
+    expect(collection.state.get(1)?.checked).toBe(true) // Last action should win
+
+    // Clean up remaining transactions
+    for (let i = 1; i < txResolvers.length; i++) {
+      txResolvers[i]?.()
+    }
+    await Promise.all([
+      tx2.isPersisted.promise,
+      tx3.isPersisted.promise,
+      tx4.isPersisted.promise,
+      tx5.isPersisted.promise,
+    ])
+  })
 })