From 3b569b4e9c08c19ad25eb8289aea303292245f2f Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Sat, 4 Jan 2025 13:25:24 +0100 Subject: [PATCH 01/12] chore: add test for switching enabled state --- .../src/__tests__/queryObserver.test.tsx | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/query-core/src/__tests__/queryObserver.test.tsx b/packages/query-core/src/__tests__/queryObserver.test.tsx index fa3890000d..fc117f0856 100644 --- a/packages/query-core/src/__tests__/queryObserver.test.tsx +++ b/packages/query-core/src/__tests__/queryObserver.test.tsx @@ -1233,4 +1233,35 @@ describe('queryObserver', () => { unsubscribe() }) + + test('switching enabled state should reuse the same promise', async () => { + const key = queryKey() + + const observer = new QueryObserver(queryClient, { + queryKey: key, + enabled: false, + queryFn: () => 'data', + }) + const results: Array = [] + + + const unsubscribe = observer.subscribe((result) => { + results.push(result) + }) + + await sleep(1) + + observer.setOptions({ queryKey: key, queryFn: () => 'data', enabled: true }) + + + await waitFor(() => { + expect(results.at(-1)?.status).toBe('success') + }) + + unsubscribe() + + + const promises = new Set(results.map((result) => result.promise)) + expect(promises.size).toBe(1) + }) }) From 7bf0f38e2b9be1e5895c06f442426c854a04badf Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Sat, 4 Jan 2025 13:26:48 +0100 Subject: [PATCH 02/12] remove sleep --- packages/query-core/src/__tests__/queryObserver.test.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/query-core/src/__tests__/queryObserver.test.tsx b/packages/query-core/src/__tests__/queryObserver.test.tsx index fc117f0856..08cb4b3693 100644 --- a/packages/query-core/src/__tests__/queryObserver.test.tsx +++ b/packages/query-core/src/__tests__/queryObserver.test.tsx @@ -1249,8 +1249,6 @@ describe('queryObserver', () => { results.push(result) }) - await sleep(1) - observer.setOptions({ queryKey: key, queryFn: () => 'data', enabled: true }) From e01497a271c4351feda91907dbe93c25c1b3ddfd Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Sat, 4 Jan 2025 13:27:48 +0100 Subject: [PATCH 03/12] mkay --- .../query-core/src/__tests__/queryObserver.test.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/query-core/src/__tests__/queryObserver.test.tsx b/packages/query-core/src/__tests__/queryObserver.test.tsx index 08cb4b3693..e44814c831 100644 --- a/packages/query-core/src/__tests__/queryObserver.test.tsx +++ b/packages/query-core/src/__tests__/queryObserver.test.tsx @@ -11,6 +11,7 @@ import { waitFor } from '@testing-library/dom' import { QueryObserver, focusManager } from '..' import { createQueryClient, queryKey, sleep } from './utils' import type { QueryClient, QueryObserverResult } from '..' +import { pendingThenable } from '../thenable' describe('queryObserver', () => { let queryClient: QueryClient @@ -1243,18 +1244,23 @@ describe('queryObserver', () => { queryFn: () => 'data', }) const results: Array = [] + + const success = pendingThenable() const unsubscribe = observer.subscribe((result) => { + results.push(result) + + if (result.status === 'success') { + success.resolve() + } }) observer.setOptions({ queryKey: key, queryFn: () => 'data', enabled: true }) - await waitFor(() => { - expect(results.at(-1)?.status).toBe('success') - }) + await success unsubscribe() From 14556693b874ee1d6f29238739e75dafbf100b52 Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Sat, 4 Jan 2025 13:28:13 +0100 Subject: [PATCH 04/12] lint --- packages/query-core/src/__tests__/queryObserver.test.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/__tests__/queryObserver.test.tsx b/packages/query-core/src/__tests__/queryObserver.test.tsx index e44814c831..5ea4c0a1ee 100644 --- a/packages/query-core/src/__tests__/queryObserver.test.tsx +++ b/packages/query-core/src/__tests__/queryObserver.test.tsx @@ -1257,7 +1257,11 @@ describe('queryObserver', () => { } }) - observer.setOptions({ queryKey: key, queryFn: () => 'data', enabled: true }) + observer.setOptions({ + queryKey: key, + queryFn: () => 'data', + enabled: true + }) await success From 2ad266e847d0c4a87db253cd2be284fda53267e0 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 4 Jan 2025 12:29:19 +0000 Subject: [PATCH 05/12] ci: apply automated fixes --- .../src/__tests__/queryObserver.test.tsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/query-core/src/__tests__/queryObserver.test.tsx b/packages/query-core/src/__tests__/queryObserver.test.tsx index 5ea4c0a1ee..898eeced8c 100644 --- a/packages/query-core/src/__tests__/queryObserver.test.tsx +++ b/packages/query-core/src/__tests__/queryObserver.test.tsx @@ -1246,10 +1246,8 @@ describe('queryObserver', () => { const results: Array = [] const success = pendingThenable() - const unsubscribe = observer.subscribe((result) => { - results.push(result) if (result.status === 'success') { @@ -1257,18 +1255,16 @@ describe('queryObserver', () => { } }) - observer.setOptions({ - queryKey: key, - queryFn: () => 'data', - enabled: true + observer.setOptions({ + queryKey: key, + queryFn: () => 'data', + enabled: true, }) - await success - + unsubscribe() - const promises = new Set(results.map((result) => result.promise)) expect(promises.size).toBe(1) }) From b7cf7fc530f45b973b199aaacf7d34acf6ffb805 Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Mon, 6 Jan 2025 18:36:27 +0100 Subject: [PATCH 06/12] wip --- packages/react-query/src/useBaseQuery.ts | 28 ++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/packages/react-query/src/useBaseQuery.ts b/packages/react-query/src/useBaseQuery.ts index bcbf700ef7..11805549e4 100644 --- a/packages/react-query/src/useBaseQuery.ts +++ b/packages/react-query/src/useBaseQuery.ts @@ -70,9 +70,7 @@ export function useBaseQuery< useClearResetErrorBoundary(errorResetBoundary) // this needs to be invoked before creating the Observer because that can create a cache entry - const isNewCacheEntry = !client - .getQueryCache() - .get(defaultedOptions.queryHash) + const cacheEntry = client.getQueryCache().get(defaultedOptions.queryHash) const [observer] = React.useState( () => @@ -82,6 +80,7 @@ export function useBaseQuery< ), ) + const result = observer.getOptimisticResult(defaultedOptions) React.useSyncExternalStore( @@ -130,6 +129,7 @@ export function useBaseQuery< >(defaultedOptions.queryHash), }) ) { + console.log('throwing error') throw result.error } @@ -138,12 +138,32 @@ export function useBaseQuery< result, ) + console.log( + 'prefetchInRender', + defaultedOptions.experimental_prefetchInRender, + { + willFetch: willFetch(result, isRestoring), + isRestoring: isRestoring, + isLoading: result.isLoading, + isFetching: result.isFetching, + isServer: isServer, + }, + ) if ( defaultedOptions.experimental_prefetchInRender && !isServer && willFetch(result, isRestoring) ) { - const promise = isNewCacheEntry + const cacheEntryState = cacheEntry?.state + + const shouldFetch = + !cacheEntryState || + (cacheEntryState.data === undefined && + cacheEntryState.status === 'pending' && + cacheEntryState.fetchStatus === 'idle') + + console.log({ shouldFetch }) + const promise = shouldFetch ? // Fetch immediately on render in order to ensure `.promise` is resolved even if the component is unmounted fetchOptimistic(defaultedOptions, observer, errorResetBoundary) : // subscribe to the "cache promise" so that we can finalize the currentThenable once data comes in From b222b14288dc2cfccb033730d462d30be372936e Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Mon, 6 Jan 2025 18:36:37 +0100 Subject: [PATCH 07/12] wip --- .../src/__tests__/useQuery.promise.test.tsx | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/packages/react-query/src/__tests__/useQuery.promise.test.tsx b/packages/react-query/src/__tests__/useQuery.promise.test.tsx index ec07d2d2b2..e2b7d057a4 100644 --- a/packages/react-query/src/__tests__/useQuery.promise.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.promise.test.tsx @@ -1377,4 +1377,75 @@ describe('useQuery().promise', () => { .observers.length, ).toBe(2) }) + + it('should handle enabled state changes with suspense', async () => { + const key = queryKey() + const renderStream = createRenderStream({ snapshotDOM: true }) + const queryFn = vi.fn(async () => { + await sleep(1) + return 'test' + }) + + function MyComponent(props: { enabled: boolean }) { + useTrackRenders() + const query = useQuery({ + queryKey: key, + queryFn, + enabled: props.enabled, + staleTime: Infinity, + }) + + const data = React.use(query.promise) + return <>{data} + } + + function Loading() { + useTrackRenders() + return <>loading.. + } + + function Page() { + useTrackRenders() + const enabledState = React.useState(false) + const enabled = enabledState[0] + const setEnabled = enabledState[1] + + return ( +
+ + }> + + +
+ ) + } + + const rendered = await renderStream.render( + + + , + ) + + { + const result = await renderStream.takeRender() + result.withinDOM().getByText('loading..') + } + + expect(queryFn).toHaveBeenCalledTimes(0) + rendered.getByText('enable').click() + + { + const result = await renderStream.takeRender() + result.withinDOM().getByText('loading..') + } + + expect(queryFn).toHaveBeenCalledTimes(1) + + { + const result = await renderStream.takeRender() + result.withinDOM().getByText('test') + } + + expect(queryFn).toHaveBeenCalledTimes(1) + }) }) From a7994c20520fd03f6c72b36a0820512c2e6f8412 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 6 Jan 2025 17:38:09 +0000 Subject: [PATCH 08/12] ci: apply automated fixes --- packages/react-query/src/useBaseQuery.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-query/src/useBaseQuery.ts b/packages/react-query/src/useBaseQuery.ts index 11805549e4..7ca3e2adea 100644 --- a/packages/react-query/src/useBaseQuery.ts +++ b/packages/react-query/src/useBaseQuery.ts @@ -80,7 +80,6 @@ export function useBaseQuery< ), ) - const result = observer.getOptimisticResult(defaultedOptions) React.useSyncExternalStore( From f1daa83c2edf93647e7cbf538f74860f3f7a0b71 Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Mon, 6 Jan 2025 18:44:08 +0100 Subject: [PATCH 09/12] meep --- packages/react-query/src/useBaseQuery.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-query/src/useBaseQuery.ts b/packages/react-query/src/useBaseQuery.ts index 11805549e4..f7703d0576 100644 --- a/packages/react-query/src/useBaseQuery.ts +++ b/packages/react-query/src/useBaseQuery.ts @@ -154,6 +154,7 @@ export function useBaseQuery< !isServer && willFetch(result, isRestoring) ) { + // This fetching in the render should likely be done as part of the getOptimisticResult() considering https://github.com/TanStack/query/issues/8507 const cacheEntryState = cacheEntry?.state const shouldFetch = @@ -162,7 +163,6 @@ export function useBaseQuery< cacheEntryState.status === 'pending' && cacheEntryState.fetchStatus === 'idle') - console.log({ shouldFetch }) const promise = shouldFetch ? // Fetch immediately on render in order to ensure `.promise` is resolved even if the component is unmounted fetchOptimistic(defaultedOptions, observer, errorResetBoundary) From a269d27a234deebeab3c7514cf5d0df4ba5bab33 Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Mon, 6 Jan 2025 18:44:42 +0100 Subject: [PATCH 10/12] cool --- packages/react-query/src/useBaseQuery.ts | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/react-query/src/useBaseQuery.ts b/packages/react-query/src/useBaseQuery.ts index f7703d0576..13a74d4866 100644 --- a/packages/react-query/src/useBaseQuery.ts +++ b/packages/react-query/src/useBaseQuery.ts @@ -129,7 +129,6 @@ export function useBaseQuery< >(defaultedOptions.queryHash), }) ) { - console.log('throwing error') throw result.error } @@ -138,30 +137,19 @@ export function useBaseQuery< result, ) - console.log( - 'prefetchInRender', - defaultedOptions.experimental_prefetchInRender, - { - willFetch: willFetch(result, isRestoring), - isRestoring: isRestoring, - isLoading: result.isLoading, - isFetching: result.isFetching, - isServer: isServer, - }, - ) if ( defaultedOptions.experimental_prefetchInRender && !isServer && willFetch(result, isRestoring) ) { // This fetching in the render should likely be done as part of the getOptimisticResult() considering https://github.com/TanStack/query/issues/8507 - const cacheEntryState = cacheEntry?.state + const state = cacheEntry?.state const shouldFetch = - !cacheEntryState || - (cacheEntryState.data === undefined && - cacheEntryState.status === 'pending' && - cacheEntryState.fetchStatus === 'idle') + !state || + (state.data === undefined && + state.status === 'pending' && + state.fetchStatus === 'idle') const promise = shouldFetch ? // Fetch immediately on render in order to ensure `.promise` is resolved even if the component is unmounted From 7669e03a15a130bcf56c91afa17770a27ea55204 Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Mon, 6 Jan 2025 18:46:25 +0100 Subject: [PATCH 11/12] tweak --- packages/react-query/src/__tests__/useQuery.promise.test.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.promise.test.tsx b/packages/react-query/src/__tests__/useQuery.promise.test.tsx index e2b7d057a4..440e0119b5 100644 --- a/packages/react-query/src/__tests__/useQuery.promise.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.promise.test.tsx @@ -1387,7 +1387,6 @@ describe('useQuery().promise', () => { }) function MyComponent(props: { enabled: boolean }) { - useTrackRenders() const query = useQuery({ queryKey: key, queryFn, @@ -1400,12 +1399,10 @@ describe('useQuery().promise', () => { } function Loading() { - useTrackRenders() return <>loading.. } function Page() { - useTrackRenders() const enabledState = React.useState(false) const enabled = enabledState[0] const setEnabled = enabledState[1] From 46b7e06d9ddd8fb52be9dc10744f8f4dc542002d Mon Sep 17 00:00:00 2001 From: Alexander Johansson Date: Mon, 6 Jan 2025 18:55:16 +0100 Subject: [PATCH 12/12] eeek --- packages/query-core/src/__tests__/queryObserver.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/query-core/src/__tests__/queryObserver.test.tsx b/packages/query-core/src/__tests__/queryObserver.test.tsx index 898eeced8c..d54da63309 100644 --- a/packages/query-core/src/__tests__/queryObserver.test.tsx +++ b/packages/query-core/src/__tests__/queryObserver.test.tsx @@ -1,3 +1,4 @@ +import { waitFor } from '@testing-library/dom' import { afterEach, beforeEach, @@ -7,11 +8,10 @@ import { test, vi, } from 'vitest' -import { waitFor } from '@testing-library/dom' import { QueryObserver, focusManager } from '..' +import { pendingThenable } from '../thenable' import { createQueryClient, queryKey, sleep } from './utils' import type { QueryClient, QueryObserverResult } from '..' -import { pendingThenable } from '../thenable' describe('queryObserver', () => { let queryClient: QueryClient