[v4] Fix infinite re-renders with synchronous queries in suspense mode #9584
Conversation
…nt infinite loops
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit 84f568a
☁️ Nx Cloud last updated this comment at |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 84f568a:
|
…nt infinite loops - fix test
…nt infinite loops - fix test
|
|
|
not sure why CI hangs here but I really don’t want to spend time backporting something to a 1+ year old code-base that has already been fixed in v5. @manudeli if you want to investigate why this hangs, please do. Otherwise, let’s close this PR. |
Totally understand not wanting to spend time backporting to the old v4 branch — thanks a lot for all the work you’ve already done there. For now, @lachlancollins kindly offered in Discord to take a look at the CI issue, so I’ll wait for that. As for v4 backports, I think we’ll only need to cover a few essentials:
Beyond these, I don’t expect further backports will be necessary. I’ll make sure to take care of these on my side, so you don’t need to worry about v4 anymore. |
|
Hello @manudeli , I had removed the most critical test for this issue because I believed it was causing an Nx CI problem. it('should not cause infinite re-renders with synchronous query function and cacheTime: 0', async () => {
const key = queryKey()
let renderCount = 0
let queryFnCallCount = 0
const maxChecks = 20
function Page() {
renderCount++
if (renderCount > maxChecks) {
throw new Error(`Infinite loop detected! Renders: ${renderCount}`)
}
const result = useQuery(
key,
() => {
queryFnCallCount++
return 42
},
{
cacheTime: 0,
suspense: true,
},
)
return <div>data: {result.data}</div>
}
const rendered = renderWithClient(
queryClient,
<React.Suspense fallback="loading">
<Page />
</React.Suspense>,
)
await waitFor(() => rendered.getByText('data: 42'))
expect(renderCount).toBeLessThan(5)
expect(queryFnCallCount).toBe(1)
expect(rendered.queryByText('data: 42')).not.toBeNull()
expect(rendered.queryByText('loading')).toBeNull()
}) |
fixes: #9583
comment: #9583 (comment)
Problem
After comparing the suspension-related codes, the two biggest differences between v5 and v4 are as follows.
In suspense.ts,
ensureStaleTime (renamed to ensureSuspenseTimers in v5) in v4 only includes a guard for staleTime, while in v5 it includes guards for both staleTime and gcTime.
query/packages/react-query/src/suspense.ts
Line 7 in 7e5e9f5
query/packages/react-query/src/suspense.ts
Line 21 in a1b1279
Another difference is that in v4, shouldSuspend determines suspension using willFetch(result, isRestoring), whereas in v5 it relies on result.isPending. However, since v4 does not handle isPending (more specifically, it does not handle thenables),
query/packages/react-query/src/suspense.ts
Line 30 in 7e5e9f5
query/packages/react-query/src/suspense.ts
Line 53 in a1b1279
I'm not sure if this is intentional, but in v5, when in suspense mode, ensureSuspenseTimers seems to set the minimum gcTime to 1000 (even if you explicitly set it to less than 1000).
Therefore, perhaps, the simplest solution without major structural changes is to add a guard for cacheTime to ensureStaleTime.
Solution
This PR adds a guard for cacheTime in the ensureStaleTime function to match v5's behavior. When suspense mode is enabled, the function now ensures a minimum cacheTime of 1000ms, preventing the cache from being deleted before React's rendering cycle completes.
The fix specifically addresses the scenario where synchronous query functions with suspense: true and cacheTime < 2ms cause infinite re-render loops. By enforcing a minimum cacheTime, we ensure that
Testing
Added comprehensive test cases to verify