Skip to content

Commit

Permalink
fix(react-query): ensure we have a gcTime of at least 1 second when u…
Browse files Browse the repository at this point in the history
…sing suspense (#7860)

Because React continues to render components that have already thrown to error boundaries, a small gcTime would lead to infinite fetches because if the cache entry is already removed, the next render will treat the extra render as a new suspending query.
  • Loading branch information
TkDodo authored Aug 8, 2024
1 parent 1fc6124 commit c744f99
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 23 deletions.
44 changes: 44 additions & 0 deletions packages/react-query/src/__tests__/suspense.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1192,4 +1192,48 @@ describe('useSuspenseQueries', () => {

await waitFor(() => rendered.getByText('data1'))
})

it('should show error boundary even with gcTime:0 (#7853)', async () => {
const consoleMock = vi
.spyOn(console, 'error')
.mockImplementation(() => undefined)
const key = queryKey()
let count = 0

function Page() {
useSuspenseQuery({
queryKey: key,
queryFn: async () => {
count++
console.log('queryFn')
throw new Error('Query failed')
},
gcTime: 0,
retry: false,
})

return null
}

function App() {
return (
<React.Suspense fallback="loading">
<ErrorBoundary
fallbackRender={() => {
console.log('fallback renders')
return <div>There was an error!</div>
}}
>
<Page />
</ErrorBoundary>
</React.Suspense>
)
}

const rendered = renderWithClient(queryClient, <App />)

await waitFor(() => rendered.getByText('There was an error!'))
expect(count).toBe(1)
consoleMock.mockRestore()
})
})
21 changes: 3 additions & 18 deletions packages/react-query/src/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4198,17 +4198,15 @@ describe('useQuery', () => {

it('should not interval fetch with a refetchInterval of 0', async () => {
const key = queryKey()
const states: Array<UseQueryResult<number>> = []
const queryFn = vi.fn(() => 1)

function Page() {
const queryInfo = useQuery({
queryKey: key,
queryFn: () => 1,
queryFn,
refetchInterval: 0,
})

states.push(queryInfo)

return <div>count: {queryInfo.data}</div>
}

Expand All @@ -4218,20 +4216,7 @@ describe('useQuery', () => {

await sleep(10) // extra sleep to make sure we're not re-fetching

expect(states.length).toEqual(2)

expect(states).toMatchObject([
{
status: 'pending',
isFetching: true,
data: undefined,
},
{
status: 'success',
isFetching: false,
data: 1,
},
])
expect(queryFn).toHaveBeenCalledTimes(1)
})

it('should accept an empty string as query key', async () => {
Expand Down
5 changes: 4 additions & 1 deletion packages/react-query/src/suspense.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const defaultThrowOnError = <
query: Query<TQueryFnData, TError, TData, TQueryKey>,
) => query.state.data === undefined

export const ensureStaleTime = (
export const ensureSuspenseTimers = (
defaultedOptions: DefaultedQueryObserverOptions<any, any, any, any, any>,
) => {
if (defaultedOptions.suspense) {
Expand All @@ -27,6 +27,9 @@ export const ensureStaleTime = (
if (typeof defaultedOptions.staleTime !== 'number') {
defaultedOptions.staleTime = 1000
}
if (typeof defaultedOptions.gcTime === 'number') {
defaultedOptions.gcTime = Math.max(defaultedOptions.gcTime, 1000)
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions packages/react-query/src/useBaseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import {
getHasError,
useClearResetErrorBoundary,
} from './errorBoundaryUtils'
import { ensureStaleTime, fetchOptimistic, shouldSuspend } from './suspense'
import {
ensureSuspenseTimers,
fetchOptimistic,
shouldSuspend,
} from './suspense'
import type { UseBaseQueryOptions } from './types'
import type {
QueryClient,
Expand Down Expand Up @@ -58,7 +62,7 @@ export function useBaseQuery<
? 'isRestoring'
: 'optimistic'

ensureStaleTime(defaultedOptions)
ensureSuspenseTimers(defaultedOptions)
ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary)

useClearResetErrorBoundary(errorResetBoundary)
Expand Down
4 changes: 2 additions & 2 deletions packages/react-query/src/useQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
useClearResetErrorBoundary,
} from './errorBoundaryUtils'
import {
ensureStaleTime,
ensureSuspenseTimers,
fetchOptimistic,
shouldSuspend,
willFetch,
Expand Down Expand Up @@ -255,7 +255,7 @@ export function useQueries<
)

defaultedQueries.forEach((query) => {
ensureStaleTime(query)
ensureSuspenseTimers(query)
ensurePreventErrorBoundaryRetry(query, errorResetBoundary)
})

Expand Down

0 comments on commit c744f99

Please sign in to comment.