Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/eleven-gifts-shine.md
Original file line number Diff line number Diff line change
@@ -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.
112 changes: 59 additions & 53 deletions packages/react-db/src/useLiveQuery.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useRef, useSyncExternalStore } from 'react'
import { useCallback, useState, useSyncExternalStore } from 'react'
import {
BaseQueryBuilder,
createLiveQueryCollection,
Expand All @@ -13,7 +13,6 @@ import type {
InferResultType,
InitialQueryBuilder,
LiveQueryCollectionConfig,
LiveQueryObserver,
NonSingleResult,
QueryBuilder,
SingleResult,
Expand Down Expand Up @@ -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<Collection<object, string | number, {}> | null>(
null,
)
const depsRef = useRef<Array<unknown> | null>(null)
const configRef = useRef<unknown>(null)

// The shared observer owns subscription, the ready-race, and the snapshot.
const observerRef = useRef<LiveQueryObserver<object, string | number> | 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
Expand All @@ -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`) {
Expand All @@ -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,
Expand All @@ -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
}
48 changes: 26 additions & 22 deletions packages/react-db/src/useLiveSuspenseQuery.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useRef } from 'react'
import { useMemo, useState } from 'react'
import { useLiveQuery } from './useLiveQuery'
import type {
Collection,
Expand All @@ -12,6 +12,8 @@ import type {
SingleResult,
} from '@tanstack/db'

const safePromiseCache = new WeakMap<Promise<void>, Promise<void>>()

/**
* Create a live query with React Suspense support
* @param queryFn - Query function that defines what data to fetch
Expand Down Expand Up @@ -156,18 +158,16 @@ export function useLiveSuspenseQuery(
configOrQueryOrCollection: any,
deps: Array<unknown> = [],
) {
const promiseRef = useRef<Promise<void> | null>(null)
const collectionRef = useRef<Collection<any, any, any> | 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
Expand All @@ -188,36 +188,40 @@ 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)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// 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`)
}

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],
)
}