Skip to content
Merged
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
44 changes: 44 additions & 0 deletions .changeset/client-react-dependency-identity-loops.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
---
"@objectstack/client-react": patch
---

fix(client-react): stop five hooks from looping on dependency identity (#4693, #4694)

Five hooks keyed a `useCallback`/`useEffect` on values the caller supplies
inline — `where` / `fields` / `orderBy` objects, `onSuccess` / `onError`
handlers, and the `fetcher` `useMetadata` takes as a required positional
argument. Inline means a fresh identity on every render, so the effect re-ran on
every render; because the fetch hooks call `setState`, that render caused
another. The result was an unbounded request loop under the hooks' own
documented usage.

Requests issued in 250ms by a single mounted component, measured before and
after:

| hook | before | after |
|-----------------------------------|-------:|------:|
| `useQuery` (inline `where`) | 4691 | 1 |
| `useInfiniteQuery` (inline `where`) | 6611 | 1 |
| `useObject` (no options at all) | 4306 | 1 |
| `useView` (inline `onSuccess`) | 8197 | 1 |
| `useMetadata` (inline `fetcher`) | 7654 | 1 |

`useObject` and `useMetadata` needed no particular usage to loop: the former
depended on its own `data` and `etag` state while writing both, and the latter
takes its fetcher positionally, so there is no non-inline way to call it.
`useMutation` was never affected — no effect drives it.

The same root cause churned the realtime subscriptions (#4694):
`useAutoRefresh` with an unmemoized `refetch` — which is what `useQuery`
returned on every render — resubscribed on both streams every render, losing any
event delivered in the unsubscribe/resubscribe gap.

Two internal primitives fix both halves: `stableKey` derives a dependency from a
structural value (sorted keys, array order preserved) so a rebuilt-but-equal
object is a no-op, and `useEventCallback` gives a handler a fixed identity while
always invoking its latest version. Neither is exported.

A changed *value* still refetches, and every stabilized handler is asserted to
run its newest version rather than the one captured when the effect first ran —
the ref indirection would otherwise trade a loop for a stale closure. 13 tests
cover this, each verified by reverting the fix it guards.
45 changes: 39 additions & 6 deletions packages/client-react/src/data-hooks.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { useState, useEffect, useCallback, useRef } from 'react';
import { QueryAST, FilterCondition } from '@objectstack/spec/data';
import { PaginatedResult } from '@objectstack/client';
import { useClient } from './context';
import { stableKey, useEventCallback } from './internal-deps';

/**
* Query options for useQuery hook.
Expand DownExpand Up@@ -113,6 +114,26 @@ export function useQuery<T = any>(
const resolvedLimit = limit;
const resolvedOffset = offset;

// The query shape as a VALUE (#4693). `where` / `fields` / `orderBy` are
// objects and arrays, and the documented usage builds them inline, so keying
// the fetch on their identities re-ran it every render — and since it calls
// `setData`, every render caused another render. Measured before this fix:
// `useQuery('todo_task', { where: { status: 'open' } })` issued 4691 `find`
// calls in 250ms; the same call with a hoisted options object issued 1.
const queryKey = stableKey({
query,
where: resolvedWhere,
fields: resolvedFields,
orderBy: resolvedSort,
limit: resolvedLimit,
offset: resolvedOffset,
});

// Handlers say what to do with a result; they are not part of what is being
// fetched, so they must not drive refetching.
const handleSuccess = useEventCallback(onSuccess);
const handleError = useEventCallback(onError);

const fetchData = useCallback(async (isRefetch = false) => {
if (!enabled) return;

Expand DownExpand Up@@ -141,16 +162,16 @@ export function useQuery<T = any>(
}

setData(result);
onSuccess?.(result);
handleSuccess(result);
} catch (err) {
const error = err instanceof Error ? err : new Error('Query failed');
setError(error);
onError?.(error);
handleError(error);
} finally {
setIsLoading(false);
setIsRefetching(false);
}
}, [client, object, query, resolvedFields, resolvedWhere, resolvedSort, resolvedLimit, resolvedOffset, enabled, onSuccess, onError]);
}, [client, object, queryKey, enabled, handleSuccess, handleError]);

// Initial fetch and dependency-based refetch
useEffect(() => {
Expand DownExpand Up@@ -520,6 +541,18 @@ export function useInfiniteQuery<T = any>(
const resolvedWhere = where;
const resolvedSort = orderBy;

// Same value-keyed dependency as useQuery (#4693) — measured at 6611 `find`
// calls in 250ms before this fix, with inline options.
const queryKey = stableKey({
query,
where: resolvedWhere,
fields: resolvedFields,
orderBy: resolvedSort,
pageSize,
});
const handleSuccess = useEventCallback(onSuccess);
const handleError = useEventCallback(onError);

const [pages, setPages] = useState<PaginatedResult<T>[]>([]);
const [isLoading, setIsLoading] = useState(true);
const [isFetchingNextPage, setIsFetchingNextPage] = useState(false);
Expand DownExpand Up@@ -564,16 +597,16 @@ export function useInfiniteQuery<T = any>(
const hasMore = fetchedCount === pageSize;
setHasNextPage(hasMore);

onSuccess?.(result);
handleSuccess(result);
} catch (err) {
const error = err instanceof Error ? err : new Error('Query failed');
setError(error);
onError?.(error);
handleError(error);
} finally {
setIsLoading(false);
setIsFetchingNextPage(false);
}
}, [client, object, query, resolvedFields, resolvedWhere, resolvedSort, pageSize, onSuccess, onError]);
}, [client, object, queryKey, handleSuccess, handleError]);

// Initial fetch
useEffect(() => {
Expand Down
Loading
Loading