diff --git a/.changeset/fix-select-write-operations.md b/.changeset/fix-select-write-operations.md new file mode 100644 index 000000000..bbf28f6b6 --- /dev/null +++ b/.changeset/fix-select-write-operations.md @@ -0,0 +1,9 @@ +--- +'@tanstack/query-db-collection': patch +--- + +Fix writeInsert/writeUpsert throwing error when collection uses select option + +When a Query Collection was configured with a `select` option to extract items from a wrapped API response (e.g., `{ data: [...], meta: {...} }`), calling `writeInsert()` or `writeUpsert()` would corrupt the query cache and trigger the error: "select() must return an array of objects". + +The fix routes cache updates through a new `updateCacheData` function that preserves the wrapper structure by using the `select` function to identify which property contains the items array (via reference equality), then updates only that property while keeping metadata intact. diff --git a/examples/react/paced-mutations-demo/package.json b/examples/react/paced-mutations-demo/package.json index 850bb2f95..1bace882e 100644 --- a/examples/react/paced-mutations-demo/package.json +++ b/examples/react/paced-mutations-demo/package.json @@ -10,7 +10,7 @@ }, "dependencies": { "@tanstack/db": "^0.5.11", - "@tanstack/react-db": "^0.1.55", + "@tanstack/react-db": "^0.1.56", "mitt": "^3.0.1", "react": "^19.2.1", "react-dom": "^19.2.1" diff --git a/examples/react/todo/package.json b/examples/react/todo/package.json index eb81b798c..3dc6432fa 100644 --- a/examples/react/todo/package.json +++ b/examples/react/todo/package.json @@ -5,8 +5,8 @@ "dependencies": { "@tanstack/electric-db-collection": "^0.2.12", "@tanstack/query-core": "^5.90.12", - "@tanstack/query-db-collection": "^1.0.6", - "@tanstack/react-db": "^0.1.55", + "@tanstack/query-db-collection": "^1.0.7", + "@tanstack/react-db": "^0.1.56", "@tanstack/react-router": "^1.140.0", "@tanstack/react-start": "^1.140.0", "@tanstack/trailbase-db-collection": "^0.1.55", diff --git a/examples/solid/todo/package.json b/examples/solid/todo/package.json index 5fb3d60ef..438b61a11 100644 --- a/examples/solid/todo/package.json +++ b/examples/solid/todo/package.json @@ -5,7 +5,7 @@ "dependencies": { "@tanstack/electric-db-collection": "^0.2.12", "@tanstack/query-core": "^5.90.12", - "@tanstack/query-db-collection": "^1.0.6", + "@tanstack/query-db-collection": "^1.0.7", "@tanstack/solid-db": "^0.1.54", "@tanstack/solid-router": "^1.140.0", "@tanstack/solid-start": "^1.140.0", diff --git a/packages/query-db-collection/src/manual-sync.ts b/packages/query-db-collection/src/manual-sync.ts index c5d3c0874..b772550e3 100644 --- a/packages/query-db-collection/src/manual-sync.ts +++ b/packages/query-db-collection/src/manual-sync.ts @@ -38,6 +38,12 @@ export interface SyncContext< begin: () => void write: (message: Omit, `key`>) => void commit: () => void + /** + * Optional function to update the query cache with the latest synced data. + * Handles both direct array caches and wrapped response formats (when `select` is used). + * If not provided, falls back to directly setting the cache with the raw array. + */ + updateCacheData?: (items: Array) => void } interface NormalizedOperation< @@ -205,7 +211,12 @@ export function performWriteOperations< // Update query cache after successful commit const updatedData = Array.from(ctx.collection._state.syncedData.values()) - ctx.queryClient.setQueryData(ctx.queryKey, updatedData) + if (ctx.updateCacheData) { + ctx.updateCacheData(updatedData) + } else { + // Fallback: directly set the cache with raw array (for non-Query Collection consumers) + ctx.queryClient.setQueryData(ctx.queryKey, updatedData) + } } // Factory function to create write utils diff --git a/packages/query-db-collection/src/query.ts b/packages/query-db-collection/src/query.ts index 2c81267dd..4cbdae5a2 100644 --- a/packages/query-db-collection/src/query.ts +++ b/packages/query-db-collection/src/query.ts @@ -1130,6 +1130,73 @@ export function queryCollectionOptions( await Promise.all(refetchPromises) } + /** + * Updates the query cache with new items, handling both direct arrays + * and wrapped response formats (when `select` is used). + */ + const updateCacheData = (items: Array): void => { + // Get the base query key (handle both static and function-based keys) + const key = + typeof queryKey === `function` + ? queryKey({}) + : (queryKey as unknown as QueryKey) + + if (select) { + // When `select` is used, the cache contains a wrapped response (e.g., { data: [...], meta: {...} }) + // We need to update the cache while preserving the wrapper structure + queryClient.setQueryData(key, (oldData: any) => { + if (!oldData || typeof oldData !== `object`) { + // No existing cache or not an object - don't corrupt the cache + return oldData + } + + if (Array.isArray(oldData)) { + // Cache is already a raw array (shouldn't happen with select, but handle it) + return items + } + + // Use the select function to identify which property contains the items array. + // This is more robust than guessing based on property order. + const selectedArray = select(oldData) + + if (Array.isArray(selectedArray)) { + // Find the property that matches the selected array by reference equality + for (const propKey of Object.keys(oldData)) { + if (oldData[propKey] === selectedArray) { + // Found the exact property - create a shallow copy with updated items + return { ...oldData, [propKey]: items } + } + } + } + + // Fallback: check common property names used for data arrays + if (Array.isArray(oldData.data)) { + return { ...oldData, data: items } + } + if (Array.isArray(oldData.items)) { + return { ...oldData, items: items } + } + if (Array.isArray(oldData.results)) { + return { ...oldData, results: items } + } + + // Last resort: find first array property + for (const propKey of Object.keys(oldData)) { + if (Array.isArray(oldData[propKey])) { + return { ...oldData, [propKey]: items } + } + } + + // Couldn't safely identify the array property - don't corrupt the cache + // Return oldData unchanged to avoid breaking select + return oldData + }) + } else { + // No select - cache contains raw array, just set it directly + queryClient.setQueryData(key, items) + } + } + // Create write context for manual write operations let writeContext: { collection: any @@ -1139,21 +1206,29 @@ export function queryCollectionOptions( begin: () => void write: (message: Omit, `key`>) => void commit: () => void + updateCacheData?: (items: Array) => void } | null = null // Enhanced internalSync that captures write functions for manual use const enhancedInternalSync: SyncConfig[`sync`] = (params) => { const { begin, write, commit, collection } = params + // Get the base query key for the context (handle both static and function-based keys) + const contextQueryKey = + typeof queryKey === `function` + ? (queryKey({}) as unknown as Array) + : (queryKey as unknown as Array) + // Store references for manual write operations writeContext = { collection, queryClient, - queryKey: queryKey as unknown as Array, + queryKey: contextQueryKey, getKey: getKey as (item: any) => string | number, begin, write, commit, + updateCacheData, } // Call the original internalSync logic diff --git a/packages/query-db-collection/tests/query.test.ts b/packages/query-db-collection/tests/query.test.ts index 429547f43..1845641aa 100644 --- a/packages/query-db-collection/tests/query.test.ts +++ b/packages/query-db-collection/tests/query.test.ts @@ -692,6 +692,179 @@ describe(`QueryCollection`, () => { ) as MetaDataType expect(initialCache).toEqual(initialMetaData) }) + + it(`should not throw error when using writeInsert with select option`, async () => { + const queryKey = [`select-writeInsert-test`] + const consoleErrorSpy = vi + .spyOn(console, `error`) + .mockImplementation(() => {}) + + const queryFn = vi.fn().mockResolvedValue(initialMetaData) + const select = vi.fn((data: MetaDataType) => data.data) + + const options = queryCollectionOptions({ + id: `select-writeInsert-test`, + queryClient, + queryKey, + queryFn, + select, + getKey, + startSync: true, + }) + const collection = createCollection(options) + + // Wait for collection to be ready + await vi.waitFor(() => { + expect(collection.status).toBe(`ready`) + expect(collection.size).toBe(2) + }) + + // This should NOT cause an error - but with the bug it does + const newItem: TestItem = { id: `3`, name: `New Item` } + collection.utils.writeInsert(newItem) + + // Verify the item was inserted + expect(collection.size).toBe(3) + expect(collection.get(`3`)).toEqual(newItem) + + // Wait a tick to allow any async error handlers to run + await flushPromises() + + // Verify no error was logged about select returning non-array + const errorCallArgs = consoleErrorSpy.mock.calls.find((call) => + call[0]?.includes?.( + `@tanstack/query-db-collection: select() must return an array of objects`, + ), + ) + expect(errorCallArgs).toBeUndefined() + + consoleErrorSpy.mockRestore() + }) + + it(`should not throw error when using writeUpsert with select option`, async () => { + const queryKey = [`select-writeUpsert-test`] + const consoleErrorSpy = vi + .spyOn(console, `error`) + .mockImplementation(() => {}) + + const queryFn = vi.fn().mockResolvedValue(initialMetaData) + const select = vi.fn((data: MetaDataType) => data.data) + + const options = queryCollectionOptions({ + id: `select-writeUpsert-test`, + queryClient, + queryKey, + queryFn, + select, + getKey, + startSync: true, + }) + const collection = createCollection(options) + + // Wait for collection to be ready + await vi.waitFor(() => { + expect(collection.status).toBe(`ready`) + expect(collection.size).toBe(2) + }) + + // This should NOT cause an error - but with the bug it does + // Test upsert for new item + const newItem: TestItem = { id: `3`, name: `Upserted New Item` } + collection.utils.writeUpsert(newItem) + + // Verify the item was inserted + expect(collection.size).toBe(3) + expect(collection.get(`3`)).toEqual(newItem) + + // Test upsert for existing item + collection.utils.writeUpsert({ id: `1`, name: `Updated First Item` }) + + // Verify the item was updated + expect(collection.get(`1`)?.name).toBe(`Updated First Item`) + + // Wait a tick to allow any async error handlers to run + await flushPromises() + + // Verify no error was logged about select returning non-array + const errorCallArgs = consoleErrorSpy.mock.calls.find((call) => + call[0]?.includes?.( + `@tanstack/query-db-collection: select() must return an array of objects`, + ), + ) + expect(errorCallArgs).toBeUndefined() + + consoleErrorSpy.mockRestore() + }) + + it(`should update query cache with wrapped format preserved when using writeInsert with select option`, async () => { + const queryKey = [`select-cache-update-test`] + + const queryFn = vi.fn().mockResolvedValue(initialMetaData) + const select = vi.fn((data: MetaDataType) => data.data) + + const options = queryCollectionOptions({ + id: `select-cache-update-test`, + queryClient, + queryKey, + queryFn, + select, + getKey, + startSync: true, + }) + const collection = createCollection(options) + + // Wait for collection to be ready + await vi.waitFor(() => { + expect(collection.status).toBe(`ready`) + expect(collection.size).toBe(2) + }) + + // Verify initial cache has wrapped format + const initialCache = queryClient.getQueryData( + queryKey, + ) as MetaDataType + expect(initialCache.metaDataOne).toBe(`example metadata`) + expect(initialCache.metaDataTwo).toBe(`example metadata`) + expect(initialCache.data).toHaveLength(2) + + // Insert a new item + const newItem: TestItem = { id: `3`, name: `New Item` } + collection.utils.writeInsert(newItem) + + // Verify the cache still has wrapped format with metadata preserved + const cacheAfterInsert = queryClient.getQueryData( + queryKey, + ) as MetaDataType + expect(cacheAfterInsert.metaDataOne).toBe(`example metadata`) + expect(cacheAfterInsert.metaDataTwo).toBe(`example metadata`) + expect(cacheAfterInsert.data).toHaveLength(3) + expect(cacheAfterInsert.data).toContainEqual(newItem) + + // Update an existing item + collection.utils.writeUpdate({ id: `1`, name: `Updated First Item` }) + + // Verify the cache still has wrapped format + const cacheAfterUpdate = queryClient.getQueryData( + queryKey, + ) as MetaDataType + expect(cacheAfterUpdate.metaDataOne).toBe(`example metadata`) + expect(cacheAfterUpdate.data).toHaveLength(3) + const updatedItem = cacheAfterUpdate.data.find((item) => item.id === `1`) + expect(updatedItem?.name).toBe(`Updated First Item`) + + // Delete an item + collection.utils.writeDelete(`2`) + + // Verify the cache still has wrapped format + const cacheAfterDelete = queryClient.getQueryData( + queryKey, + ) as MetaDataType + expect(cacheAfterDelete.metaDataOne).toBe(`example metadata`) + expect(cacheAfterDelete.data).toHaveLength(2) + expect(cacheAfterDelete.data).not.toContainEqual( + expect.objectContaining({ id: `2` }), + ) + }) }) describe(`Direct persistence handlers`, () => { it(`should pass through direct persistence handlers to collection options`, () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c6a532eb9..7d45b0b0e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -257,7 +257,7 @@ importers: specifier: ^0.5.11 version: link:../../../packages/db '@tanstack/react-db': - specifier: ^0.1.55 + specifier: ^0.1.56 version: link:../../../packages/react-db mitt: specifier: ^3.0.1 @@ -433,10 +433,10 @@ importers: specifier: ^5.90.12 version: 5.90.12 '@tanstack/query-db-collection': - specifier: ^1.0.6 + specifier: ^1.0.7 version: link:../../../packages/query-db-collection '@tanstack/react-db': - specifier: ^0.1.55 + specifier: ^0.1.56 version: link:../../../packages/react-db '@tanstack/react-router': specifier: ^1.140.0 @@ -554,7 +554,7 @@ importers: specifier: ^5.90.12 version: 5.90.12 '@tanstack/query-db-collection': - specifier: ^1.0.6 + specifier: ^1.0.7 version: link:../../../packages/query-db-collection '@tanstack/solid-db': specifier: ^0.1.54