Skip to content

Commit

Permalink
fix(core): correctly gc query when suspense is used when query unmoun…
Browse files Browse the repository at this point in the history
…ts while it's still fetching (#8168)

* fix(core): correctly gc query when suspense is used when query unmounts while it's still fetching

the `isFetchingOptimistic` workaround was added to make sure some "thing" I don't remember anymore works in suspense; we added tests for that;

however, in the meantime, we added a minimum gcTime for suspense queries, which also solves that problem (tests still work); this makes the `isFetchingOptimistic` workaround unnecessary, and this workaround is actually what causes the unmount issue

* chore: try to stabilize a flaky test
  • Loading branch information
TkDodo authored Oct 11, 2024
1 parent 931d98d commit 5edd617
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 26 deletions.
15 changes: 4 additions & 11 deletions packages/query-core/src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export class Query<
queryHash: string
options!: QueryOptions<TQueryFnData, TError, TData, TQueryKey>
state: QueryState<TData, TError>
isFetchingOptimistic?: boolean

#initialState: QueryState<TData, TError>
#revertState?: QueryState<TData, TError>
Expand Down Expand Up @@ -482,11 +481,8 @@ export class Query<
)
}

if (!this.isFetchingOptimistic) {
// Schedule query gc after fetching
this.scheduleGc()
}
this.isFetchingOptimistic = false
// Schedule query gc after fetching
this.scheduleGc()
}

// Try to fetch the data
Expand Down Expand Up @@ -522,11 +518,8 @@ export class Query<
this as Query<any, any, any, any>,
)

if (!this.isFetchingOptimistic) {
// Schedule query gc after fetching
this.scheduleGc()
}
this.isFetchingOptimistic = false
// Schedule query gc after fetching
this.scheduleGc()
},
onError,
onFail: (failureCount, error) => {
Expand Down
1 change: 0 additions & 1 deletion packages/query-core/src/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ export class QueryObserver<
const query = this.#client
.getQueryCache()
.build(this.#client, defaultedOptions)
query.isFetchingOptimistic = true

return query.fetch().then(() => this.createResult(query, defaultedOptions))
}
Expand Down
76 changes: 74 additions & 2 deletions packages/react-query/src/__tests__/suspense.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it, vi } from 'vitest'
import { fireEvent, waitFor } from '@testing-library/react'
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
import { act, fireEvent, waitFor } from '@testing-library/react'
import * as React from 'react'
import { ErrorBoundary } from 'react-error-boundary'
import {
Expand Down Expand Up @@ -1235,4 +1235,76 @@ describe('useSuspenseQueries', () => {
expect(count).toBe(1)
consoleMock.mockRestore()
})

describe('gc (with fake timers)', () => {
beforeAll(() => {
vi.useFakeTimers()
})

afterAll(() => {
vi.useRealTimers()
})

it('should gc when unmounted while fetching with low gcTime (#8159)', async () => {
const key = queryKey()

function Page() {
return (
<React.Suspense fallback="loading">
<Component />
</React.Suspense>
)
}

function Component() {
const { data } = useSuspenseQuery({
queryKey: key,
queryFn: async () => {
await sleep(3000)
return 'data'
},
gcTime: 1000,
})

return <div>{data}</div>
}

function Page2() {
return <div>page2</div>
}

function App() {
const [show, setShow] = React.useState(true)
return (
<div>
{show ? <Page /> : <Page2 />}
<button onClick={() => setShow(false)}>hide</button>
</div>
)
}

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

await act(() => vi.advanceTimersByTimeAsync(200))

rendered.getByText('loading')

// unmount while still fetching
fireEvent.click(rendered.getByText('hide'))

await act(() => vi.advanceTimersByTimeAsync(800))

rendered.getByText('page2')

// wait for query to be resolved
await act(() => vi.advanceTimersByTimeAsync(2000))

expect(queryClient.getQueryData(key)).toBe('data')

// wait for gc
await act(() => vi.advanceTimersByTimeAsync(1000))

expect(queryClient.getQueryData(key)).toBe(undefined)
})
})
})
24 changes: 12 additions & 12 deletions packages/react-query/src/__tests__/useMutationState.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
createQueryClient,
doNotExecute,
renderWithClient,
setActTimeout,
sleep,
} from './utils'
import type { MutationState, MutationStatus } from '@tanstack/query-core'
Expand All @@ -27,26 +26,24 @@ describe('useIsMutating', () => {
const { mutate: mutate1 } = useMutation({
mutationKey: ['mutation1'],
mutationFn: async () => {
await sleep(150)
await sleep(50)
return 'data'
},
})
const { mutate: mutate2 } = useMutation({
mutationKey: ['mutation2'],
mutationFn: async () => {
await sleep(50)
await sleep(10)
return 'data'
},
})

React.useEffect(() => {
mutate1()
setActTimeout(() => {
mutate2()
}, 50)
}, [mutate1, mutate2])

return null
return (
<div>
<button onClick={() => mutate1()}>mutate1</button>
<button onClick={() => mutate2()}>mutate2</button>
</div>
)
}

function Page() {
Expand All @@ -58,7 +55,10 @@ describe('useIsMutating', () => {
)
}

renderWithClient(queryClient, <Page />)
const rendered = renderWithClient(queryClient, <Page />)
fireEvent.click(rendered.getByRole('button', { name: /mutate1/i }))
await sleep(10)
fireEvent.click(rendered.getByRole('button', { name: /mutate2/i }))
await waitFor(() => expect(isMutatingArray).toEqual([0, 1, 2, 1, 0]))
})

Expand Down

0 comments on commit 5edd617

Please sign in to comment.