Skip to content

Commit 3d65bb1

Browse files
kevin-dpclaudeautofix-ci[bot]
authored
fix: prevent stale query cache from re-inserting deleted items (#1387)
* test: add stale cache consistency test for deleted item re-insertion Adds a test that verifies deleted items are not re-inserted into syncedData when a destroyed query observer is recreated and picks up stale TanStack Query cache data (gcTime > 0). Reproduces the scenario from #1354. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * ci: apply automated fixes * fix: update stale query cache entries on manual writes to prevent ghost items updateCacheData previously only updated active query cache entries tracked in hashToQueryKey. Stale cache entries from destroyed observers (kept alive by gcTime > 0) were never updated, causing syncedData and the TQ cache to diverge. When a new observer later picked up stale cache data, makeQueryResultHandler would re-insert deleted items or revert updated values. Now uses queryCache.findAll({ queryKey: baseKey }) to update all matching cache entries (active and stale). Also adds prefix validation for function-based queryKey to ensure derived keys extend the base key prefix. Fixes #1354 Fixes #1385 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: add changeset for stale cache fix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent b8abc02 commit 3d65bb1

File tree

3 files changed

+258
-15
lines changed

3 files changed

+258
-15
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/query-db-collection': patch
3+
---
4+
5+
fix: Prevent stale query cache from re-inserting deleted items when a destroyed observer is recreated with gcTime > 0.

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

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,33 @@ export function queryCollectionOptions(
558558
// Default to eager sync mode if not provided
559559
const syncMode = baseCollectionConfig.syncMode ?? `eager`
560560

561+
// Compute the base query key once for cache lookups.
562+
// All derived keys (from on-demand predicates or function-based queryKey) must
563+
// share this prefix so that queryCache.findAll({ queryKey: baseKey }) can find them.
564+
const baseKey: QueryKey =
565+
typeof queryKey === `function`
566+
? (queryKey({}) as unknown as QueryKey)
567+
: (queryKey as unknown as QueryKey)
568+
569+
/**
570+
* Validates that a derived query key extends the base key prefix.
571+
* TanStack Query uses prefix matching in findAll(), so all keys for this collection
572+
* must start with baseKey for stale cache updates to work correctly.
573+
*/
574+
const validateQueryKeyPrefix = (key: QueryKey): void => {
575+
if (typeof queryKey !== `function`) return
576+
const isValidPrefix =
577+
key.length >= baseKey.length &&
578+
baseKey.every((segment, i) => deepEquals(segment, key[i]))
579+
if (!isValidPrefix) {
580+
console.warn(
581+
`[QueryCollection] queryKey function must return keys that extend the base key prefix. ` +
582+
`Base: ${JSON.stringify(baseKey)}, Got: ${JSON.stringify(key)}. ` +
583+
`This can cause stale cache issues.`,
584+
)
585+
}
586+
}
587+
561588
// Validate required parameters
562589

563590
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
@@ -680,6 +707,8 @@ export function queryCollectionOptions(
680707
const hashedQueryKey = hashKey(key)
681708
const extendedMeta = { ...meta, loadSubsetOptions: opts }
682709

710+
validateQueryKeyPrefix(key)
711+
683712
if (state.observers.has(hashedQueryKey)) {
684713
// We already have a query for this queryKey
685714
// Increment reference count since another consumer is using this observer
@@ -1233,26 +1262,28 @@ export function queryCollectionOptions(
12331262
}
12341263

12351264
/**
1236-
* Updates the query cache with new items for ALL active query keys.
1237-
* This is critical for on-demand mode where multiple query keys may exist
1238-
* (each with different predicates).
1265+
* Updates the query cache with new items for ALL query keys matching this collection,
1266+
* including stale/inactive cache entries from destroyed observers.
1267+
*
1268+
* This prevents ghost items: when an observer is destroyed but gcTime > 0, TanStack Query
1269+
* keeps the cached data. If syncedData changes (via writeDelete/writeInsert/writeUpdate)
1270+
* after the observer is destroyed, the stale cache becomes inconsistent. When a new observer
1271+
* later picks up this stale cache, makeQueryResultHandler would create spurious sync
1272+
* operations (re-inserting deleted items, reverting updated values, etc).
1273+
*
1274+
* By updating all cache entries (active and stale), we ensure the cache always reflects
1275+
* the current syncedData state.
12391276
*/
12401277
const updateCacheData = (items: Array<any>): void => {
1241-
// Get all active query keys from the hashToQueryKey map
1242-
const activeQueryKeys = Array.from(hashToQueryKey.values())
1278+
const allCached = queryClient.getQueryCache().findAll({ queryKey: baseKey })
12431279

1244-
if (activeQueryKeys.length > 0) {
1245-
// Update all active query keys in the cache
1246-
for (const key of activeQueryKeys) {
1247-
updateCacheDataForKey(key, items)
1280+
if (allCached.length > 0) {
1281+
for (const query of allCached) {
1282+
updateCacheDataForKey(query.queryKey, items)
12481283
}
12491284
} else {
1250-
// Fallback: no active queries yet, use the base query key
1251-
// This handles the case where updateCacheData is called before any queries are created
1252-
const baseKey =
1253-
typeof queryKey === `function`
1254-
? queryKey({})
1255-
: (queryKey as unknown as QueryKey)
1285+
// Fallback: no queries in cache yet, seed the base query key.
1286+
// This handles the case where updateCacheData is called before any queries are created.
12561287
updateCacheDataForKey(baseKey, items)
12571288
}
12581289
}

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

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
createLiveQueryCollection,
66
eq,
77
ilike,
8+
inArray,
89
or,
910
} from '@tanstack/db'
1011
import { stripVirtualProps } from '../../db/tests/utils'
@@ -5207,4 +5208,210 @@ describe(`QueryCollection`, () => {
52075208
customQueryClient.clear()
52085209
})
52095210
})
5211+
5212+
describe('Stale cache consistency', () => {
5213+
const sleep = (ms: number) =>
5214+
new Promise((resolve) => setTimeout(resolve, ms))
5215+
5216+
it('should not re-insert deleted item when a destroyed query is recreated with stale cache', async () => {
5217+
const customQueryClient = new QueryClient({
5218+
defaultOptions: {
5219+
queries: {
5220+
staleTime: Infinity,
5221+
gcTime: Infinity,
5222+
retry: false,
5223+
refetchOnWindowFocus: false,
5224+
refetchOnMount: false,
5225+
refetchOnReconnect: false,
5226+
},
5227+
},
5228+
})
5229+
5230+
interface Assignment {
5231+
id: number
5232+
task_id: number
5233+
resource_id: number
5234+
hours_per_day: number
5235+
}
5236+
5237+
const MUTATION_DELAY = 50
5238+
5239+
let serverItems: Array<Assignment> = [
5240+
{ id: 1, task_id: 10, resource_id: 1, hours_per_day: 8 },
5241+
{ id: 2, task_id: 10, resource_id: 2, hours_per_day: 4 },
5242+
{ id: 3, task_id: 20, resource_id: 3, hours_per_day: 8 },
5243+
]
5244+
let nextId = 100
5245+
5246+
const apiCreate = async (
5247+
item: Omit<Assignment, 'id'>,
5248+
): Promise<Assignment> => {
5249+
await sleep(MUTATION_DELAY)
5250+
const created = { ...item, id: nextId++ }
5251+
serverItems.push(created)
5252+
return created
5253+
}
5254+
5255+
const apiDelete = async (id: number): Promise<void> => {
5256+
await sleep(MUTATION_DELAY)
5257+
serverItems = serverItems.filter((a) => a.id !== id)
5258+
}
5259+
5260+
const collection = createCollection(
5261+
queryCollectionOptions<Assignment>({
5262+
id: `stale-cache-${Date.now()}-${Math.random()}`,
5263+
queryClient: customQueryClient,
5264+
queryKey: ['stale-cache', String(Date.now()), String(Math.random())],
5265+
queryFn: async () => [...serverItems],
5266+
getKey: (item) => item.id,
5267+
syncMode: 'on-demand',
5268+
startSync: true,
5269+
5270+
onInsert: async ({ transaction, collection: col }) => {
5271+
const { id: _id, ...rest } = transaction.mutations[0].modified
5272+
const serverItem = await apiCreate(rest)
5273+
col.utils.writeInsert(serverItem)
5274+
return { refetch: false }
5275+
},
5276+
5277+
onDelete: async ({ transaction, collection: col }) => {
5278+
const id = transaction.mutations[0].key as number
5279+
await apiDelete(id)
5280+
col.utils.writeDelete(id)
5281+
return { refetch: false }
5282+
},
5283+
}),
5284+
)
5285+
5286+
// Always-active queries
5287+
const taskQuery = createLiveQueryCollection({
5288+
query: (q) =>
5289+
q
5290+
.from({ assignment: collection })
5291+
.where(({ assignment }) => inArray(assignment.task_id, [10])),
5292+
})
5293+
await taskQuery.preload()
5294+
5295+
const projectQuery = createLiveQueryCollection({
5296+
query: (q) =>
5297+
q
5298+
.from({ assignment: collection })
5299+
.where(({ assignment }) => eq(assignment.task_id, 10)),
5300+
})
5301+
await projectQuery.preload()
5302+
5303+
await vi.waitFor(() => {
5304+
expect(collection.size).toBe(3)
5305+
})
5306+
5307+
// Step 1: Toggle ON → add item → toggle OFF
5308+
let workloadQuery = createLiveQueryCollection({
5309+
query: (q) =>
5310+
q
5311+
.from({ assignment: collection })
5312+
.where(({ assignment }) =>
5313+
inArray(assignment.resource_id, [1, 2, 3, 4]),
5314+
),
5315+
})
5316+
await workloadQuery.preload()
5317+
5318+
collection.insert({
5319+
id: -1,
5320+
task_id: 10,
5321+
resource_id: 4,
5322+
hours_per_day: 8,
5323+
})
5324+
5325+
workloadQuery.cleanup()
5326+
workloadQuery = createLiveQueryCollection({
5327+
query: (q) =>
5328+
q
5329+
.from({ assignment: collection })
5330+
.where(({ assignment }) =>
5331+
inArray(assignment.resource_id, [1, 2, 4]),
5332+
),
5333+
})
5334+
await workloadQuery.preload()
5335+
await flushPromises()
5336+
await sleep(MUTATION_DELAY + 50)
5337+
await flushPromises()
5338+
5339+
const carolId = 100
5340+
expect(collection.has(carolId)).toBe(true)
5341+
5342+
// Step 2: Delete
5343+
collection.delete(carolId)
5344+
5345+
workloadQuery.cleanup()
5346+
workloadQuery = createLiveQueryCollection({
5347+
query: (q) =>
5348+
q
5349+
.from({ assignment: collection })
5350+
.where(({ assignment }) => inArray(assignment.resource_id, [1, 2])),
5351+
})
5352+
await workloadQuery.preload()
5353+
await flushPromises()
5354+
await sleep(MUTATION_DELAY + 50)
5355+
await flushPromises()
5356+
5357+
expect(collection.has(carolId)).toBe(false)
5358+
expect(collection._state.syncedData.has(carolId)).toBe(false)
5359+
5360+
// Step 3: Toggle ON again (stale cache)
5361+
workloadQuery.cleanup()
5362+
workloadQuery = createLiveQueryCollection({
5363+
query: (q) =>
5364+
q
5365+
.from({ assignment: collection })
5366+
.where(({ assignment }) =>
5367+
inArray(assignment.resource_id, [1, 2, 3, 4]),
5368+
),
5369+
})
5370+
await workloadQuery.preload()
5371+
await flushPromises()
5372+
5373+
// Step 4: Re-add → toggle OFF
5374+
const t2 = collection.insert({
5375+
id: -2,
5376+
task_id: 10,
5377+
resource_id: 4,
5378+
hours_per_day: 8,
5379+
})
5380+
5381+
workloadQuery.cleanup()
5382+
workloadQuery = createLiveQueryCollection({
5383+
query: (q) =>
5384+
q
5385+
.from({ assignment: collection })
5386+
.where(({ assignment }) =>
5387+
inArray(assignment.resource_id, [1, 2, 4]),
5388+
),
5389+
})
5390+
await workloadQuery.preload()
5391+
await flushPromises()
5392+
5393+
await t2.isPersisted.promise
5394+
await flushPromises()
5395+
await sleep(50)
5396+
5397+
// The deleted item (id=100) must NOT be in syncedData
5398+
expect(collection._state.syncedData.has(carolId)).toBe(false)
5399+
5400+
// The new item (id=101) should exist
5401+
expect(collection.has(101)).toBe(true)
5402+
5403+
// Only one assignment with resource_id=4
5404+
const allItems = Array.from(collection.values())
5405+
const carolAssignments = allItems.filter((a) => a.resource_id === 4)
5406+
expect(carolAssignments).toHaveLength(1)
5407+
expect(carolAssignments[0]?.id).toBe(101)
5408+
5409+
expect(collection.size).toBe(4)
5410+
5411+
workloadQuery.cleanup()
5412+
taskQuery.cleanup()
5413+
projectQuery.cleanup()
5414+
customQueryClient.clear()
5415+
})
5416+
})
52105417
})

0 commit comments

Comments
 (0)