From 5c93a2ec3dd744e73ba5dbeab74a75a7e44c6134 Mon Sep 17 00:00:00 2001 From: Niek Bosch Date: Sat, 9 Jan 2021 21:31:50 +0100 Subject: [PATCH] fix: prevent missing renders (#1608) --- src/react/tests/suspense.test.tsx | 2 +- src/react/tests/useInfiniteQuery.test.tsx | 24 +++++- src/react/tests/useQuery.test.tsx | 89 ++++++++++++++++++----- src/react/useBaseQuery.ts | 16 +--- 4 files changed, 95 insertions(+), 36 deletions(-) diff --git a/src/react/tests/suspense.test.tsx b/src/react/tests/suspense.test.tsx index a8534bc883..da6638e946 100644 --- a/src/react/tests/suspense.test.tsx +++ b/src/react/tests/suspense.test.tsx @@ -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' }) diff --git a/src/react/tests/useInfiniteQuery.test.tsx b/src/react/tests/useInfiniteQuery.test.tsx index f9d8333e8d..ddc1e25f22 100644 --- a/src/react/tests/useInfiniteQuery.test.tsx +++ b/src/react/tests/useInfiniteQuery.test.tsx @@ -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, @@ -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, @@ -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, @@ -840,8 +848,16 @@ 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, @@ -849,7 +865,7 @@ describe('useInfiniteQuery', () => { isSuccess: true, }) // Refetch done - expect(states[4]).toMatchObject({ + expect(states[5]).toMatchObject({ hasNextPage: true, data: { pages: [7, 8] }, isFetching: false, diff --git a/src/react/tests/useQuery.test.tsx b/src/react/tests/useQuery.test.tsx index cd879164cd..3848e5a874 100644 --- a/src/react/tests/useQuery.test.tsx +++ b/src/react/tests/useQuery.test.tsx @@ -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 () => { @@ -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 () => { @@ -1080,7 +1084,7 @@ describe('useQuery', () => { renderWithClient(queryClient, ) - await waitFor(() => expect(states.length).toBe(4)) + await waitFor(() => expect(states.length).toBe(5)) // Initial expect(states[0]).toMatchObject({ @@ -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, @@ -1141,7 +1152,7 @@ describe('useQuery', () => { renderWithClient(queryClient, ) - await waitFor(() => expect(states.length).toBe(4)) + await waitFor(() => expect(states.length).toBe(5)) // Initial expect(states[0]).toMatchObject({ @@ -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, @@ -1210,7 +1228,7 @@ describe('useQuery', () => { renderWithClient(queryClient, ) - await waitFor(() => expect(states.length).toBe(6)) + await waitFor(() => expect(states.length).toBe(7)) // Disabled query expect(states[0]).toMatchObject({ @@ -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, @@ -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({ @@ -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, @@ -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
{state.data}
+ } + + const rendered = renderWithClient(queryClient, ) + + 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() @@ -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 () => { diff --git a/src/react/useBaseQuery.ts b/src/react/useBaseQuery.ts index efa0e77427..05c7becdde 100644 --- a/src/react/useBaseQuery.ts +++ b/src/react/useBaseQuery.ts @@ -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' @@ -59,23 +58,12 @@ export function useBaseQuery( } 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