From 3c31124ddb1de7900d07d7a56884b8c2324b1e52 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 5 Apr 2024 19:56:40 +0200 Subject: [PATCH] feat(core): memoize result of combine (#7233) * feat(core): memoize result of combine this PR makes sure to only re-run the combine function if the function itself changed referentially or any of the inputs changed * docs: combine memoization * Update docs/framework/react/reference/useQueries.md --- docs/framework/react/reference/useQueries.md | 9 + packages/query-core/src/queriesObserver.ts | 17 +- .../src/__tests__/useQueries.test.tsx | 154 +++++++++++++++++- 3 files changed, 178 insertions(+), 2 deletions(-) diff --git a/docs/framework/react/reference/useQueries.md b/docs/framework/react/reference/useQueries.md index 06443499ff..f6d6e3178a 100644 --- a/docs/framework/react/reference/useQueries.md +++ b/docs/framework/react/reference/useQueries.md @@ -56,3 +56,12 @@ const combinedQueries = useQueries({ ``` In the above example, `combinedQueries` will be an object with a `data` and a `pending` property. Note that all other properties of the Query results will be lost. + +### Memoization + +The `combine` function will only re-run if: + +- the `combine` function itself changed referentially +- any of the query results changed + +This means that an inlined `combine` function, as shown above, will run on every render. To avoid this, you can wrap the `combine` function in `useCallback`, or extract it so a stable function reference if it doesn't have any dependencies. diff --git a/packages/query-core/src/queriesObserver.ts b/packages/query-core/src/queriesObserver.ts index 37f8a5539a..8bfb33e954 100644 --- a/packages/query-core/src/queriesObserver.ts +++ b/packages/query-core/src/queriesObserver.ts @@ -40,6 +40,8 @@ export class QueriesObserver< #queries: Array #observers: Array #combinedResult?: TCombinedResult + #lastCombine?: CombineFn + #lastResult?: Array constructor( client: QueryClient, @@ -181,7 +183,20 @@ export class QueriesObserver< combine: CombineFn | undefined, ): TCombinedResult { if (combine) { - return replaceEqualDeep(this.#combinedResult, combine(input)) + if ( + !this.#combinedResult || + this.#result !== this.#lastResult || + combine !== this.#lastCombine + ) { + this.#lastCombine = combine + this.#lastResult = this.#result + this.#combinedResult = replaceEqualDeep( + this.#combinedResult, + combine(input), + ) + } + + return this.#combinedResult } return input as any } diff --git a/packages/react-query/src/__tests__/useQueries.test.tsx b/packages/react-query/src/__tests__/useQueries.test.tsx index 98957ea8b6..757a1fca71 100644 --- a/packages/react-query/src/__tests__/useQueries.test.tsx +++ b/packages/react-query/src/__tests__/useQueries.test.tsx @@ -2,11 +2,13 @@ import { describe, expect, expectTypeOf, it, vi } from 'vitest' import { fireEvent, render, waitFor } from '@testing-library/react' import * as React from 'react' import { ErrorBoundary } from 'react-error-boundary' +import { QueryClient } from '@tanstack/query-core' import { QueryCache, queryOptions, skipToken, useQueries } from '..' import { createQueryClient, queryKey, renderWithClient, sleep } from './utils' import type { QueryFunction, QueryKey, + QueryObserverResult, UseQueryOptions, UseQueryResult, } from '..' @@ -975,7 +977,7 @@ describe('useQueries', () => { ) }) - it.skip('should not return new instances when called without queries', async () => { + it('should not return new instances when called without queries', async () => { const key = queryKey() const ids: Array = [] let resultChanged = 0 @@ -1278,4 +1280,154 @@ describe('useQueries', () => { await waitFor(() => rendered.getByText('data: 1 result')) }) + + it('should optimize combine if it is a stable reference', async () => { + const key1 = queryKey() + const key2 = queryKey() + + const client = new QueryClient() + + const spy = vi.fn() + let value = 0 + + function Page() { + const [state, setState] = React.useState(0) + const queries = useQueries( + { + queries: [ + { + queryKey: key1, + queryFn: async () => { + await sleep(10) + return 'first result:' + value + }, + }, + { + queryKey: key2, + queryFn: async () => { + await sleep(20) + return 'second result:' + value + }, + }, + ], + combine: React.useCallback((results: Array) => { + const result = { + combined: true, + res: results.map((res) => res.data).join(','), + } + spy(result) + return result + }, []), + }, + client, + ) + + return ( +
+
+ data: {String(queries.combined)} {queries.res} +
+ +
+ ) + } + + const rendered = render() + + await waitFor(() => + rendered.getByText('data: true first result:0,second result:0'), + ) + + // both pending, one pending, both resolved + expect(spy).toHaveBeenCalledTimes(3) + + await client.refetchQueries() + // no increase because result hasn't changed + expect(spy).toHaveBeenCalledTimes(3) + + fireEvent.click(rendered.getByRole('button', { name: /rerender/i })) + + // no increase because just a re-render + expect(spy).toHaveBeenCalledTimes(3) + + value = 1 + + await client.refetchQueries() + + await waitFor(() => + rendered.getByText('data: true first result:1,second result:1'), + ) + + // two value changes = two re-renders + expect(spy).toHaveBeenCalledTimes(5) + }) + + it('should re-run combine if the functional reference changes', async () => { + const key1 = queryKey() + const key2 = queryKey() + + const client = new QueryClient() + + const spy = vi.fn() + + function Page() { + const [state, setState] = React.useState(0) + const queries = useQueries( + { + queries: [ + { + queryKey: [key1], + queryFn: async () => { + await sleep(10) + return 'first result' + }, + }, + { + queryKey: [key2], + queryFn: async () => { + await sleep(20) + return 'second result' + }, + }, + ], + combine: React.useCallback( + (results: Array) => { + const result = { + combined: true, + state, + res: results.map((res) => res.data).join(','), + } + spy(result) + return result + }, + [state], + ), + }, + client, + ) + + return ( +
+
+ data: {String(queries.state)} {queries.res} +
+ +
+ ) + } + + const rendered = render() + + await waitFor(() => + rendered.getByText('data: 0 first result,second result'), + ) + + // both pending, one pending, both resolved + expect(spy).toHaveBeenCalledTimes(3) + + fireEvent.click(rendered.getByRole('button', { name: /rerender/i })) + + // state changed, re-run combine + expect(spy).toHaveBeenCalledTimes(4) + }) })