Skip to content

Commit 5582d36

Browse files
committed
fix(react-db): idiomatic useLiveQuery & useLiveSuspenseQuery hooks
These hooks where using refs to track previous versions of certain variables, and reading those during render, which is an anti-pattern that breaks the Rules of Hooks and may lead to subtle bugs, especially in concurrent mode. Using state instead is more idiomatic and ensures there will be no state tearing, even during concurrent mode updates.
1 parent 9005885 commit 5582d36

3 files changed

Lines changed: 101 additions & 82 deletions

File tree

‎.changeset/eleven-gifts-shine.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@tanstack/react-db': patch
3+
---
4+
5+
fix(react-db): Avoid using refs to track changes in React hooks.
6+
7+
Using refs to track previous versions of variables, and reading those refs
8+
during render, is an anti-pattern that breaks the Rules of Hooks and may lead to
9+
subtle bugs, especially in concurrent mode.
10+
11+
Using state instead is more idiomatic and ensures there will be no state
12+
tearing, even during concurrent mode updates.

‎packages/react-db/src/useLiveQuery.ts‎

Lines changed: 63 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useRef, useSyncExternalStore } from 'react'
1+
import { useCallback, useState, useSyncExternalStore } from 'react'
22
import {
33
BaseQueryBuilder,
44
createLiveQueryCollection,
@@ -13,7 +13,6 @@ import type {
1313
InferResultType,
1414
InitialQueryBuilder,
1515
LiveQueryCollectionConfig,
16-
LiveQueryObserver,
1716
NonSingleResult,
1817
QueryBuilder,
1918
SingleResult,
@@ -321,26 +320,24 @@ export function useLiveQuery(
321320
// Check if it's already a collection
322321
const inputIsCollection = isCollection(configOrQueryOrCollection)
323322

324-
// Use refs to cache collection and track dependencies
325-
const collectionRef = useRef<Collection<object, string | number, {}> | null>(
326-
null,
327-
)
328-
const depsRef = useRef<Array<unknown> | null>(null)
329-
const configRef = useRef<unknown>(null)
330-
331-
// The shared observer owns subscription, the ready-race, and the snapshot.
332-
const observerRef = useRef<LiveQueryObserver<object, string | number> | null>(
333-
null,
334-
)
323+
// Use state to cache collection and track dependencies
324+
const [collection, setCollection] = useState<Collection<
325+
object,
326+
string | number,
327+
{}
328+
> | null>(null)
329+
const [prevCollection, setPrevCollection] = useState(collection)
330+
const [prevDeps, setPrevDeps] = useState<Array<unknown> | null>(null)
331+
const [prevConfig, setPrevConfig] = useState<unknown>(null)
335332

336333
// Check if we need to create/recreate the collection
337334
const needsNewCollection =
338-
!collectionRef.current ||
339-
(inputIsCollection && configRef.current !== configOrQueryOrCollection) ||
335+
!collection ||
336+
(inputIsCollection && prevConfig !== configOrQueryOrCollection) ||
340337
(!inputIsCollection &&
341-
(depsRef.current === null ||
342-
depsRef.current.length !== deps.length ||
343-
depsRef.current.some((dep, i) => dep !== deps[i])))
338+
(prevDeps === null ||
339+
prevDeps.length !== deps.length ||
340+
prevDeps.some((dep, i) => dep !== deps[i])))
344341

345342
if (needsNewCollection) {
346343
if (inputIsCollection) {
@@ -361,8 +358,8 @@ export function useLiveQuery(
361358
}
362359
// It's already a collection, ensure sync is started for React hooks
363360
configOrQueryOrCollection.startSyncImmediate()
364-
collectionRef.current = configOrQueryOrCollection
365-
configRef.current = configOrQueryOrCollection
361+
setCollection(configOrQueryOrCollection)
362+
setPrevConfig(configOrQueryOrCollection)
366363
} else {
367364
// Handle different callback return types
368365
if (typeof configOrQueryOrCollection === `function`) {
@@ -372,77 +369,83 @@ export function useLiveQuery(
372369

373370
if (result === undefined || result === null) {
374371
// Callback returned undefined/null - disabled query
375-
collectionRef.current = null
372+
setCollection(null)
376373
} else if (isCollection(result)) {
377374
// Callback returned a Collection instance - use it directly
378375
result.startSyncImmediate()
379-
collectionRef.current = result
376+
setCollection(result)
380377
} else if (result instanceof BaseQueryBuilder) {
381378
// Callback returned QueryBuilder - create live query collection using the original callback
382379
// (not the result, since the result might be from a different query builder instance)
383-
collectionRef.current = createLiveQueryCollection({
384-
query: configOrQueryOrCollection,
385-
startSync: true,
386-
gcTime: DEFAULT_GC_TIME_MS,
387-
})
380+
setCollection(
381+
createLiveQueryCollection({
382+
query: configOrQueryOrCollection,
383+
startSync: true,
384+
gcTime: DEFAULT_GC_TIME_MS,
385+
}),
386+
)
388387
} else if (result && typeof result === `object`) {
389388
// Assume it's a LiveQueryCollectionConfig
390-
collectionRef.current = createLiveQueryCollection({
391-
startSync: true,
392-
gcTime: DEFAULT_GC_TIME_MS,
393-
...result,
394-
})
389+
setCollection(
390+
createLiveQueryCollection({
391+
startSync: true,
392+
gcTime: DEFAULT_GC_TIME_MS,
393+
...result,
394+
}),
395+
)
395396
} else {
396397
// Unexpected return type
397398
throw new Error(
398399
`useLiveQuery callback must return a QueryBuilder, LiveQueryCollectionConfig, Collection, undefined, or null. Got: ${typeof result}`,
399400
)
400401
}
401-
depsRef.current = [...deps]
402+
setPrevDeps([...deps])
402403
} else {
403404
// Original logic for config objects
404-
collectionRef.current = createLiveQueryCollection({
405-
startSync: true,
406-
gcTime: DEFAULT_GC_TIME_MS,
407-
...configOrQueryOrCollection,
408-
})
409-
depsRef.current = [...deps]
405+
setCollection(
406+
createLiveQueryCollection({
407+
startSync: true,
408+
gcTime: DEFAULT_GC_TIME_MS,
409+
...configOrQueryOrCollection,
410+
}),
411+
)
412+
setPrevDeps([...deps])
410413
}
411414
}
412415
}
413416

414-
// Recreate the observer when the underlying collection changes. The observer
415-
// is not disposed explicitly here or on unmount: `useSyncExternalStore`
416-
// unsubscribes it when the subscribe changes or the component unmounts, which
417-
// detaches the collection subscription; the observer is then GC'd. (An unmount
418-
// effect that disposed it would misfire under StrictMode/offscreen effect
419-
// replay, leaving a disposed observer in the ref.)
420-
if (needsNewCollection) {
417+
// The shared observer owns subscription, the ready-race, and the snapshot.
418+
const [observer, setObserver] = useState(() =>
421419
// Defer the initial notify: useSyncExternalStore must not be notified
422420
// synchronously during subscribe.
423421
// Wholesale mode: React re-reads getSnapshot() on notify, keeps the
424422
// hook's pre-observer loading policy, and — because wholesale delivers
425423
// nothing synchronously during subscribe — never notifies
426424
// useSyncExternalStore inside its own subscribe call.
427-
observerRef.current = createLiveQueryObserver(collectionRef.current, {
428-
mode: `wholesale`,
429-
})
425+
createLiveQueryObserver(collection, { mode: `wholesale` }),
426+
)
427+
428+
// Recreate the observer when the underlying collection changes. The observer
429+
// is not disposed explicitly here or on unmount: `useSyncExternalStore`
430+
// unsubscribes it when the subscribe changes or the component unmounts, which
431+
// detaches the collection subscription; the observer is then GC'd. (An unmount
432+
// effect that disposed it would misfire under StrictMode/offscreen effect
433+
// replay, leaving a disposed observer in the ref.)
434+
if (prevCollection !== collection) {
435+
setPrevCollection(collection)
436+
setObserver(createLiveQueryObserver(collection, { mode: `wholesale` }))
430437
}
431-
const observer = observerRef.current!
432438

433439
// Stable subscribe bound to the current observer; the observer owns the
434440
// subscription, ready-race, and disposal.
435-
const subscribeRef = useRef<
436-
((onStoreChange: () => void) => () => void) | null
437-
>(null)
438-
if (!subscribeRef.current || needsNewCollection) {
439-
subscribeRef.current = (onStoreChange) =>
440-
observer.subscribe(() => onStoreChange())
441-
}
441+
const subscribe = useCallback(
442+
(onStoreChange: () => void) => observer.subscribe(() => onStoreChange()),
443+
[observer],
444+
)
445+
446+
const getSnapshot = useCallback(() => observer.getSnapshot(), [observer])
442447

443448
// The observer returns a stable snapshot per revision, which is the return
444449
// shape this hook exposes. Keep the return loose to satisfy the overloads.
445-
return useSyncExternalStore(subscribeRef.current, () =>
446-
observer.getSnapshot(),
447-
) as any
450+
return useSyncExternalStore(subscribe, getSnapshot) as any
448451
}

‎packages/react-db/src/useLiveSuspenseQuery.ts‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useRef } from 'react'
1+
import { useMemo, useState } from 'react'
22
import { useLiveQuery } from './useLiveQuery'
33
import type {
44
Collection,
@@ -12,6 +12,8 @@ import type {
1212
SingleResult,
1313
} from '@tanstack/db'
1414

15+
const safePromiseCache = new WeakMap<Promise<void>, Promise<void>>()
16+
1517
/**
1618
* Create a live query with React Suspense support
1719
* @param queryFn - Query function that defines what data to fetch
@@ -156,18 +158,16 @@ export function useLiveSuspenseQuery(
156158
configOrQueryOrCollection: any,
157159
deps: Array<unknown> = [],
158160
) {
159-
const promiseRef = useRef<Promise<void> | null>(null)
160-
const collectionRef = useRef<Collection<any, any, any> | null>(null)
161-
const hasBeenReadyRef = useRef(false)
162-
163161
// Use useLiveQuery to handle collection management and reactivity
164162
const result = useLiveQuery(configOrQueryOrCollection, deps)
165163

164+
const [prevCollection, setPrevCollection] = useState(result.collection)
165+
const [hasBeenReady, setHasBeenReady] = useState(false)
166+
166167
// Reset promise and ready state when collection changes (deps changed)
167-
if (collectionRef.current !== result.collection) {
168-
promiseRef.current = null
169-
collectionRef.current = result.collection
170-
hasBeenReadyRef.current = false
168+
if (prevCollection !== result.collection) {
169+
setPrevCollection(result.collection)
170+
setHasBeenReady(false)
171171
}
172172

173173
// SUSPENSE LOGIC: Throw promise or error based on collection status
@@ -188,36 +188,40 @@ export function useLiveSuspenseQuery(
188188
const collectionStatus = result.collection.status
189189

190190
// Track when we reach ready state
191-
if (collectionStatus === `ready`) {
192-
hasBeenReadyRef.current = true
193-
promiseRef.current = null
191+
if (collectionStatus === `ready` && !hasBeenReady) {
192+
setHasBeenReady(true)
194193
}
195194

196195
// Only throw errors during initial load (before first ready)
197196
// After success, errors surface as stale data (matches TanStack Query behavior)
198-
if (collectionStatus === `error` && !hasBeenReadyRef.current) {
199-
promiseRef.current = null
197+
if (collectionStatus === `error` && !hasBeenReady) {
200198
// TODO: Once collections hold a reference to their last error object (#671),
201199
// we should rethrow that actual error instead of creating a generic message
202200
throw new Error(`Collection "${result.collection.id}" failed to load`)
203201
}
204202

205203
if (collectionStatus === `loading` || collectionStatus === `idle`) {
206204
// Create or reuse promise for current collection
207-
if (!promiseRef.current) {
208-
promiseRef.current = result.collection.preload()
205+
const promise = result.collection.preload()
206+
let safePromise = safePromiseCache.get(promise)
207+
if (!safePromise) {
208+
safePromise = promise.catch(() => {})
209+
safePromiseCache.set(promise, safePromise)
209210
}
210211
// THROW PROMISE - React Suspense catches this (React 18+ required)
211212
// Note: We don't check React version here. In React <18, this will be caught
212213
// by an Error Boundary, which provides a reasonable failure mode.
213-
throw promiseRef.current
214+
throw safePromise
214215
}
215216

216217
// Return data without status/loading flags (handled by Suspense/ErrorBoundary)
217218
// If error after success, return last known good state (stale data)
218-
return {
219-
state: result.state,
220-
data: result.data,
221-
collection: result.collection,
222-
}
219+
return useMemo(
220+
() => ({
221+
state: result.state,
222+
data: result.data,
223+
collection: result.collection,
224+
}),
225+
[result.collection, result.data, result.state],
226+
)
223227
}

0 commit comments

Comments
 (0)