From 7c883ebad361bd320d238e39e554a6c2e2f0fae3 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 11 Oct 2024 21:33:24 +0200 Subject: [PATCH 1/4] fix: move thenable-recreation into createResult `updateResult` will only be called after a fetch, but when we switch between caches without a fetch, we will only call `createResult`; this fix stops `data` from the queryResult and the `thenable` to go out-of-sync; it's backwards compatible because `updateResult` also invokes `createResult` --- packages/query-core/src/queryObserver.ts | 45 +++++----- .../src/__tests__/useQuery.test.tsx | 87 +++++++++++++++++++ 2 files changed, 111 insertions(+), 21 deletions(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 5a4db60269..9844a068dd 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -594,27 +594,7 @@ export class QueryObserver< promise: this.#currentThenable, } - return result as QueryObserverResult - } - - updateResult(notifyOptions?: NotifyOptions): void { - const prevResult = this.#currentResult as - | QueryObserverResult - | undefined - - const nextResult = this.createResult(this.#currentQuery, this.options) - - this.#currentResultState = this.#currentQuery.state - this.#currentResultOptions = this.options - - if (this.#currentResultState.data !== undefined) { - this.#lastQueryWithDefinedData = this.#currentQuery - } - - // Only notify and update result if something has changed - if (shallowEqualObjects(nextResult, prevResult)) { - return - } + const nextResult = result as QueryObserverResult if (this.options.experimental_prefetchInRender) { const finalizeThenableIfPossible = (thenable: PendingThenable) => { @@ -648,6 +628,7 @@ export class QueryObserver< nextResult.status === 'error' || nextResult.data !== prevThenable.value ) { + console.log('recreating thenable') recreateThenable() } break @@ -662,6 +643,28 @@ export class QueryObserver< } } + return nextResult + } + + updateResult(notifyOptions?: NotifyOptions): void { + const prevResult = this.#currentResult as + | QueryObserverResult + | undefined + + const nextResult = this.createResult(this.#currentQuery, this.options) + + this.#currentResultState = this.#currentQuery.state + this.#currentResultOptions = this.options + + if (this.#currentResultState.data !== undefined) { + this.#lastQueryWithDefinedData = this.#currentQuery + } + + // Only notify and update result if something has changed + if (shallowEqualObjects(nextResult, prevResult)) { + return + } + this.#currentResult = nextResult // Determine which callbacks to trigger diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 491970764c..6a60386872 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -7385,5 +7385,92 @@ describe('useQuery', () => { fireEvent.click(rendered.getByText('enable')) await waitFor(() => rendered.getByText('test1')) }) + + it('should show correct data when read from cache only (staleTime)', async () => { + const key = queryKey() + let suspenseRenderCount = 0 + queryClient.setQueryData(key, 'initial') + + function MyComponent(props: { promise: Promise }) { + const data = React.use(props.promise) + + return <>{data} + } + + function Loading() { + suspenseRenderCount++ + return <>loading.. + } + function Page() { + const query = useQuery({ + queryKey: key, + queryFn: async () => { + await sleep(1) + return 'test' + }, + staleTime: Infinity, + }) + + return ( + }> + + + ) + } + + const rendered = renderWithClient(queryClient, ) + await waitFor(() => rendered.getByText('initial')) + + expect(suspenseRenderCount).toBe(0) + }) + + it('should show correct data when switching between cache entries without re-fetches', async () => { + const key = queryKey() + + function MyComponent(props: { promise: Promise }) { + const data = React.use(props.promise) + + return <>{data} + } + + function Loading() { + return <>loading.. + } + function Page() { + const [count, setCount] = React.useState(0) + const query = useQuery({ + queryKey: [key, count], + queryFn: async () => { + await sleep(10) + return 'test' + count + }, + staleTime: Infinity, + }) + + return ( +
+ }> + + + + +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + await waitFor(() => rendered.getByText('loading..')) + await waitFor(() => rendered.getByText('test0')) + + fireEvent.click(rendered.getByText('inc')) + await waitFor(() => rendered.getByText('loading..')) + + await waitFor(() => rendered.getByText('test1')) + + console.log('---------dec------------') + fireEvent.click(rendered.getByText('dec')) + + await waitFor(() => rendered.getByText('test0')) + }) }) }) From 388bc256e011ad517fe851d632c7684e38b9cc03 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 11 Oct 2024 21:36:38 +0200 Subject: [PATCH 2/4] oops --- packages/query-core/src/queryObserver.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 9844a068dd..eecf792b41 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -628,7 +628,6 @@ export class QueryObserver< nextResult.status === 'error' || nextResult.data !== prevThenable.value ) { - console.log('recreating thenable') recreateThenable() } break From 8e4a340ba8205ca61fd3615df15569b9177a1fc8 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sat, 12 Oct 2024 09:25:43 +0200 Subject: [PATCH 3/4] test: I'm sick of this flaky test --- .../src/__tests__/useMutationState.test.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/react-query/src/__tests__/useMutationState.test.tsx b/packages/react-query/src/__tests__/useMutationState.test.tsx index 32fb793cc0..f35206525c 100644 --- a/packages/react-query/src/__tests__/useMutationState.test.tsx +++ b/packages/react-query/src/__tests__/useMutationState.test.tsx @@ -59,7 +59,19 @@ describe('useIsMutating', () => { fireEvent.click(rendered.getByRole('button', { name: /mutate1/i })) await sleep(10) fireEvent.click(rendered.getByRole('button', { name: /mutate2/i })) - await waitFor(() => expect(isMutatingArray).toEqual([0, 1, 2, 1, 0])) + + // we don't really care if this yields + // [ +0, 1, 2, +0 ] + // or + // [ +0, 1, 2, 1, +0 ] + // our batching strategy might yield different results + + await waitFor(() => expect(isMutatingArray[0]).toEqual(0)) + await waitFor(() => expect(isMutatingArray[1]).toEqual(1)) + await waitFor(() => expect(isMutatingArray[2]).toEqual(2)) + await waitFor(() => + expect(isMutatingArray[isMutatingArray.length - 1]).toEqual(0), + ) }) it('should filter correctly by mutationKey', async () => { From c48a77f4328289175211d214886f29d6780310ec Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sat, 12 Oct 2024 09:27:03 +0200 Subject: [PATCH 4/4] chore: eslint reports an unused type assertion here --- packages/vue-query/src/useMutationState.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/vue-query/src/useMutationState.ts b/packages/vue-query/src/useMutationState.ts index 465c123ca0..110f076d47 100644 --- a/packages/vue-query/src/useMutationState.ts +++ b/packages/vue-query/src/useMutationState.ts @@ -71,12 +71,9 @@ export function useMutationState( ): Readonly>> { const filters = computed(() => cloneDeepUnref(options.filters)) const mutationCache = (queryClient || useQueryClient()).getMutationCache() - const state = shallowRef(getResult(mutationCache, options)) as Ref< - Array - > + const state = shallowRef(getResult(mutationCache, options)) const unsubscribe = mutationCache.subscribe(() => { - const result = getResult(mutationCache, options) - state.value = result + state.value = getResult(mutationCache, options) }) watch(filters, () => {