From c62e0afe38fd43d08a232966d21a8a5fbc3b8e9e Mon Sep 17 00:00:00 2001 From: zorzysty Date: Sun, 6 Nov 2022 13:48:59 +0100 Subject: [PATCH 1/6] test: stabilize the test for notifying query cache when a query becomes stale --- .../src/__tests__/useQuery.test.tsx | 60 +++++++++++++++-- .../src/__tests__/createQuery.test.tsx | 66 ++++++++++++++++--- 2 files changed, 113 insertions(+), 13 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index e0e4cc1015..346a8d0d4b 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -2219,9 +2219,6 @@ describe('useQuery', () => { renderWithClient(queryClient, ) - await sleep(20) - unsubscribe() - // 1. Query added -> loading // 2. Observer result updated -> loading // 3. Observer added @@ -2231,7 +2228,62 @@ describe('useQuery', () => { // 7. Observer options updated // 8. Observer result updated -> stale // 9. Observer options updated - expect(fn).toHaveBeenCalledTimes(9) + + await waitFor(() => { + expect(fn).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ type: 'added' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ type: 'observerResultsUpdated' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ type: 'observerAdded' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 4, + expect.objectContaining({ + type: 'updated', + action: expect.objectContaining({ type: 'fetch' }), + }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 5, + expect.objectContaining({ type: 'observerResultsUpdated' }), + ) + + // here query goes stale + expect(fn).toHaveBeenNthCalledWith( + 6, + expect.objectContaining({ + type: 'updated', + action: expect.objectContaining({ type: 'success' }), + }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 7, + expect.objectContaining({ type: 'observerOptionsUpdated' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 8, + expect.objectContaining({ type: 'observerResultsUpdated' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 9, + expect.objectContaining({ type: 'observerOptionsUpdated' }), + ) + }) + + unsubscribe() }) it('should not re-render when it should only re-render on data changes and the data did not change', async () => { diff --git a/packages/solid-query/src/__tests__/createQuery.test.tsx b/packages/solid-query/src/__tests__/createQuery.test.tsx index 6509af7eef..57f79c8ae3 100644 --- a/packages/solid-query/src/__tests__/createQuery.test.tsx +++ b/packages/solid-query/src/__tests__/createQuery.test.tsx @@ -2447,21 +2447,69 @@ describe('createQuery', () => { )) - await sleep(20) - unsubscribe() - // 1. Query added -> loading // 2. Observer result updated -> loading // 3. Observer added // 4. Query updated -> success - // 5. Observer result updated -> success - // 6. Query updated -> stale - // 7. Observer options updated + // 5. Observer options updated + // 6. Observer result updated -> success + // 7. Query updated -> stale // 8. Observer result updated -> stale - // 9. Observer options updated - // Number 9 wont run in Solid JS + // ~9. Observer options updated (won't run in Solid JS) + // Number 9 runs in react because the component re-renders after 8 - expect(fn).toHaveBeenCalledTimes(8) + // Order of actions 5-7 is different from React + + await waitFor(() => { + expect(fn).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ type: 'added' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ type: 'observerResultsUpdated' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ type: 'observerAdded' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 4, + expect.objectContaining({ + type: 'updated', + action: expect.objectContaining({ type: 'fetch' }), + }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 5, + expect.objectContaining({ type: 'observerOptionsUpdated' }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 6, + expect.objectContaining({ type: 'observerResultsUpdated' }), + ) + + // here query goes stale + expect(fn).toHaveBeenNthCalledWith( + 7, + expect.objectContaining({ + type: 'updated', + action: expect.objectContaining({ type: 'success' }), + }), + ) + + expect(fn).toHaveBeenNthCalledWith( + 8, + expect.objectContaining({ type: 'observerResultsUpdated' }), + ) + }) + + unsubscribe() }) it('should not re-render when it should only re-render on data changes and the data did not change', async () => { From e52afc54f542b41f891aaa978939e5727b6064e9 Mon Sep 17 00:00:00 2001 From: zorzysty Date: Sun, 6 Nov 2022 13:56:18 +0100 Subject: [PATCH 2/6] test: stabilize the test for notifying query cache when a query becomes stale - attempt#2 --- .../src/__tests__/useQuery.test.tsx | 44 ++-------------- .../src/__tests__/createQuery.test.tsx | 50 ++++--------------- 2 files changed, 15 insertions(+), 79 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 346a8d0d4b..fe1c74551a 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -2230,57 +2230,23 @@ describe('useQuery', () => { // 9. Observer options updated await waitFor(() => { - expect(fn).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ type: 'added' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ type: 'observerResultsUpdated' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 3, - expect.objectContaining({ type: 'observerAdded' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 4, + //here query succeeds + expect(fn).toHaveBeenCalledWith( expect.objectContaining({ type: 'updated', action: expect.objectContaining({ type: 'fetch' }), }), ) + }) - expect(fn).toHaveBeenNthCalledWith( - 5, - expect.objectContaining({ type: 'observerResultsUpdated' }), - ) - + await waitFor(() => { // here query goes stale - expect(fn).toHaveBeenNthCalledWith( - 6, + expect(fn).toHaveBeenCalledWith( expect.objectContaining({ type: 'updated', action: expect.objectContaining({ type: 'success' }), }), ) - - expect(fn).toHaveBeenNthCalledWith( - 7, - expect.objectContaining({ type: 'observerOptionsUpdated' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 8, - expect.objectContaining({ type: 'observerResultsUpdated' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 9, - expect.objectContaining({ type: 'observerOptionsUpdated' }), - ) }) unsubscribe() diff --git a/packages/solid-query/src/__tests__/createQuery.test.tsx b/packages/solid-query/src/__tests__/createQuery.test.tsx index 57f79c8ae3..b2baa96697 100644 --- a/packages/solid-query/src/__tests__/createQuery.test.tsx +++ b/packages/solid-query/src/__tests__/createQuery.test.tsx @@ -2451,62 +2451,32 @@ describe('createQuery', () => { // 2. Observer result updated -> loading // 3. Observer added // 4. Query updated -> success - // 5. Observer options updated - // 6. Observer result updated -> success - // 7. Query updated -> stale + // 5. Observer result updated -> success + // 6. Query updated -> stale + // 7. Observer options updated // 8. Observer result updated -> stale - // ~9. Observer options updated (won't run in Solid JS) - + // 9. Observer options updated + // Number 9 won't run in Solid JS // Number 9 runs in react because the component re-renders after 8 - // Order of actions 5-7 is different from React await waitFor(() => { - expect(fn).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ type: 'added' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ type: 'observerResultsUpdated' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 3, - expect.objectContaining({ type: 'observerAdded' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 4, + //here query succeeds + expect(fn).toHaveBeenCalledWith( expect.objectContaining({ type: 'updated', action: expect.objectContaining({ type: 'fetch' }), }), ) + }) - expect(fn).toHaveBeenNthCalledWith( - 5, - expect.objectContaining({ type: 'observerOptionsUpdated' }), - ) - - expect(fn).toHaveBeenNthCalledWith( - 6, - expect.objectContaining({ type: 'observerResultsUpdated' }), - ) - + await waitFor(() => { // here query goes stale - expect(fn).toHaveBeenNthCalledWith( - 7, + expect(fn).toHaveBeenCalledWith( expect.objectContaining({ type: 'updated', action: expect.objectContaining({ type: 'success' }), }), ) - - expect(fn).toHaveBeenNthCalledWith( - 8, - expect.objectContaining({ type: 'observerResultsUpdated' }), - ) }) unsubscribe() From a36518fe50308a0084f80ed795f50b1694ee081c Mon Sep 17 00:00:00 2001 From: zorzysty Date: Sun, 6 Nov 2022 14:06:38 +0100 Subject: [PATCH 3/6] test: stabilize the test persisting data when enabled is changed to false --- .../src/__tests__/useQuery.test.tsx | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index fe1c74551a..61b23f9642 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -3839,18 +3839,35 @@ describe('useQuery', () => { results.push(result) - React.useEffect(() => { - setActTimeout(() => { - setShouldFetch(false) - }, 5) - }, []) - - return null + return ( +
+
{result.data}
+
{shouldFetch ? 'enabled' : 'disabled'}
+ +
+ ) } - renderWithClient(queryClient, ) + const rendered = renderWithClient(queryClient, ) + + await waitFor(() => { + rendered.getByText('fetched data') + rendered.getByText('enabled') + }) + + fireEvent.click(rendered.getByRole('button', { name: /enable/i })) + + await waitFor(() => { + rendered.getByText('fetched data') + rendered.getByText('disabled') + }) - await sleep(50) expect(results.length).toBe(3) expect(results[0]).toMatchObject({ data: 'initial', isStale: true }) expect(results[1]).toMatchObject({ data: 'fetched data', isStale: true }) From 2630a1f4d45e3b33df87cd266e7a0de7a6bc5e1c Mon Sep 17 00:00:00 2001 From: zorzysty Date: Sun, 6 Nov 2022 14:21:55 +0100 Subject: [PATCH 4/6] test: stabilize the test for rendering with selected data --- .../src/__tests__/useQuery.test.tsx | 32 ++++++++++++------- .../src/__tests__/createQuery.test.tsx | 2 +- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 61b23f9642..04baf6f6e3 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -1022,24 +1022,32 @@ describe('useQuery', () => { states.push(state) - const { refetch } = state + return ( +
+
{state?.data}
+ +
+ ) + } - React.useEffect(() => { - setActTimeout(() => { - refetch() - }, 5) - }, [refetch]) + const rendered = renderWithClient(queryClient, ) - return null - } + await waitFor(() => { + rendered.getByText('test') + }) - renderWithClient(queryClient, ) + fireEvent.click(rendered.getByRole('button', { name: 'refetch' })) - await sleep(10) + await waitFor(() => { + rendered.getByText('test') + }) - expect(states.length).toBe(2) expect(states[0]).toMatchObject({ data: undefined }) expect(states[1]).toMatchObject({ data: 'test' }) + + // make sure no additional renders happen + await sleep(50) + expect(states.length).toBe(2) }) it('should throw an error when a selector throws', async () => { @@ -2230,7 +2238,7 @@ describe('useQuery', () => { // 9. Observer options updated await waitFor(() => { - //here query succeeds + // here query succeeds expect(fn).toHaveBeenCalledWith( expect.objectContaining({ type: 'updated', diff --git a/packages/solid-query/src/__tests__/createQuery.test.tsx b/packages/solid-query/src/__tests__/createQuery.test.tsx index b2baa96697..5c99685971 100644 --- a/packages/solid-query/src/__tests__/createQuery.test.tsx +++ b/packages/solid-query/src/__tests__/createQuery.test.tsx @@ -2460,7 +2460,7 @@ describe('createQuery', () => { // Number 9 runs in react because the component re-renders after 8 await waitFor(() => { - //here query succeeds + // here query succeeds expect(fn).toHaveBeenCalledWith( expect.objectContaining({ type: 'updated', From d94ed82c9deed66866ea6967863ad2ffa97c2702 Mon Sep 17 00:00:00 2001 From: zorzysty Date: Sun, 6 Nov 2022 15:52:38 +0100 Subject: [PATCH 5/6] test: stabilize the test for notifying query cache when a query becomes stale - attempt#3 (thanks TkDodo) --- .../query-core/src/tests/queryCache.test.tsx | 37 +++++++++++- .../src/__tests__/useQuery.test.tsx | 50 ---------------- .../src/__tests__/createQuery.test.tsx | 58 ------------------- 3 files changed, 36 insertions(+), 109 deletions(-) diff --git a/packages/query-core/src/tests/queryCache.test.tsx b/packages/query-core/src/tests/queryCache.test.tsx index 3707856ed2..1cacf9896d 100644 --- a/packages/query-core/src/tests/queryCache.test.tsx +++ b/packages/query-core/src/tests/queryCache.test.tsx @@ -1,7 +1,8 @@ import { sleep, queryKey, createQueryClient } from './utils' import type { QueryClient } from '..' -import { QueryCache } from '..' +import { QueryCache, QueryObserver } from '..' import type { Query } from '.././query' +import { waitFor } from '@testing-library/react' describe('queryCache', () => { let queryClient: QueryClient @@ -37,6 +38,40 @@ describe('queryCache', () => { expect(callback).toHaveBeenCalled() }) + test('should notify query cache when a query becomes stale', async () => { + const key = queryKey() + const events: Array = [] + const unsubscribe = queryCache.subscribe((event) => { + events.push(event.type) + }) + + const observer = new QueryObserver(queryClient, { + queryKey: key, + queryFn: () => 'data', + staleTime: 10, + }) + + const unsubScribeObserver = observer.subscribe(jest.fn) + + await waitFor(() => { + expect(events.length).toBe(8) + }) + + expect(events).toEqual([ + 'added', // 1. Query added -> loading + 'observerResultsUpdated', // 2. Observer result updated -> loading + 'observerAdded', // 3. Observer added + 'observerResultsUpdated', // 4. Observer result updated -> fetching + 'updated', // 5. Query updated -> fetching + 'observerResultsUpdated', // 6. Observer result updated -> success + 'updated', // 7. Query updated -> success + 'observerResultsUpdated', // 8. Observer result updated -> stale + ]) + + unsubscribe() + unsubScribeObserver() + }) + test('should include the queryCache and query when notifying listeners', async () => { const key = queryKey() const callback = jest.fn() diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 04baf6f6e3..537c98f7b7 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -2210,56 +2210,6 @@ describe('useQuery', () => { expect(states[2]).toMatchObject({ isStale: true }) }) - it('should notify query cache when a query becomes stale', async () => { - const key = queryKey() - const states: UseQueryResult[] = [] - const fn = jest.fn() - - const unsubscribe = queryCache.subscribe(fn) - - function Page() { - const state = useQuery(key, () => 'test', { - staleTime: 10, - }) - states.push(state) - return null - } - - renderWithClient(queryClient, ) - - // 1. Query added -> loading - // 2. Observer result updated -> loading - // 3. Observer added - // 4. Query updated -> success - // 5. Observer result updated -> success - // 6. Query updated -> stale - // 7. Observer options updated - // 8. Observer result updated -> stale - // 9. Observer options updated - - await waitFor(() => { - // here query succeeds - expect(fn).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'updated', - action: expect.objectContaining({ type: 'fetch' }), - }), - ) - }) - - await waitFor(() => { - // here query goes stale - expect(fn).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'updated', - action: expect.objectContaining({ type: 'success' }), - }), - ) - }) - - unsubscribe() - }) - it('should not re-render when it should only re-render on data changes and the data did not change', async () => { const key = queryKey() const states: UseQueryResult[] = [] diff --git a/packages/solid-query/src/__tests__/createQuery.test.tsx b/packages/solid-query/src/__tests__/createQuery.test.tsx index 5c99685971..7f4217501b 100644 --- a/packages/solid-query/src/__tests__/createQuery.test.tsx +++ b/packages/solid-query/src/__tests__/createQuery.test.tsx @@ -2424,64 +2424,6 @@ describe('createQuery', () => { expect(states[2]).toMatchObject({ isStale: true }) }) - it('should notify query cache when a query becomes stale', async () => { - const key = queryKey() - const states: CreateQueryResult[] = [] - const fn = jest.fn() - - const unsubscribe = queryCache.subscribe(fn) - - function Page() { - const state = createQuery(key, () => 'test', { - staleTime: 10, - }) - createRenderEffect(() => { - states.push({ ...state }) - }) - return null - } - - render(() => ( - - - - )) - - // 1. Query added -> loading - // 2. Observer result updated -> loading - // 3. Observer added - // 4. Query updated -> success - // 5. Observer result updated -> success - // 6. Query updated -> stale - // 7. Observer options updated - // 8. Observer result updated -> stale - // 9. Observer options updated - // Number 9 won't run in Solid JS - // Number 9 runs in react because the component re-renders after 8 - - await waitFor(() => { - // here query succeeds - expect(fn).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'updated', - action: expect.objectContaining({ type: 'fetch' }), - }), - ) - }) - - await waitFor(() => { - // here query goes stale - expect(fn).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'updated', - action: expect.objectContaining({ type: 'success' }), - }), - ) - }) - - unsubscribe() - }) - it('should not re-render when it should only re-render on data changes and the data did not change', async () => { const key = queryKey() const states: CreateQueryResult[] = [] From 37be96d680ec96385776bc6d949d0a1fc6e6212f Mon Sep 17 00:00:00 2001 From: zorzysty Date: Sun, 6 Nov 2022 16:10:09 +0100 Subject: [PATCH 6/6] test: stabilize the test for keeping the previous data when keepPreviousData is set --- .../src/__tests__/useQuery.test.tsx | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 537c98f7b7..6667f16139 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -1610,18 +1610,21 @@ describe('useQuery', () => { states.push(state) - React.useEffect(() => { - setActTimeout(() => { - setCount(1) - }, 20) - }, []) - - return null + return ( +
+
data: {state.data}
+ +
+ ) } - renderWithClient(queryClient, ) + const rendered = renderWithClient(queryClient, ) - await waitFor(() => expect(states.length).toBe(5)) + await waitFor(() => rendered.getByText('data: 0')) + + fireEvent.click(rendered.getByRole('button', { name: 'setCount' })) + + await waitFor(() => rendered.getByText('data: 1')) // Initial expect(states[0]).toMatchObject({ @@ -1644,15 +1647,8 @@ describe('useQuery', () => { isSuccess: true, isPreviousData: true, }) - // Hook state update - expect(states[3]).toMatchObject({ - data: 0, - isFetching: true, - isSuccess: true, - isPreviousData: true, - }) // New data - expect(states[4]).toMatchObject({ + expect(states[3]).toMatchObject({ data: 1, isFetching: false, isSuccess: true,