Skip to content

Conversation

joseph0926
Copy link
Contributor

@joseph0926 joseph0926 commented Aug 21, 2025

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.

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

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

  • The query result remains cached long enough for React to complete its suspense boundary resolution
  • Synchronous queries behave consistently with asynchronous queries in suspense mode
  • The behavior aligns with v5's implementation, making future migrations smoother

Testing

Added comprehensive test cases to verify

  • Boundary value handling (cacheTime values from 0 to 2000ms)
  • User-specified cacheTime values are preserved when >= 1000ms
  • Both synchronous and asynchronous queries work correctly with the adjustment
  • The relationship between staleTime and cacheTime is maintained
  • The infinite re-render loop with synchronous queries and cacheTime: 0 is resolved

Copy link
Contributor

coderabbitai bot commented Aug 21, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

nx-cloud bot commented Aug 21, 2025

View your CI Pipeline Execution ↗ for commit 6519558

Command Status Duration Result
nx affected --targets=test:lib,test:types,test:... ✅ Succeeded 48s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-06 02:13:09 UTC

Copy link

codesandbox-ci bot commented Aug 21, 2025

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 6519558:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

@joseph0926
Copy link
Contributor Author

joseph0926 commented Aug 21, 2025

The v4 quarterly CI is currently delayed due to an issue (taking over an hour and failing due to an Nx Cloud error).
This issue appears to be similar in other v4 PRs (https://github.com/TanStack/query/actions/runs/16601605388/job/46962541804?pr=9334) as well as this PR. Local tests pass successfully.
Therefore, I will switch to Draft for now.

@joseph0926 joseph0926 marked this pull request as draft August 21, 2025 03:07
@joseph0926 joseph0926 marked this pull request as ready for review August 24, 2025 09:53
@TkDodo
Copy link
Collaborator

TkDodo commented Sep 1, 2025

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.

@manudeli manudeli self-requested a review September 1, 2025 11:37
@manudeli
Copy link
Collaborator

manudeli commented Sep 1, 2025

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:

  1. infiniteQueryOptions, suspense hook
  2. Fix for infinite re-render in Suspense mode
  3. React 19 compatibility

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.

@manudeli manudeli self-assigned this Sep 1, 2025
@joseph0926
Copy link
Contributor Author

joseph0926 commented Sep 1, 2025

84f568a

Hello @manudeli ,

I had removed the most critical test for this issue because I believed it was causing an Nx CI problem.
However, since it has been confirmed that this test does not cause CI issues, I will add it back.

  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()
  })

@manudeli
Copy link
Collaborator

manudeli commented Sep 9, 2025

@joseph0926 @suhdonghwi I tested this locally as well, and it works. Thanks

@manudeli manudeli merged commit f464aa3 into TanStack:v4 Sep 9, 2025
5 checks passed
@joseph0926 joseph0926 deleted the fix/suspense-sync-query-cachetime-v4 branch September 9, 2025 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(v4) Infinite re-render loop with suspense: true and cacheTime: 0 for synchronous fetchers
3 participants