From 7c38a96537d0fc6a20d73eec6aa84e341b53fbb6 Mon Sep 17 00:00:00 2001 From: MINSEONG KIM Date: Wed, 4 Dec 2024 14:07:35 +0900 Subject: [PATCH 1/6] =?UTF-8?q?fix(react-query):=20ensureSuspenseTimers?= =?UTF-8?q?=C2=A0should=20ALWAYS=20set=20staleTime=20to=201000=20when=20it?= =?UTF-8?q?=20is=20below=201000.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/react-query/src/suspense.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/react-query/src/suspense.ts b/packages/react-query/src/suspense.ts index 497bb83bd8..b021af7cdb 100644 --- a/packages/react-query/src/suspense.ts +++ b/packages/react-query/src/suspense.ts @@ -22,11 +22,14 @@ export const ensureSuspenseTimers = ( defaultedOptions: DefaultedQueryObserverOptions, ) => { if (defaultedOptions.suspense) { - // Always set stale time when using suspense to prevent - // fetching again when directly mounting after suspending - if (defaultedOptions.staleTime === undefined) { - defaultedOptions.staleTime = 1000 - } + // Handle staleTime to ensure minimum 1000ms in Suspense mode + // This prevents unnecessary refetching when components remount after suspending + defaultedOptions.staleTime = + typeof defaultedOptions.staleTime === 'function' + ? (...args: Parameters) => + Math.max(defaultedOptions.staleTime(...args) ?? 1000, 1000) + : Math.max(defaultedOptions.staleTime ?? 1000, 1000) + if (typeof defaultedOptions.gcTime === 'number') { defaultedOptions.gcTime = Math.max(defaultedOptions.gcTime, 1000) } @@ -65,3 +68,4 @@ export const fetchOptimistic = < observer.fetchOptimistic(defaultedOptions).catch(() => { errorResetBoundary.clearReset() }) + From 2ebd02bfbb9dcffeb23028bc1f903963e8cc4db0 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 08:05:17 +0000 Subject: [PATCH 2/6] ci: apply automated fixes --- packages/react-query/src/suspense.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-query/src/suspense.ts b/packages/react-query/src/suspense.ts index b021af7cdb..00c18d8093 100644 --- a/packages/react-query/src/suspense.ts +++ b/packages/react-query/src/suspense.ts @@ -68,4 +68,3 @@ export const fetchOptimistic = < observer.fetchOptimistic(defaultedOptions).catch(() => { errorResetBoundary.clearReset() }) - From 7a80a0219451aa825fd2a2465ecb23aea0713b2f Mon Sep 17 00:00:00 2001 From: minseong Date: Thu, 5 Dec 2024 00:32:16 +0900 Subject: [PATCH 3/6] fix(react-query): fix ensureSuspenseTimers logic --- packages/react-query/src/suspense.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/react-query/src/suspense.ts b/packages/react-query/src/suspense.ts index 00c18d8093..6981e07422 100644 --- a/packages/react-query/src/suspense.ts +++ b/packages/react-query/src/suspense.ts @@ -21,14 +21,15 @@ export const defaultThrowOnError = < export const ensureSuspenseTimers = ( defaultedOptions: DefaultedQueryObserverOptions, ) => { + const originalStaleTime = defaultedOptions.staleTime + if (defaultedOptions.suspense) { // Handle staleTime to ensure minimum 1000ms in Suspense mode // This prevents unnecessary refetching when components remount after suspending defaultedOptions.staleTime = - typeof defaultedOptions.staleTime === 'function' - ? (...args: Parameters) => - Math.max(defaultedOptions.staleTime(...args) ?? 1000, 1000) - : Math.max(defaultedOptions.staleTime ?? 1000, 1000) + typeof originalStaleTime === 'function' + ? (...args) => Math.max(originalStaleTime(...args), 1000) + : Math.max(originalStaleTime ?? 1000, 1000) if (typeof defaultedOptions.gcTime === 'number') { defaultedOptions.gcTime = Math.max(defaultedOptions.gcTime, 1000) From 8bf1909cbb222d4f80f435adcd6d372e52a27aca Mon Sep 17 00:00:00 2001 From: minseong Date: Thu, 5 Dec 2024 00:33:55 +0900 Subject: [PATCH 4/6] test(react-query): add test code --- .../src/__tests__/suspense.test.tsx | 231 ++++++++++++++++++ 1 file changed, 231 insertions(+) create mode 100644 packages/react-query/src/__tests__/suspense.test.tsx diff --git a/packages/react-query/src/__tests__/suspense.test.tsx b/packages/react-query/src/__tests__/suspense.test.tsx new file mode 100644 index 0000000000..0a285fa6f4 --- /dev/null +++ b/packages/react-query/src/__tests__/suspense.test.tsx @@ -0,0 +1,231 @@ +import { act, render, waitFor } from '@testing-library/react' +import { Suspense } from 'react' +import { beforeEach, describe, expect, it } from 'vitest' +import { QueryClient, QueryClientProvider, useSuspenseQuery } from '..' +import { queryKey, renderWithClient, sleep } from './utils' + +describe('Suspense Timer Tests', () => { + let queryClient: QueryClient + + beforeEach(() => { + queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }) + }) + + it('should enforce minimum staleTime of 1000ms when using suspense with number', async () => { + const shortStaleTime = 10 + let fetchCount = 0 + + function TestComponent() { + const { data } = useSuspenseQuery({ + queryKey: ['test'], + queryFn: () => { + fetchCount++ + return 'data' + }, + staleTime: shortStaleTime, + }) + return
{data}
+ } + + const wrapper = render( + + + + + , + ) + + await waitFor(() => { + expect(wrapper.getByText('data')).toBeInTheDocument() + }) + + wrapper.rerender( + + + + + , + ) + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 100)) + }) + + expect(fetchCount).toBe(1) + }) + + it('should enforce minimum staleTime of 1000ms when using suspense with function', async () => { + let fetchCount = 0 + const staleTimeFunc = () => 10 + + function TestComponent() { + const { data } = useSuspenseQuery({ + queryKey: ['test-func'], + queryFn: () => { + fetchCount++ + return 'data' + }, + staleTime: staleTimeFunc, + }) + return
{data}
+ } + + const wrapper = render( + + + + + , + ) + + await waitFor(() => { + expect(wrapper.getByText('data')).toBeInTheDocument() + }) + + wrapper.rerender( + + + + + , + ) + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 100)) + }) + + expect(fetchCount).toBe(1) + }) + + it('should respect staleTime when value is greater than 1000ms', async () => { + const key = queryKey() + const staleTime = 2000 + let fetchCount = 0 + + function Component() { + const result = useSuspenseQuery({ + queryKey: key, + queryFn: async () => { + fetchCount++ + await sleep(10) + return 'data' + }, + staleTime, + }) + return
data: {result.data}
+ } + + const rendered = renderWithClient( + queryClient, + + + , + ) + + await waitFor(() => rendered.getByText('data: data')) + + // Manually trigger a remount + rendered.rerender( + + + , + ) + + // Wait longer than 1000ms but less than staleTime + await act(async () => { + await sleep(1500) + }) + + // Should not refetch as we're still within the staleTime + expect(fetchCount).toBe(1) + }) + + it('should enforce minimum staleTime when undefined is provided', async () => { + const key = queryKey() + let fetchCount = 0 + + function Component() { + const result = useSuspenseQuery({ + queryKey: key, + queryFn: async () => { + fetchCount++ + await sleep(10) + return 'data' + }, + // Explicitly set staleTime as undefined + staleTime: undefined, + }) + return
data: {result.data}
+ } + + const rendered = renderWithClient( + queryClient, + + + , + ) + + await waitFor(() => rendered.getByText('data: data')) + + rendered.rerender( + + + , + ) + + // Wait less than enforced 1000ms + await act(async () => { + await sleep(500) + }) + + // Should not refetch as minimum staleTime of 1000ms is enforced + expect(fetchCount).toBe(1) + }) + + it('should respect staleTime when function returns value greater than 1000ms', async () => { + const key = queryKey() + let fetchCount = 0 + + function Component() { + const result = useSuspenseQuery({ + queryKey: key, + queryFn: async () => { + fetchCount++ + await sleep(10) + return 'data' + }, + staleTime: () => 3000, + }) + return
data: {result.data}
+ } + + const rendered = renderWithClient( + queryClient, + + + , + ) + + await waitFor(() => rendered.getByText('data: data')) + + rendered.rerender( + + + , + ) + + // Wait longer than 1000ms but less than returned staleTime + await act(async () => { + await sleep(2000) + }) + + // Should not refetch as we're still within the returned staleTime + expect(fetchCount).toBe(1) + }) +}) From 618c6d765088cb7df9cfaeaf4261876ce3f459d4 Mon Sep 17 00:00:00 2001 From: minseong Date: Thu, 5 Dec 2024 00:34:22 +0900 Subject: [PATCH 5/6] test(react-query): delete unnecessary test code --- .../src/__tests__/useSuspenseQuery.test.tsx | 55 ------------------- 1 file changed, 55 deletions(-) diff --git a/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx b/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx index 003655ec1d..617ba40d20 100644 --- a/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx +++ b/packages/react-query/src/__tests__/useSuspenseQuery.test.tsx @@ -327,61 +327,6 @@ describe('useSuspenseQuery', () => { consoleMock.mockRestore() }) - it('should refetch when re-mounting', async () => { - const key = queryKey() - let count = 0 - - function Component() { - const result = useSuspenseQuery({ - queryKey: key, - queryFn: async () => { - await sleep(100) - count++ - return count - }, - retry: false, - staleTime: 0, - }) - return ( -
- data: {result.data} - fetching: {result.isFetching ? 'true' : 'false'} -
- ) - } - - function Page() { - const [show, setShow] = React.useState(true) - return ( -
- - - {show && } - -
- ) - } - - const rendered = renderWithClient(queryClient, ) - - await waitFor(() => rendered.getByText('Loading...')) - await waitFor(() => rendered.getByText('data: 1')) - await waitFor(() => rendered.getByText('fetching: false')) - await waitFor(() => rendered.getByText('hide')) - fireEvent.click(rendered.getByText('hide')) - await waitFor(() => rendered.getByText('show')) - fireEvent.click(rendered.getByText('show')) - await waitFor(() => rendered.getByText('fetching: true')) - await waitFor(() => rendered.getByText('data: 2')) - await waitFor(() => rendered.getByText('fetching: false')) - }) - it('should set staleTime when having passed a function', async () => { const key = queryKey() let count = 0 From 46057ca7076a40c7019346f7d0e4720d00f078f3 Mon Sep 17 00:00:00 2001 From: minseong Date: Fri, 13 Dec 2024 04:32:11 +0900 Subject: [PATCH 6/6] test(react-query): Refectoring test code to reduce test time --- .../src/__tests__/suspense.test.tsx | 265 ++++++++---------- 1 file changed, 114 insertions(+), 151 deletions(-) diff --git a/packages/react-query/src/__tests__/suspense.test.tsx b/packages/react-query/src/__tests__/suspense.test.tsx index 0a285fa6f4..cc8ae1fe7f 100644 --- a/packages/react-query/src/__tests__/suspense.test.tsx +++ b/packages/react-query/src/__tests__/suspense.test.tsx @@ -1,11 +1,55 @@ import { act, render, waitFor } from '@testing-library/react' import { Suspense } from 'react' -import { beforeEach, describe, expect, it } from 'vitest' +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest' import { QueryClient, QueryClientProvider, useSuspenseQuery } from '..' -import { queryKey, renderWithClient, sleep } from './utils' +import { queryKey } from './utils' +import type { QueryKey } from '..' + +function renderWithSuspense(client: QueryClient, ui: React.ReactNode) { + return render( + + {ui} + , + ) +} + +function createTestQuery(options: { + fetchCount: { count: number } + queryKey: QueryKey + staleTime?: number | (() => number) +}) { + return function TestComponent() { + const { data } = useSuspenseQuery({ + queryKey: options.queryKey, + queryFn: () => { + options.fetchCount.count++ + return 'data' + }, + staleTime: options.staleTime, + }) + return
data: {data}
+ } +} describe('Suspense Timer Tests', () => { let queryClient: QueryClient + let fetchCount: { count: number } + + beforeAll(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + }) + + afterAll(() => { + vi.useRealTimers() + }) beforeEach(() => { queryClient = new QueryClient({ @@ -15,37 +59,21 @@ describe('Suspense Timer Tests', () => { }, }, }) + fetchCount = { count: 0 } }) it('should enforce minimum staleTime of 1000ms when using suspense with number', async () => { - const shortStaleTime = 10 - let fetchCount = 0 - - function TestComponent() { - const { data } = useSuspenseQuery({ - queryKey: ['test'], - queryFn: () => { - fetchCount++ - return 'data' - }, - staleTime: shortStaleTime, - }) - return
{data}
- } + const TestComponent = createTestQuery({ + fetchCount, + queryKey: ['test'], + staleTime: 10, + }) - const wrapper = render( - - - - - , - ) + const rendered = renderWithSuspense(queryClient, ) - await waitFor(() => { - expect(wrapper.getByText('data')).toBeInTheDocument() - }) + await waitFor(() => rendered.getByText('data: data')) - wrapper.rerender( + rendered.rerender( @@ -53,42 +81,25 @@ describe('Suspense Timer Tests', () => { , ) - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 100)) + act(() => { + vi.advanceTimersByTime(100) }) - expect(fetchCount).toBe(1) + expect(fetchCount.count).toBe(1) }) it('should enforce minimum staleTime of 1000ms when using suspense with function', async () => { - let fetchCount = 0 - const staleTimeFunc = () => 10 - - function TestComponent() { - const { data } = useSuspenseQuery({ - queryKey: ['test-func'], - queryFn: () => { - fetchCount++ - return 'data' - }, - staleTime: staleTimeFunc, - }) - return
{data}
- } + const TestComponent = createTestQuery({ + fetchCount, + queryKey: ['test-func'], + staleTime: () => 10, + }) - const wrapper = render( - - - - - , - ) + const rendered = renderWithSuspense(queryClient, ) - await waitFor(() => { - expect(wrapper.getByText('data')).toBeInTheDocument() - }) + await waitFor(() => rendered.getByText('data: data')) - wrapper.rerender( + rendered.rerender( @@ -96,136 +107,88 @@ describe('Suspense Timer Tests', () => { , ) - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 100)) + act(() => { + vi.advanceTimersByTime(100) }) - expect(fetchCount).toBe(1) + expect(fetchCount.count).toBe(1) }) it('should respect staleTime when value is greater than 1000ms', async () => { - const key = queryKey() - const staleTime = 2000 - let fetchCount = 0 - - function Component() { - const result = useSuspenseQuery({ - queryKey: key, - queryFn: async () => { - fetchCount++ - await sleep(10) - return 'data' - }, - staleTime, - }) - return
data: {result.data}
- } - - const rendered = renderWithClient( - queryClient, - - - , - ) + const TestComponent = createTestQuery({ + fetchCount, + queryKey: queryKey(), + staleTime: 2000, + }) + + const rendered = renderWithSuspense(queryClient, ) await waitFor(() => rendered.getByText('data: data')) - // Manually trigger a remount rendered.rerender( - - - , + + + + + , ) - // Wait longer than 1000ms but less than staleTime - await act(async () => { - await sleep(1500) + act(() => { + vi.advanceTimersByTime(1500) }) - // Should not refetch as we're still within the staleTime - expect(fetchCount).toBe(1) + expect(fetchCount.count).toBe(1) }) it('should enforce minimum staleTime when undefined is provided', async () => { - const key = queryKey() - let fetchCount = 0 - - function Component() { - const result = useSuspenseQuery({ - queryKey: key, - queryFn: async () => { - fetchCount++ - await sleep(10) - return 'data' - }, - // Explicitly set staleTime as undefined - staleTime: undefined, - }) - return
data: {result.data}
- } - - const rendered = renderWithClient( - queryClient, - - - , - ) + const TestComponent = createTestQuery({ + fetchCount, + queryKey: queryKey(), + staleTime: undefined, + }) + + const rendered = renderWithSuspense(queryClient, ) await waitFor(() => rendered.getByText('data: data')) rendered.rerender( - - - , + + + + + , ) - // Wait less than enforced 1000ms - await act(async () => { - await sleep(500) + act(() => { + vi.advanceTimersByTime(500) }) - // Should not refetch as minimum staleTime of 1000ms is enforced - expect(fetchCount).toBe(1) + expect(fetchCount.count).toBe(1) }) it('should respect staleTime when function returns value greater than 1000ms', async () => { - const key = queryKey() - let fetchCount = 0 - - function Component() { - const result = useSuspenseQuery({ - queryKey: key, - queryFn: async () => { - fetchCount++ - await sleep(10) - return 'data' - }, - staleTime: () => 3000, - }) - return
data: {result.data}
- } - - const rendered = renderWithClient( - queryClient, - - - , - ) + const TestComponent = createTestQuery({ + fetchCount, + queryKey: queryKey(), + staleTime: () => 3000, + }) + + const rendered = renderWithSuspense(queryClient, ) await waitFor(() => rendered.getByText('data: data')) rendered.rerender( - - - , + + + + + , ) - // Wait longer than 1000ms but less than returned staleTime - await act(async () => { - await sleep(2000) + act(() => { + vi.advanceTimersByTime(2000) }) - // Should not refetch as we're still within the returned staleTime - expect(fetchCount).toBe(1) + expect(fetchCount.count).toBe(1) }) })