Skip to content

Commit

Permalink
fix: prevent missing renders (#1608)
Browse files Browse the repository at this point in the history
  • Loading branch information
boschni authored Jan 9, 2021
1 parent ca947a6 commit 5c93a2e
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 36 deletions.
2 changes: 1 addition & 1 deletion src/react/tests/suspense.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe("useQuery's in Suspense mode", () => {

await sleep(20)

expect(renders).toBe(4)
expect(renders).toBe(5)
expect(states.length).toBe(2)
expect(states[0]).toMatchObject({ data: 1, status: 'success' })
expect(states[1]).toMatchObject({ data: 2, status: 'success' })
Expand Down
24 changes: 20 additions & 4 deletions src/react/tests/useInfiniteQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ describe('useInfiniteQuery', () => {

await sleep(300)

expect(states.length).toBe(6)
expect(states.length).toBe(7)
expect(states[0]).toMatchObject({
data: undefined,
isFetching: true,
Expand Down Expand Up @@ -243,7 +243,15 @@ describe('useInfiniteQuery', () => {
isSuccess: true,
isPreviousData: true,
})
// Hook state update
expect(states[5]).toMatchObject({
data: { pages: ['0-desc', '1-desc'] },
isFetching: true,
isFetchingNextPage: false,
isSuccess: true,
isPreviousData: true,
})
expect(states[6]).toMatchObject({
data: { pages: ['0-asc'] },
isFetching: false,
isFetchingNextPage: false,
Expand Down Expand Up @@ -816,7 +824,7 @@ describe('useInfiniteQuery', () => {

await sleep(100)

expect(states.length).toBe(5)
expect(states.length).toBe(6)
expect(states[0]).toMatchObject({
hasNextPage: undefined,
data: undefined,
Expand All @@ -840,16 +848,24 @@ describe('useInfiniteQuery', () => {
isFetchingNextPage: false,
isSuccess: true,
})
// Refetch
// Hook state update
expect(states[3]).toMatchObject({
hasNextPage: true,
data: { pages: [7, 8] },
isFetching: false,
isFetchingNextPage: false,
isSuccess: true,
})
// Refetch
expect(states[4]).toMatchObject({
hasNextPage: true,
data: { pages: [7, 8] },
isFetching: true,
isFetchingNextPage: false,
isSuccess: true,
})
// Refetch done
expect(states[4]).toMatchObject({
expect(states[5]).toMatchObject({
hasNextPage: true,
data: { pages: [7, 8] },
isFetching: false,
Expand Down
89 changes: 72 additions & 17 deletions src/react/tests/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -568,15 +568,17 @@ describe('useQuery', () => {

await sleep(100)

expect(states.length).toBe(4)
expect(states.length).toBe(5)
// First load
expect(states[0]).toMatchObject({ isLoading: true, isSuccess: false })
// First success
expect(states[1]).toMatchObject({ isLoading: false, isSuccess: true })
// Second load
// Remove
expect(states[2]).toMatchObject({ isLoading: true, isSuccess: false })
// Hook state update
expect(states[3]).toMatchObject({ isLoading: true, isSuccess: false })
// Second success
expect(states[3]).toMatchObject({ isLoading: false, isSuccess: true })
expect(states[4]).toMatchObject({ isLoading: false, isSuccess: true })
})

it('should fetch when refetchOnMount is false and nothing has been fetched yet', async () => {
Expand Down Expand Up @@ -766,15 +768,17 @@ describe('useQuery', () => {

await sleep(20)

expect(states.length).toBe(4)
expect(states.length).toBe(5)
// Initial
expect(states[0]).toMatchObject({ data: undefined })
// Fetched
expect(states[1]).toMatchObject({ data: 1 })
// Switch
// Remove
expect(states[2]).toMatchObject({ data: undefined })
// Hook state update
expect(states[3]).toMatchObject({ data: undefined })
// Fetched
expect(states[3]).toMatchObject({ data: 2 })
expect(states[4]).toMatchObject({ data: 2 })
})

it('should be create a new query when refetching a removed query', async () => {
Expand Down Expand Up @@ -1080,7 +1084,7 @@ describe('useQuery', () => {

renderWithClient(queryClient, <Page />)

await waitFor(() => expect(states.length).toBe(4))
await waitFor(() => expect(states.length).toBe(5))

// Initial
expect(states[0]).toMatchObject({
Expand All @@ -1103,8 +1107,15 @@ describe('useQuery', () => {
isSuccess: true,
isPreviousData: true,
})
// New data
// Hook state update
expect(states[3]).toMatchObject({
data: 0,
isFetching: true,
isSuccess: true,
isPreviousData: true,
})
// New data
expect(states[4]).toMatchObject({
data: 1,
isFetching: false,
isSuccess: true,
Expand Down Expand Up @@ -1141,7 +1152,7 @@ describe('useQuery', () => {

renderWithClient(queryClient, <Page />)

await waitFor(() => expect(states.length).toBe(4))
await waitFor(() => expect(states.length).toBe(5))

// Initial
expect(states[0]).toMatchObject({
Expand All @@ -1164,8 +1175,15 @@ describe('useQuery', () => {
isSuccess: true,
isPreviousData: true,
})
// New data
// Hook state update
expect(states[3]).toMatchObject({
data: 0,
isFetching: true,
isSuccess: true,
isPreviousData: true,
})
// New data
expect(states[4]).toMatchObject({
data: 1,
isFetching: false,
isSuccess: true,
Expand Down Expand Up @@ -1210,7 +1228,7 @@ describe('useQuery', () => {

renderWithClient(queryClient, <Page />)

await waitFor(() => expect(states.length).toBe(6))
await waitFor(() => expect(states.length).toBe(7))

// Disabled query
expect(states[0]).toMatchObject({
Expand Down Expand Up @@ -1240,15 +1258,22 @@ describe('useQuery', () => {
isSuccess: true,
isPreviousData: true,
})
// Fetching new query
// Hook state update
expect(states[4]).toMatchObject({
data: 0,
isFetching: false,
isSuccess: true,
isPreviousData: true,
})
// Fetching new query
expect(states[5]).toMatchObject({
data: 0,
isFetching: true,
isSuccess: true,
isPreviousData: true,
})
// Fetched new query
expect(states[5]).toMatchObject({
expect(states[6]).toMatchObject({
data: 1,
isFetching: false,
isSuccess: true,
Expand Down Expand Up @@ -1299,7 +1324,7 @@ describe('useQuery', () => {

await sleep(100)

expect(states.length).toBe(5)
expect(states.length).toBe(6)

// Disabled query
expect(states[0]).toMatchObject({
Expand All @@ -1322,15 +1347,22 @@ describe('useQuery', () => {
isSuccess: true,
isPreviousData: true,
})
// Switched query key
// Hook state update
expect(states[3]).toMatchObject({
data: 10,
isFetching: false,
isSuccess: true,
isPreviousData: true,
})
// Refetch
expect(states[4]).toMatchObject({
data: 10,
isFetching: true,
isSuccess: true,
isPreviousData: true,
})
// Refetch done
expect(states[4]).toMatchObject({
expect(states[5]).toMatchObject({
data: 12,
isFetching: false,
isSuccess: true,
Expand Down Expand Up @@ -1690,6 +1722,27 @@ describe('useQuery', () => {
expect(renderedCount).toBe(2)
})

it('should render latest data even if react has discarded certain renders', async () => {
const key = queryKey()

function Page() {
const [, setNewState] = React.useState('state')
const state = useQuery(key, () => 'data')
React.useEffect(() => {
setActTimeout(() => {
queryClient.setQueryData(key, 'new')
// Update with same state to make react discard the next render
setNewState('state')
}, 10)
}, [])
return <div>{state.data}</div>
}

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

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

// See https://github.com/tannerlinsley/react-query/issues/170
it('should start with status idle if enabled is false', async () => {
const key1 = queryKey()
Expand Down Expand Up @@ -2163,11 +2216,13 @@ describe('useQuery', () => {

await sleep(100)

expect(states.length).toBe(2)
expect(states.length).toBe(3)
// Initial
expect(states[0]).toMatchObject({ data: { count: 0 } })
// Set state
expect(states[1]).toMatchObject({ data: { count: 1 } })
// Hook state update
expect(states[2]).toMatchObject({ data: { count: 1 } })
})

it('should retry specified number of times', async () => {
Expand Down
16 changes: 2 additions & 14 deletions src/react/useBaseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react'

import { notifyManager } from '../core/notifyManager'
import { QueryObserver } from '../core/queryObserver'
import { QueryObserverResult } from '../core/types'
import { useQueryErrorResetBoundary } from './QueryErrorResetBoundary'
import { useQueryClient } from './QueryClientProvider'
import { UseBaseQueryOptions } from './types'
Expand Down Expand Up @@ -59,23 +58,12 @@ export function useBaseQuery<TQueryFnData, TError, TData, TQueryData>(
}

const currentResult = observer.getCurrentResult()

// Remember latest result to prevent redundant renders
const latestResultRef = React.useRef(currentResult)
latestResultRef.current = currentResult

const [, rerender] = React.useState({})
const [, setCurrentResult] = React.useState(currentResult)

// Subscribe to the observer
React.useEffect(() => {
errorResetBoundary.clearReset()
return observer.subscribe(
notifyManager.batchCalls((result: QueryObserverResult) => {
if (result !== latestResultRef.current) {
rerender({})
}
})
)
return observer.subscribe(notifyManager.batchCalls(setCurrentResult))
}, [observer, errorResetBoundary])

// Handle suspense
Expand Down

1 comment on commit 5c93a2e

@vercel
Copy link

@vercel vercel bot commented on 5c93a2e Jan 9, 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.