fix(query-db-collection): use deep equality for object field comparison (#967)

* test: add test for object field update rollback issue

Add test that verifies object field updates with refetch: false
don't rollback to previous values after server response.

* fix: use deep equality for object field comparison in query observer

Replace shallow equality (===) with deep equality when comparing items
in the query observer callback. This fixes an issue where updating
object fields with refetch: false would cause rollback to previous
values every other update.

* chore: add changeset for object field update rollback fix

Author: samwillis

.changeset/fix-object-field-update-rollback.md

@@ -0,0 +1,8 @@
+---
+"@tanstack/query-db-collection": patch
+"@tanstack/db": patch
+---
+
+fix(query-db-collection): use deep equality for object field comparison in query observer
+
+Fixed an issue where updating object fields (non-primitives) with `refetch: false` in `onUpdate` handlers would cause the value to rollback to the previous state every other update. The query observer was using shallow equality (`===`) to compare items, which compares object properties by reference rather than by value. This caused the observer to incorrectly detect differences and write stale data back to syncedData. Now uses `deepEquals` for proper value comparison.

packages/db/src/index.ts

@@ -13,6 +13,7 @@ export * from "./optimistic-action"
 export * from "./local-only"
 export * from "./local-storage"
 export * from "./errors"
+export { deepEquals } from "./utils"
 export * from "./paced-mutations"
 export * from "./strategies/index.js"
 

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

@@ -1,4 +1,5 @@
 import { QueryObserver, hashKey } from "@tanstack/query-core"
+import { deepEquals } from "@tanstack/db"
 import {
   GetKeyRequiredError,
   QueryClientRequiredError,
@@ -799,39 +800,14 @@ export function queryCollectionOptions(
 
           begin()
 
-          // Helper function for shallow equality check of objects
-          const shallowEqual = (
-            obj1: Record<string, any>,
-            obj2: Record<string, any>
-          ): boolean => {
-            // Get all keys from both objects
-            const keys1 = Object.keys(obj1)
-            const keys2 = Object.keys(obj2)
-
-            // If number of keys is different, objects are not equal
-            if (keys1.length !== keys2.length) return false
-
-            // Check if all keys in obj1 have the same values in obj2
-            return keys1.every((key) => {
-              // Skip comparing functions and complex objects deeply
-              if (typeof obj1[key] === `function`) return true
-              return obj1[key] === obj2[key]
-            })
-          }
-
           currentSyncedItems.forEach((oldItem, key) => {
             const newItem = newItemsMap.get(key)
             if (!newItem) {
               const needToRemove = removeRow(key, hashedQueryKey) // returns true if the row is no longer referenced by any queries
               if (needToRemove) {
                 write({ type: `delete`, value: oldItem })
               }
-            } else if (
-              !shallowEqual(
-                oldItem as Record<string, any>,
-                newItem as Record<string, any>
-              )
-            ) {
+            } else if (!deepEquals(oldItem, newItem)) {
               // Only update if there are actual differences in the properties
               write({ type: `update`, value: newItem })
             }

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

@@ -2112,6 +2112,98 @@ describe(`QueryCollection`, () => {
       expect(todo?.id).toBe(1)
       expect(todo?.id).not.toBe(clientId)
     })
+
+    it(`should not rollback object field updates after server response with refetch: false`, async () => {
+      const queryKey = [`object-field-update-test`]
+
+      type Todo = {
+        id: string
+        metadata: { createdBy: string }
+      }
+
+      const serverTodos: Array<Todo> = [
+        { id: `1`, metadata: { createdBy: `user1` } },
+      ]
+
+      const queryFn = vi
+        .fn()
+        .mockImplementation(() => Promise.resolve([...serverTodos]))
+
+      async function updateTodo(id: string, changes: Partial<Todo>) {
+        await new Promise((resolve) => setTimeout(resolve, 10))
+        const todo = serverTodos.find((t) => t.id === id)
+        if (todo) {
+          Object.assign(todo, changes)
+        }
+        return todo
+      }
+
+      const todosCollection = createCollection(
+        queryCollectionOptions<Todo>({
+          id: `object-field-update-test`,
+          queryKey,
+          queryFn,
+          queryClient,
+          getKey: (item: Todo) => item.id,
+          startSync: true,
+          onUpdate: async ({ transaction }) => {
+            const updates = transaction.mutations.map((m) => ({
+              id: m.key as string,
+              changes: m.changes,
+            }))
+
+            const serverItems = await Promise.all(
+              updates.map((update) => updateTodo(update.id, update.changes))
+            )
+
+            todosCollection.utils.writeBatch(() => {
+              serverItems.forEach((serverItem) => {
+                if (serverItem) {
+                  todosCollection.utils.writeUpdate(serverItem)
+                }
+              })
+            })
+
+            return { refetch: false }
+          },
+        })
+      )
+
+      await vi.waitFor(() => {
+        expect(todosCollection.status).toBe(`ready`)
+      })
+
+      // Verify initial state
+      expect(todosCollection.get(`1`)?.metadata.createdBy).toBe(`user1`)
+
+      // Update 1: change metadata from user1 to user456
+      todosCollection.update(`1`, (draft) => {
+        draft.metadata = { createdBy: `user456` }
+      })
+
+      // Wait for mutation to complete
+      await new Promise((resolve) => setTimeout(resolve, 50))
+
+      // Verify Update 1 worked
+      expect(todosCollection.get(`1`)?.metadata.createdBy).toBe(`user456`)
+      expect(
+        todosCollection._state.syncedData.get(`1`)?.metadata.createdBy
+      ).toBe(`user456`)
+
+      // Update 2: change metadata from user456 to user789
+      todosCollection.update(`1`, (draft) => {
+        draft.metadata = { createdBy: `user789` }
+      })
+
+      // Wait for mutation to complete
+      await new Promise((resolve) => setTimeout(resolve, 50))
+
+      // Verify Update 2 persisted correctly
+      expect(
+        todosCollection._state.syncedData.get(`1`)?.metadata.createdBy
+      ).toBe(`user789`)
+      expect(todosCollection.get(`1`)?.metadata.createdBy).toBe(`user789`)
+    })
   })
 
   it(`should call markReady when queryFn returns an empty array`, async () => {