From 66ed096461c23ca54c8dc65825fde7db5c79b91f Mon Sep 17 00:00:00 2001 From: Xandor Schiefer Date: Wed, 12 Aug 2026 21:29:28 +0200 Subject: [PATCH] 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. --- .changeset/eleven-gifts-shine.md | 12 ++ packages/react-db/src/useLiveQuery.ts | 112 +++++++++--------- packages/react-db/src/useLiveSuspenseQuery.ts | 48 ++++---- 3 files changed, 97 insertions(+), 75 deletions(-) create mode 100644 .changeset/eleven-gifts-shine.md diff --git a/.changeset/eleven-gifts-shine.md b/.changeset/eleven-gifts-shine.md new file mode 100644 index 0000000000..c879723667 --- /dev/null +++ b/.changeset/eleven-gifts-shine.md @@ -0,0 +1,12 @@ +--- +'@tanstack/react-db': patch +--- + +fix(react-db): Avoid using refs to track changes in React hooks. + +Using refs to track previous versions of variables, and reading those refs +during render, 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. diff --git a/packages/react-db/src/useLiveQuery.ts b/packages/react-db/src/useLiveQuery.ts index 2ada390770..02427d1d33 100644 --- a/packages/react-db/src/useLiveQuery.ts +++ b/packages/react-db/src/useLiveQuery.ts @@ -1,4 +1,4 @@ -import { useRef, useSyncExternalStore } from 'react' +import { useCallback, useState, useSyncExternalStore } from 'react' import { BaseQueryBuilder, createLiveQueryCollection, @@ -13,7 +13,6 @@ import type { InferResultType, InitialQueryBuilder, LiveQueryCollectionConfig, - LiveQueryObserver, NonSingleResult, QueryBuilder, SingleResult, @@ -321,28 +320,11 @@ export function useLiveQuery( // Check if it's already a collection const inputIsCollection = isCollection(configOrQueryOrCollection) - // Use refs to cache collection and track dependencies - const collectionRef = useRef | null>( - null, - ) - const depsRef = useRef | null>(null) - const configRef = useRef(null) - - // The shared observer owns subscription, the ready-race, and the snapshot. - const observerRef = useRef | null>( - null, - ) - - // Check if we need to create/recreate the collection - const needsNewCollection = - !collectionRef.current || - (inputIsCollection && configRef.current !== configOrQueryOrCollection) || - (!inputIsCollection && - (depsRef.current === null || - depsRef.current.length !== deps.length || - depsRef.current.some((dep, i) => dep !== deps[i]))) - - if (needsNewCollection) { + const createCollection = (): Collection< + object, + string | number, + {} + > | null => { if (inputIsCollection) { // Warn when passing a collection directly with on-demand sync mode // In on-demand mode, data is only loaded when queries with predicates request it @@ -361,8 +343,7 @@ export function useLiveQuery( } // It's already a collection, ensure sync is started for React hooks configOrQueryOrCollection.startSyncImmediate() - collectionRef.current = configOrQueryOrCollection - configRef.current = configOrQueryOrCollection + return configOrQueryOrCollection } else { // Handle different callback return types if (typeof configOrQueryOrCollection === `function`) { @@ -372,22 +353,22 @@ export function useLiveQuery( if (result === undefined || result === null) { // Callback returned undefined/null - disabled query - collectionRef.current = null + return null } else if (isCollection(result)) { // Callback returned a Collection instance - use it directly result.startSyncImmediate() - collectionRef.current = result + return result } else if (result instanceof BaseQueryBuilder) { // Callback returned QueryBuilder - create live query collection using the original callback // (not the result, since the result might be from a different query builder instance) - collectionRef.current = createLiveQueryCollection({ + return createLiveQueryCollection({ query: configOrQueryOrCollection, startSync: true, gcTime: DEFAULT_GC_TIME_MS, }) } else if (result && typeof result === `object`) { // Assume it's a LiveQueryCollectionConfig - collectionRef.current = createLiveQueryCollection({ + return createLiveQueryCollection({ startSync: true, gcTime: DEFAULT_GC_TIME_MS, ...result, @@ -398,51 +379,76 @@ export function useLiveQuery( `useLiveQuery callback must return a QueryBuilder, LiveQueryCollectionConfig, Collection, undefined, or null. Got: ${typeof result}`, ) } - depsRef.current = [...deps] } else { // Original logic for config objects - collectionRef.current = createLiveQueryCollection({ + return createLiveQueryCollection({ startSync: true, gcTime: DEFAULT_GC_TIME_MS, ...configOrQueryOrCollection, }) - depsRef.current = [...deps] } } } - // Recreate the observer when the underlying collection changes. The observer - // is not disposed explicitly here or on unmount: `useSyncExternalStore` - // unsubscribes it when the subscribe changes or the component unmounts, which - // detaches the collection subscription; the observer is then GC'd. (An unmount - // effect that disposed it would misfire under StrictMode/offscreen effect - // replay, leaving a disposed observer in the ref.) + // Use state to cache collection and track dependencies + const [collection, setCollection] = useState(createCollection) + const [prevCollection, setPrevCollection] = useState(collection) + const [prevDeps, setPrevDeps] = useState( + !inputIsCollection ? [...deps] : null, + ) + const [prevConfig, setPrevConfig] = useState( + inputIsCollection ? configOrQueryOrCollection : null, + ) + + // Check if we need to create/recreate the collection + const needsNewCollection = + (inputIsCollection && prevConfig !== configOrQueryOrCollection) || + (!inputIsCollection && + (prevDeps === null || + prevDeps.length !== deps.length || + prevDeps.some((dep, i) => dep !== deps[i]))) + if (needsNewCollection) { + setCollection(createCollection) + if (isCollection(configOrQueryOrCollection)) { + setPrevConfig(configOrQueryOrCollection) + } else { + setPrevDeps([...deps]) + } + } + + // The shared observer owns subscription, the ready-race, and the snapshot. + const [observer, setObserver] = useState(() => // Defer the initial notify: useSyncExternalStore must not be notified // synchronously during subscribe. // Wholesale mode: React re-reads getSnapshot() on notify, keeps the // hook's pre-observer loading policy, and — because wholesale delivers // nothing synchronously during subscribe — never notifies // useSyncExternalStore inside its own subscribe call. - observerRef.current = createLiveQueryObserver(collectionRef.current, { - mode: `wholesale`, - }) + createLiveQueryObserver(collection, { mode: `wholesale` }), + ) + + // Recreate the observer when the underlying collection changes. The observer + // is not disposed explicitly here or on unmount: `useSyncExternalStore` + // unsubscribes it when the subscribe changes or the component unmounts, which + // detaches the collection subscription; the observer is then GC'd. (An unmount + // effect that disposed it would misfire under StrictMode/offscreen effect + // replay, leaving a disposed observer in the ref.) + if (prevCollection !== collection) { + setPrevCollection(collection) + setObserver(createLiveQueryObserver(collection, { mode: `wholesale` })) } - const observer = observerRef.current! // Stable subscribe bound to the current observer; the observer owns the // subscription, ready-race, and disposal. - const subscribeRef = useRef< - ((onStoreChange: () => void) => () => void) | null - >(null) - if (!subscribeRef.current || needsNewCollection) { - subscribeRef.current = (onStoreChange) => - observer.subscribe(() => onStoreChange()) - } + const subscribe = useCallback( + (onStoreChange: () => void) => observer.subscribe(() => onStoreChange()), + [observer], + ) + + const getSnapshot = useCallback(() => observer.getSnapshot(), [observer]) // The observer returns a stable snapshot per revision, which is the return // shape this hook exposes. Keep the return loose to satisfy the overloads. - return useSyncExternalStore(subscribeRef.current, () => - observer.getSnapshot(), - ) as any + return useSyncExternalStore(subscribe, getSnapshot) as any } diff --git a/packages/react-db/src/useLiveSuspenseQuery.ts b/packages/react-db/src/useLiveSuspenseQuery.ts index 162bf1f3fe..73892064e3 100644 --- a/packages/react-db/src/useLiveSuspenseQuery.ts +++ b/packages/react-db/src/useLiveSuspenseQuery.ts @@ -1,4 +1,4 @@ -import { useRef } from 'react' +import { useMemo, useState } from 'react' import { useLiveQuery } from './useLiveQuery' import type { Collection, @@ -12,6 +12,8 @@ import type { SingleResult, } from '@tanstack/db' +const safePromiseCache = new WeakMap, Promise>() + /** * Create a live query with React Suspense support * @param queryFn - Query function that defines what data to fetch @@ -156,18 +158,16 @@ export function useLiveSuspenseQuery( configOrQueryOrCollection: any, deps: Array = [], ) { - const promiseRef = useRef | null>(null) - const collectionRef = useRef | null>(null) - const hasBeenReadyRef = useRef(false) - // Use useLiveQuery to handle collection management and reactivity const result = useLiveQuery(configOrQueryOrCollection, deps) + const [prevCollection, setPrevCollection] = useState(result.collection) + const [hasBeenReady, setHasBeenReady] = useState(false) + // Reset promise and ready state when collection changes (deps changed) - if (collectionRef.current !== result.collection) { - promiseRef.current = null - collectionRef.current = result.collection - hasBeenReadyRef.current = false + if (prevCollection !== result.collection) { + setPrevCollection(result.collection) + setHasBeenReady(false) } // SUSPENSE LOGIC: Throw promise or error based on collection status @@ -188,15 +188,13 @@ export function useLiveSuspenseQuery( const collectionStatus = result.collection.status // Track when we reach ready state - if (collectionStatus === `ready`) { - hasBeenReadyRef.current = true - promiseRef.current = null + if (collectionStatus === `ready` && !hasBeenReady) { + setHasBeenReady(true) } // Only throw errors during initial load (before first ready) // After success, errors surface as stale data (matches TanStack Query behavior) - if (collectionStatus === `error` && !hasBeenReadyRef.current) { - promiseRef.current = null + if (collectionStatus === `error` && !hasBeenReady) { // TODO: Once collections hold a reference to their last error object (#671), // we should rethrow that actual error instead of creating a generic message throw new Error(`Collection "${result.collection.id}" failed to load`) @@ -204,20 +202,26 @@ export function useLiveSuspenseQuery( if (collectionStatus === `loading` || collectionStatus === `idle`) { // Create or reuse promise for current collection - if (!promiseRef.current) { - promiseRef.current = result.collection.preload() + const promise = result.collection.preload() + let safePromise = safePromiseCache.get(promise) + if (!safePromise) { + safePromise = promise.catch(() => {}) + safePromiseCache.set(promise, safePromise) } // THROW PROMISE - React Suspense catches this (React 18+ required) // Note: We don't check React version here. In React <18, this will be caught // by an Error Boundary, which provides a reasonable failure mode. - throw promiseRef.current + throw safePromise } // Return data without status/loading flags (handled by Suspense/ErrorBoundary) // If error after success, return last known good state (stale data) - return { - state: result.state, - data: result.data, - collection: result.collection, - } + return useMemo( + () => ({ + state: result.state, + data: result.data, + collection: result.collection, + }), + [result.collection, result.data, result.state], + ) }