Skip to content

Commit

Permalink
fix(useQuery): continue retries if query unmounts and remounts (#3032)
Browse files Browse the repository at this point in the history
* fix(useQuery): continue retries if observer unmount and remount

if we are waiting for a retry to happen, unsubscribing he last observer will cancel retries and just return the error; however, if a new observer subscribes in the meantime, we should continue with the ongoing retries.

this is especially important with "strict effects" in react18, where effects are run twice and thus observers are always unsubscribed and re-subscribed immediately.

* fix(useQuery): continue retries if observer unmount and remount

add another test to include query cancellation
  • Loading branch information
TkDodo committed Nov 28, 2021
1 parent c51498a commit dc2df10
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/core/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,8 @@ export class Query<
// Silently cancel current fetch if the user wants to cancel refetches
this.cancel({ silent: true })
} else if (this.promise) {
// make sure that retries that were potentially cancelled due to unmounts can continue
this.retryer?.continueRetry()
// Return current promise if we are already fetching
return this.promise
}
Expand Down
4 changes: 4 additions & 0 deletions src/core/retryer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export function isCancelledError(value: any): value is CancelledError {
export class Retryer<TData = unknown, TError = unknown> {
cancel: (options?: CancelOptions) => void
cancelRetry: () => void
continueRetry: () => void
continue: () => void
failureCount: number
isPaused: boolean
Expand All @@ -82,6 +83,9 @@ export class Retryer<TData = unknown, TError = unknown> {
this.cancelRetry = () => {
cancelRetry = true
}
this.continueRetry = () => {
cancelRetry = false
}
this.continue = () => continueFn?.()
this.failureCount = 0
this.isPaused = false
Expand Down
105 changes: 105 additions & 0 deletions src/react/tests/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2671,6 +2671,111 @@ describe('useQuery', () => {
consoleMock.mockRestore()
})

it('should continue retries when observers unmount and remount while waiting for a retry (#3031)', async () => {
const key = queryKey()
const consoleMock = mockConsoleError()
let count = 0

function Page() {
const result = useQuery(
key,
async () => {
count++
await sleep(10)
return Promise.reject('some error')
},
{
retry: 2,
retryDelay: 100,
}
)

return (
<div>
<div>error: {result.error ?? 'null'}</div>
<div>failureCount: {result.failureCount}</div>
</div>
)
}

function App() {
const [show, toggle] = React.useReducer(x => !x, true)

return (
<div>
<button onClick={toggle}>{show ? 'hide' : 'show'}</button>
{show && <Page />}
</div>
)
}

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

await waitFor(() => rendered.getByText('failureCount: 1'))
rendered.getByRole('button', { name: /hide/i }).click()
rendered.getByRole('button', { name: /show/i }).click()
await waitFor(() => rendered.getByText('error: some error'))

expect(count).toBe(3)

consoleMock.mockRestore()
})

it('should restart when observers unmount and remount while waiting for a retry when query was cancelled in between (#3031)', async () => {
const key = queryKey()
const consoleMock = mockConsoleError()
let count = 0

function Page() {
const result = useQuery(
key,
async () => {
count++
await sleep(10)
return Promise.reject('some error')
},
{
retry: 2,
retryDelay: 100,
}
)

return (
<div>
<div>error: {result.error ?? 'null'}</div>
<div>failureCount: {result.failureCount}</div>
</div>
)
}

function App() {
const [show, toggle] = React.useReducer(x => !x, true)

return (
<div>
<button onClick={toggle}>{show ? 'hide' : 'show'}</button>
<button onClick={() => queryClient.cancelQueries({ queryKey: key })}>
cancel
</button>
{show && <Page />}
</div>
)
}

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

await waitFor(() => rendered.getByText('failureCount: 1'))
rendered.getByRole('button', { name: /hide/i }).click()
rendered.getByRole('button', { name: /cancel/i }).click()
rendered.getByRole('button', { name: /show/i }).click()
await waitFor(() => rendered.getByText('error: some error'))

// initial fetch (1), which will be cancelled, followed by new mount(2) + 2 retries = 4
expect(count).toBe(4)

consoleMock.mockRestore()
})

it('should always fetch if refetchOnMount is set to always', async () => {
const key = queryKey()
const states: UseQueryResult<string>[] = []
Expand Down

1 comment on commit dc2df10

@vercel
Copy link

@vercel vercel bot commented on dc2df10 Nov 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.