From 853ab3bd17eca3f2dd3f44cf85e13545bc8ad5c2 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sun, 23 Jan 2022 19:54:26 +0100 Subject: [PATCH 01/15] refactor(QueryClient): add dev warning Warn when several query defaults match a given key. Could be error prone if the returned defaults are not the expected ones. The order of registration does matter. --- src/core/queryClient.ts | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index 7e2fbc02ab..d0af84ac4c 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -532,13 +532,35 @@ export class QueryClient { } } + findQueryDefaults( + queryKey?: QueryKey + ): QueryOptions | undefined { + if (!queryKey) { + return undefined + } + + // First retrieve all matching defaults for the given key + const matchingDefaults = this.queryDefaults.filter(x => + partialMatchKey(queryKey, x.queryKey) + ) + // It is ok not having defaults, but it is error prone to have more than 1 default for a given key + if (process.env.NODE_ENV !== 'production' && matchingDefaults?.length > 1) { + console.warn( + `[QueryClient] Several defaults match with key '${JSON.stringify( + queryKey + )}'. The first matching query options are used. Please check how query defaults are registered. Order does matter here. cf. http://react-query.com/some/link/to/document#queryDefaults.` + ) + } + // Explicitly returns the first one + const firstMatchingDefaults = matchingDefaults?.[0] + return firstMatchingDefaults?.defaultOptions || undefined + } + getQueryDefaults( queryKey?: QueryKey ): QueryObserverOptions | undefined { - return queryKey - ? this.queryDefaults.find(x => partialMatchKey(queryKey, x.queryKey)) - ?.defaultOptions - : undefined + const queryDefaults = this.findQueryDefaults(queryKey) + return queryDefaults } setMutationDefaults( From 1e386c3a2f5e642a6e715dde6e44070fcbd2b007 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sun, 23 Jan 2022 19:55:21 +0100 Subject: [PATCH 02/15] test(QueryClient): warning with defaults options Highlight how query defaults registration order matters. --- src/core/tests/queryClient.test.tsx | 65 +++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx index 0331415032..283fb1d6ff 100644 --- a/src/core/tests/queryClient.test.tsx +++ b/src/core/tests/queryClient.test.tsx @@ -14,6 +14,7 @@ import { QueryCache, QueryClient, QueryFunction, + QueryFunctionContext, QueryObserver, } from '../..' import { focusManager, onlineManager } from '..' @@ -130,6 +131,70 @@ describe('queryClient', () => { queryClient.setQueryDefaults(key, queryOptions2) expect(queryClient.getQueryDefaults(key)).toMatchObject(queryOptions2) }) + + test('should warn in dev if several query defaults match a given key', () => { + // Check discussion here: https://github.com/tannerlinsley/react-query/discussions/3199 + const consoleWarnMock = jest.spyOn(console, 'warn') + consoleWarnMock.mockImplementation(() => true) + + const keyABCD = [ + { + a: 'a', + b: 'b', + c: 'c', + d: 'd', + }, + ] + + // The key below "contains" keyABCD => it is more generic + const keyABC = [ + { + a: 'a', + b: 'b', + c: 'c', + }, + ] + + // The defaults for query matching key "ABCD" (least generic) + const defaultsOfABCD = { + queryFn: function ABCDQueryFn() { + return 'ABCD' + }, + } + + // The defaults for query matching key "ABC" (most generic) + const defaultsOfABC = { + queryFn: function ABCQueryFn() { + return 'ABC' + }, + } + + // No defaults, no warning + const noDefaults = queryClient.getQueryDefaults(keyABCD) + expect(noDefaults).toBeUndefined() + expect(consoleWarnMock).not.toHaveBeenCalled() + + // If defaults for key ABCD are registered **before** the ones of key ABC (more generic)… + queryClient.setQueryDefaults(keyABCD, defaultsOfABCD) + queryClient.setQueryDefaults(keyABC, defaultsOfABC) + // … then the "good" defaults are retrieved: we get the ones for key "ABCD" + const goodDefaults = queryClient.getQueryDefaults(keyABCD) + expect(goodDefaults).toBe(defaultsOfABCD) + expect(consoleWarnMock).toHaveBeenCalledTimes(1) + + // Let's reset the defaults query options and change the order of registration + queryClient.queryDefaults.length = 0 + // The defaults for key ABC (more generic) are registered **before** the ones of key ABCD… + queryClient.setQueryDefaults(keyABC, defaultsOfABC) + queryClient.setQueryDefaults(keyABCD, defaultsOfABCD) + // … then the "wrong" defaults are retrieved: we get the ones for key "ABC" + const badDefaults = queryClient.getQueryDefaults(keyABCD) + expect(badDefaults).not.toBe(defaultsOfABCD) + expect(badDefaults).toBe(defaultsOfABC) + expect(consoleWarnMock).toHaveBeenCalledTimes(2) + + consoleWarnMock.mockRestore() + }) }) describe('setQueryData', () => { From fa4b4826214e43329bf3da6ecedbec09f473408b Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sun, 23 Jan 2022 20:02:19 +0100 Subject: [PATCH 03/15] doc(QueryClient): add notes about query defaults In `getQueryDefaults`, the **first** matching default is returned. In `setQueryDefaults`, highlight how the registration order is important. --- docs/src/pages/reference/QueryClient.md | 57 ++++++++++++++----------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/docs/src/pages/reference/QueryClient.md b/docs/src/pages/reference/QueryClient.md index b195e050d0..28f2d43f82 100644 --- a/docs/src/pages/reference/QueryClient.md +++ b/docs/src/pages/reference/QueryClient.md @@ -23,31 +23,32 @@ await queryClient.prefetchQuery(['posts'], fetchPosts) Its available methods are: -- [`queryClient.fetchQuery`](#queryclientfetchquery) -- [`queryClient.fetchInfiniteQuery`](#queryclientfetchinfinitequery) -- [`queryClient.prefetchQuery`](#queryclientprefetchquery) -- [`queryClient.prefetchInfiniteQuery`](#queryclientprefetchinfinitequery) -- [`queryClient.getQueryData`](#queryclientgetquerydata) -- [`queryClient.getQueriesData`](#queryclientgetqueriesdata) -- [`queryClient.setQueryData`](#queryclientsetquerydata) -- [`queryClient.getQueryState`](#queryclientgetquerystate) -- [`queryClient.setQueriesData`](#queryclientsetqueriesdata) -- [`queryClient.invalidateQueries`](#queryclientinvalidatequeries) -- [`queryClient.refetchQueries`](#queryclientrefetchqueries) -- [`queryClient.cancelQueries`](#queryclientcancelqueries) -- [`queryClient.removeQueries`](#queryclientremovequeries) -- [`queryClient.resetQueries`](#queryclientresetqueries) -- [`queryClient.isFetching`](#queryclientisfetching) -- [`queryClient.isMutating`](#queryclientismutating) -- [`queryClient.getDefaultOptions`](#queryclientgetdefaultoptions) -- [`queryClient.setDefaultOptions`](#queryclientsetdefaultoptions) -- [`queryClient.getQueryDefaults`](#queryclientgetquerydefaults) -- [`queryClient.setQueryDefaults`](#queryclientsetquerydefaults) -- [`queryClient.getMutationDefaults`](#queryclientgetmutationdefaults) -- [`queryClient.setMutationDefaults`](#queryclientsetmutationdefaults) -- [`queryClient.getQueryCache`](#queryclientgetquerycache) -- [`queryClient.getMutationCache`](#queryclientgetmutationcache) -- [`queryClient.clear`](#queryclientclear) +* [`QueryClient`](#queryclient) +* [`queryClient.fetchQuery`](#queryclientfetchquery) +* [`queryClient.fetchInfiniteQuery`](#queryclientfetchinfinitequery) +* [`queryClient.prefetchQuery`](#queryclientprefetchquery) +* [`queryClient.prefetchInfiniteQuery`](#queryclientprefetchinfinitequery) +* [`queryClient.getQueryData`](#queryclientgetquerydata) +* [`queryClient.getQueriesData`](#queryclientgetqueriesdata) +* [`queryClient.setQueryData`](#queryclientsetquerydata) +* [`queryClient.getQueryState`](#queryclientgetquerystate) +* [`queryClient.setQueriesData`](#queryclientsetqueriesdata) +* [`queryClient.invalidateQueries`](#queryclientinvalidatequeries) +* [`queryClient.refetchQueries`](#queryclientrefetchqueries) +* [`queryClient.cancelQueries`](#queryclientcancelqueries) +* [`queryClient.removeQueries`](#queryclientremovequeries) +* [`queryClient.resetQueries`](#queryclientresetqueries) +* [`queryClient.isFetching`](#queryclientisfetching) +* [`queryClient.isMutating`](#queryclientismutating) +* [`queryClient.getDefaultOptions`](#queryclientgetdefaultoptions) +* [`queryClient.setDefaultOptions`](#queryclientsetdefaultoptions) +* [`queryClient.getQueryDefaults`](#queryclientgetquerydefaults) +* [`queryClient.setQueryDefaults`](#queryclientsetquerydefaults) +* [`queryClient.getMutationDefaults`](#queryclientgetmutationdefaults) +* [`queryClient.setMutationDefaults`](#queryclientsetmutationdefaults) +* [`queryClient.getQueryCache`](#queryclientgetquerycache) +* [`queryClient.getMutationCache`](#queryclientgetmutationcache) +* [`queryClient.clear`](#queryclientclear) **Options** @@ -474,6 +475,9 @@ The `getQueryDefaults` method returns the default options which have been set fo const defaultOptions = queryClient.getQueryDefaults(['posts']) ``` +> Note that if several query defaults match the given query key, the **first** matching one is returned. +> This could lead to unexpected behaviours. See [`setquerydefaults`](#queryclientsetquerydefaults). + ## `queryClient.setQueryDefaults` `setQueryDefaults` can be used to set default options for specific queries: @@ -491,6 +495,9 @@ function Component() { - `queryKey: QueryKey`: [Query Keys](../guides/query-keys) - `options: QueryOptions` +> As stated in [`getquerydefaults`](#queryclientgetquerydefaults), the order of registration of query defaults does matter. +> Since the **first** matching defaults are returned by `getquerydefaults`, the registration should be made in the following order: from the **least generic key** to the **most generic one**. This way, in case of specific key, the first matching one would be the expected one. + ## `queryClient.getMutationDefaults` The `getMutationDefaults` method returns the default options which have been set for specific mutations: From 8c43779317ffb804071b79e53afc000fad6bcb08 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sat, 29 Jan 2022 14:24:10 +0100 Subject: [PATCH 04/15] doc(QueryClient): fix link to documentation --- docs/src/pages/reference/QueryClient.md | 6 +++--- src/core/queryClient.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/src/pages/reference/QueryClient.md b/docs/src/pages/reference/QueryClient.md index 28f2d43f82..84732e7211 100644 --- a/docs/src/pages/reference/QueryClient.md +++ b/docs/src/pages/reference/QueryClient.md @@ -476,7 +476,7 @@ const defaultOptions = queryClient.getQueryDefaults(['posts']) ``` > Note that if several query defaults match the given query key, the **first** matching one is returned. -> This could lead to unexpected behaviours. See [`setquerydefaults`](#queryclientsetquerydefaults). +> This could lead to unexpected behaviours. See [`setQueryDefaults`](#queryclientsetquerydefaults). ## `queryClient.setQueryDefaults` @@ -495,8 +495,8 @@ function Component() { - `queryKey: QueryKey`: [Query Keys](../guides/query-keys) - `options: QueryOptions` -> As stated in [`getquerydefaults`](#queryclientgetquerydefaults), the order of registration of query defaults does matter. -> Since the **first** matching defaults are returned by `getquerydefaults`, the registration should be made in the following order: from the **least generic key** to the **most generic one**. This way, in case of specific key, the first matching one would be the expected one. +> As stated in [`getQueryDefaults`](#queryclientgetquerydefaults), the order of registration of query defaults does matter. +> Since the **first** matching defaults are returned by `getQueryDefaults`, the registration should be made in the following order: from the **least generic key** to the **most generic one**. This way, in case of specific key, the first matching one would be the expected one. ## `queryClient.getMutationDefaults` diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index d0af84ac4c..ef5f704c9e 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -532,7 +532,7 @@ export class QueryClient { } } - findQueryDefaults( + findFirstMatchingQueryDefaults( queryKey?: QueryKey ): QueryOptions | undefined { if (!queryKey) { @@ -548,7 +548,7 @@ export class QueryClient { console.warn( `[QueryClient] Several defaults match with key '${JSON.stringify( queryKey - )}'. The first matching query options are used. Please check how query defaults are registered. Order does matter here. cf. http://react-query.com/some/link/to/document#queryDefaults.` + )}'. The first matching query defaults are used. Please check how query defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetquerydefaults.` ) } // Explicitly returns the first one @@ -559,7 +559,7 @@ export class QueryClient { getQueryDefaults( queryKey?: QueryKey ): QueryObserverOptions | undefined { - const queryDefaults = this.findQueryDefaults(queryKey) + const queryDefaults = this.findFirstMatchingQueryDefaults(queryKey) return queryDefaults } From bd3b946cb82265a48ae70c4b35f1c225dacc38b6 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Mon, 31 Jan 2022 12:37:48 +0100 Subject: [PATCH 05/15] test(QueryClient): better test --- src/core/tests/queryClient.test.tsx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx index 283fb1d6ff..0758bada28 100644 --- a/src/core/tests/queryClient.test.tsx +++ b/src/core/tests/queryClient.test.tsx @@ -14,7 +14,6 @@ import { QueryCache, QueryClient, QueryFunction, - QueryFunctionContext, QueryObserver, } from '../..' import { focusManager, onlineManager } from '..' @@ -182,13 +181,13 @@ describe('queryClient', () => { expect(goodDefaults).toBe(defaultsOfABCD) expect(consoleWarnMock).toHaveBeenCalledTimes(1) - // Let's reset the defaults query options and change the order of registration - queryClient.queryDefaults.length = 0 + // Let's create another queryClient and change the order of registration + const newQueryClient = new QueryClient() // The defaults for key ABC (more generic) are registered **before** the ones of key ABCD… - queryClient.setQueryDefaults(keyABC, defaultsOfABC) - queryClient.setQueryDefaults(keyABCD, defaultsOfABCD) + newQueryClient.setQueryDefaults(keyABC, defaultsOfABC) + newQueryClient.setQueryDefaults(keyABCD, defaultsOfABCD) // … then the "wrong" defaults are retrieved: we get the ones for key "ABC" - const badDefaults = queryClient.getQueryDefaults(keyABCD) + const badDefaults = newQueryClient.getQueryDefaults(keyABCD) expect(badDefaults).not.toBe(defaultsOfABCD) expect(badDefaults).toBe(defaultsOfABC) expect(consoleWarnMock).toHaveBeenCalledTimes(2) From 37ff6f8d0c464995d40857f9b37f8073bf2cac5a Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Mon, 31 Jan 2022 12:38:04 +0100 Subject: [PATCH 06/15] refactor(QueryClient): use internal logger --- src/core/queryClient.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index ef5f704c9e..27033b5eb0 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -38,6 +38,7 @@ import { onlineManager } from './onlineManager' import { notifyManager } from './notifyManager' import { infiniteQueryBehavior } from './infiniteQueryBehavior' import { CancelOptions, DefaultedQueryObserverOptions } from './types' +import { getLogger } from './logger' // TYPES @@ -545,7 +546,7 @@ export class QueryClient { ) // It is ok not having defaults, but it is error prone to have more than 1 default for a given key if (process.env.NODE_ENV !== 'production' && matchingDefaults?.length > 1) { - console.warn( + getLogger().warn( `[QueryClient] Several defaults match with key '${JSON.stringify( queryKey )}'. The first matching query defaults are used. Please check how query defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetquerydefaults.` From 9bbe2a8212c65c048840572a9a9c1e54891b69cc Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Mon, 31 Jan 2022 12:48:21 +0100 Subject: [PATCH 07/15] doc(QueryClient): fix markup --- docs/src/pages/reference/QueryClient.md | 52 ++++++++++++------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/docs/src/pages/reference/QueryClient.md b/docs/src/pages/reference/QueryClient.md index 84732e7211..810544b5d7 100644 --- a/docs/src/pages/reference/QueryClient.md +++ b/docs/src/pages/reference/QueryClient.md @@ -23,32 +23,32 @@ await queryClient.prefetchQuery(['posts'], fetchPosts) Its available methods are: -* [`QueryClient`](#queryclient) -* [`queryClient.fetchQuery`](#queryclientfetchquery) -* [`queryClient.fetchInfiniteQuery`](#queryclientfetchinfinitequery) -* [`queryClient.prefetchQuery`](#queryclientprefetchquery) -* [`queryClient.prefetchInfiniteQuery`](#queryclientprefetchinfinitequery) -* [`queryClient.getQueryData`](#queryclientgetquerydata) -* [`queryClient.getQueriesData`](#queryclientgetqueriesdata) -* [`queryClient.setQueryData`](#queryclientsetquerydata) -* [`queryClient.getQueryState`](#queryclientgetquerystate) -* [`queryClient.setQueriesData`](#queryclientsetqueriesdata) -* [`queryClient.invalidateQueries`](#queryclientinvalidatequeries) -* [`queryClient.refetchQueries`](#queryclientrefetchqueries) -* [`queryClient.cancelQueries`](#queryclientcancelqueries) -* [`queryClient.removeQueries`](#queryclientremovequeries) -* [`queryClient.resetQueries`](#queryclientresetqueries) -* [`queryClient.isFetching`](#queryclientisfetching) -* [`queryClient.isMutating`](#queryclientismutating) -* [`queryClient.getDefaultOptions`](#queryclientgetdefaultoptions) -* [`queryClient.setDefaultOptions`](#queryclientsetdefaultoptions) -* [`queryClient.getQueryDefaults`](#queryclientgetquerydefaults) -* [`queryClient.setQueryDefaults`](#queryclientsetquerydefaults) -* [`queryClient.getMutationDefaults`](#queryclientgetmutationdefaults) -* [`queryClient.setMutationDefaults`](#queryclientsetmutationdefaults) -* [`queryClient.getQueryCache`](#queryclientgetquerycache) -* [`queryClient.getMutationCache`](#queryclientgetmutationcache) -* [`queryClient.clear`](#queryclientclear) +- [`QueryClient`](#queryclient) +- [`queryClient.fetchQuery`](#queryclientfetchquery) +- [`queryClient.fetchInfiniteQuery`](#queryclientfetchinfinitequery) +- [`queryClient.prefetchQuery`](#queryclientprefetchquery) +- [`queryClient.prefetchInfiniteQuery`](#queryclientprefetchinfinitequery) +- [`queryClient.getQueryData`](#queryclientgetquerydata) +- [`queryClient.getQueriesData`](#queryclientgetqueriesdata) +- [`queryClient.setQueryData`](#queryclientsetquerydata) +- [`queryClient.getQueryState`](#queryclientgetquerystate) +- [`queryClient.setQueriesData`](#queryclientsetqueriesdata) +- [`queryClient.invalidateQueries`](#queryclientinvalidatequeries) +- [`queryClient.refetchQueries`](#queryclientrefetchqueries) +- [`queryClient.cancelQueries`](#queryclientcancelqueries) +- [`queryClient.removeQueries`](#queryclientremovequeries) +- [`queryClient.resetQueries`](#queryclientresetqueries) +- [`queryClient.isFetching`](#queryclientisfetching) +- [`queryClient.isMutating`](#queryclientismutating) +- [`queryClient.getDefaultOptions`](#queryclientgetdefaultoptions) +- [`queryClient.setDefaultOptions`](#queryclientsetdefaultoptions) +- [`queryClient.getQueryDefaults`](#queryclientgetquerydefaults) +- [`queryClient.setQueryDefaults`](#queryclientsetquerydefaults) +- [`queryClient.getMutationDefaults`](#queryclientgetmutationdefaults) +- [`queryClient.setMutationDefaults`](#queryclientsetmutationdefaults) +- [`queryClient.getQueryCache`](#queryclientgetquerycache) +- [`queryClient.getMutationCache`](#queryclientgetmutationcache) +- [`queryClient.clear`](#queryclientclear) **Options** From 137466bb6bf7a710fed65efff792d9df20025468 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Mon, 31 Jan 2022 12:49:47 +0100 Subject: [PATCH 08/15] doc(QueryClient): remove extra entry --- docs/src/pages/reference/QueryClient.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/src/pages/reference/QueryClient.md b/docs/src/pages/reference/QueryClient.md index 810544b5d7..84c1576ef1 100644 --- a/docs/src/pages/reference/QueryClient.md +++ b/docs/src/pages/reference/QueryClient.md @@ -23,7 +23,6 @@ await queryClient.prefetchQuery(['posts'], fetchPosts) Its available methods are: -- [`QueryClient`](#queryclient) - [`queryClient.fetchQuery`](#queryclientfetchquery) - [`queryClient.fetchInfiniteQuery`](#queryclientfetchinfinitequery) - [`queryClient.prefetchQuery`](#queryclientprefetchquery) From 78501eeec2ffa80503a34a47159d6cfd1f35b38c Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Tue, 1 Feb 2022 09:12:58 +0100 Subject: [PATCH 09/15] refacto(QueryClient): warn about several query defaults Warning must be displayed any time a conflict is detected, not just for dev build. The warning is aimed at helping developers *using* react-query, not those *developping* react-query. --- src/core/queryClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index 27033b5eb0..604b39e3dd 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -545,7 +545,7 @@ export class QueryClient { partialMatchKey(queryKey, x.queryKey) ) // It is ok not having defaults, but it is error prone to have more than 1 default for a given key - if (process.env.NODE_ENV !== 'production' && matchingDefaults?.length > 1) { + if (matchingDefaults?.length > 1) { getLogger().warn( `[QueryClient] Several defaults match with key '${JSON.stringify( queryKey From 58684a7e703476a399deea9cf9a3021022f5d317 Mon Sep 17 00:00:00 2001 From: GLabat Date: Sat, 5 Feb 2022 12:10:09 +0100 Subject: [PATCH 10/15] Update src/core/queryClient.ts Remove useless optional chaining. Co-authored-by: Dominik Dorfmeister --- src/core/queryClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index 604b39e3dd..1b64cb1cfe 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -545,7 +545,7 @@ export class QueryClient { partialMatchKey(queryKey, x.queryKey) ) // It is ok not having defaults, but it is error prone to have more than 1 default for a given key - if (matchingDefaults?.length > 1) { + if (matchingDefaults.length > 1) { getLogger().warn( `[QueryClient] Several defaults match with key '${JSON.stringify( queryKey From 05c3fe19cd3277bd99740564c2e771412b7cf109 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sat, 5 Feb 2022 18:44:19 +0100 Subject: [PATCH 11/15] feat(utils): add assert helper --- src/core/tests/utils.test.tsx | 21 +++++++++++++++++++++ src/core/utils.ts | 10 ++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/core/tests/utils.test.tsx b/src/core/tests/utils.test.tsx index af8b618a82..57e9d6ac47 100644 --- a/src/core/tests/utils.test.tsx +++ b/src/core/tests/utils.test.tsx @@ -5,6 +5,7 @@ import { parseMutationArgs, matchMutation, scheduleMicrotask, + assert, } from '../utils' import { QueryClient, QueryCache, setLogger, Logger } from '../..' import { queryKey } from '../../reactjs/tests/utils' @@ -389,4 +390,24 @@ describe('core/utils', () => { jest.useRealTimers() }) }) + + describe('assert', () => { + it('should assert acccording to condition', () => { + const logger: Logger = { + error: jest.fn(), + log: jest.fn(), + warn: jest.fn(), + } + setLogger(logger) + + assert(true, '') + expect(logger.warn).not.toHaveBeenCalled() + assert(false, '') + expect(logger.warn).toHaveBeenCalledTimes(1) + assert(false, '') + expect(logger.warn).toHaveBeenCalledTimes(2) + + setLogger(console) + }) + }) }) diff --git a/src/core/utils.ts b/src/core/utils.ts index 0c4c9b0fcf..c2e9ae59c2 100644 --- a/src/core/utils.ts +++ b/src/core/utils.ts @@ -1,3 +1,4 @@ +import { getLogger } from './logger' import type { Mutation } from './mutation' import type { Query } from './query' import type { @@ -74,6 +75,15 @@ export function noop(): undefined { return undefined } +/** + * Raise a dev warning message when the condition is not fulfilled + */ +export function assert(condition: boolean, message: string): void { + if (process.env.NODE_ENV !== 'production' && !condition) { + getLogger().warn(message) + } +} + export function functionalUpdate( updater: Updater, input: TInput From 7cc609b2a70e8e8748a82f82b6d9bc2c029fba71 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sat, 5 Feb 2022 19:04:48 +0100 Subject: [PATCH 12/15] refactor(QueryClient): add dev warning for mutation defaults --- docs/src/pages/reference/QueryClient.md | 2 + src/core/queryClient.ts | 73 +++++++++++++++---------- src/core/tests/queryClient.test.tsx | 62 +++++++++++++++++++++ 3 files changed, 109 insertions(+), 28 deletions(-) diff --git a/docs/src/pages/reference/QueryClient.md b/docs/src/pages/reference/QueryClient.md index 84c1576ef1..7aad8ff00a 100644 --- a/docs/src/pages/reference/QueryClient.md +++ b/docs/src/pages/reference/QueryClient.md @@ -522,6 +522,8 @@ function Component() { - `mutationKey: string | unknown[]` - `options: MutationOptions` +> Similar to [`setQueryDefaults`](#queryclientsetquerydefaults), the order of registration does matter here. + ## `queryClient.getQueryCache` The `getQueryCache` method returns the query cache this client is connected to. diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index 1b64cb1cfe..224ca9d740 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -8,6 +8,7 @@ import { partialMatchKey, hashQueryKeyByOptions, MutationFilters, + assert, } from './utils' import type { QueryClientConfig, @@ -38,7 +39,6 @@ import { onlineManager } from './onlineManager' import { notifyManager } from './notifyManager' import { infiniteQueryBehavior } from './infiniteQueryBehavior' import { CancelOptions, DefaultedQueryObserverOptions } from './types' -import { getLogger } from './logger' // TYPES @@ -519,20 +519,6 @@ export class QueryClient { this.defaultOptions = options } - setQueryDefaults( - queryKey: QueryKey, - options: QueryObserverOptions - ): void { - const result = this.queryDefaults.find( - x => hashQueryKey(queryKey) === hashQueryKey(x.queryKey) - ) - if (result) { - result.defaultOptions = options - } else { - this.queryDefaults.push({ queryKey, defaultOptions: options }) - } - } - findFirstMatchingQueryDefaults( queryKey?: QueryKey ): QueryOptions | undefined { @@ -545,16 +531,50 @@ export class QueryClient { partialMatchKey(queryKey, x.queryKey) ) // It is ok not having defaults, but it is error prone to have more than 1 default for a given key - if (matchingDefaults.length > 1) { - getLogger().warn( - `[QueryClient] Several defaults match with key '${JSON.stringify( - queryKey - )}'. The first matching query defaults are used. Please check how query defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetquerydefaults.` - ) + assert( + matchingDefaults.length <= 1, + `[QueryClient] Several query defaults match with key '${JSON.stringify( + queryKey + )}'. The first matching query defaults are used. Please check how query defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetquerydefaults.` + ) + // Explicitly returns the first one + return matchingDefaults[0]?.defaultOptions + } + + findFirstMatchingMutationDefaults( + mutationKey?: MutationKey + ): MutationOptions | undefined { + if (!mutationKey) { + return undefined } + + // First retrieve all matching defaults for the given key + const matchingDefaults = this.mutationDefaults.filter(x => + partialMatchKey(mutationKey, x.mutationKey) + ) + // It is ok not having defaults, but it is error prone to have more than 1 default for a given key + assert( + matchingDefaults.length <= 1, + `[QueryClient] Several mutation defaults match with key '${JSON.stringify( + mutationKey + )}'. The first matching mutation defaults are used. Please check how mutation defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetmutationdefaults.` + ) // Explicitly returns the first one - const firstMatchingDefaults = matchingDefaults?.[0] - return firstMatchingDefaults?.defaultOptions || undefined + return matchingDefaults[0]?.defaultOptions + } + + setQueryDefaults( + queryKey: QueryKey, + options: QueryObserverOptions + ): void { + const result = this.queryDefaults.find( + x => hashQueryKey(queryKey) === hashQueryKey(x.queryKey) + ) + if (result) { + result.defaultOptions = options + } else { + this.queryDefaults.push({ queryKey, defaultOptions: options }) + } } getQueryDefaults( @@ -581,11 +601,8 @@ export class QueryClient { getMutationDefaults( mutationKey?: MutationKey ): MutationObserverOptions | undefined { - return mutationKey - ? this.mutationDefaults.find(x => - partialMatchKey(mutationKey, x.mutationKey) - )?.defaultOptions - : undefined + const mutationDefaults = this.findFirstMatchingMutationDefaults(mutationKey) + return mutationDefaults } defaultQueryOptions< diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx index 0758bada28..2d786c0edf 100644 --- a/src/core/tests/queryClient.test.tsx +++ b/src/core/tests/queryClient.test.tsx @@ -179,6 +179,7 @@ describe('queryClient', () => { // … then the "good" defaults are retrieved: we get the ones for key "ABCD" const goodDefaults = queryClient.getQueryDefaults(keyABCD) expect(goodDefaults).toBe(defaultsOfABCD) + // The warning is still raised since several defaults are matching expect(consoleWarnMock).toHaveBeenCalledTimes(1) // Let's create another queryClient and change the order of registration @@ -194,6 +195,67 @@ describe('queryClient', () => { consoleWarnMock.mockRestore() }) + + test('should warn in dev if several mutation defaults match a given key', () => { + // Check discussion here: https://github.com/tannerlinsley/react-query/discussions/3199 + const consoleWarnMock = jest.spyOn(console, 'warn') + consoleWarnMock.mockImplementation(() => true) + + const keyABCD = [ + { + a: 'a', + b: 'b', + c: 'c', + d: 'd', + }, + ] + + // The key below "contains" keyABCD => it is more generic + const keyABC = [ + { + a: 'a', + b: 'b', + c: 'c', + }, + ] + + // The defaults for mutation matching key "ABCD" (least generic) + const defaultsOfABCD = { + mutationFn: Promise.resolve, + } + + // The defaults for mutation matching key "ABC" (most generic) + const defaultsOfABC = { + mutationFn: Promise.resolve, + } + + // No defaults, no warning + const noDefaults = queryClient.getMutationDefaults(keyABCD) + expect(noDefaults).toBeUndefined() + expect(consoleWarnMock).not.toHaveBeenCalled() + + // If defaults for key ABCD are registered **before** the ones of key ABC (more generic)… + queryClient.setMutationDefaults(keyABCD, defaultsOfABCD) + queryClient.setMutationDefaults(keyABC, defaultsOfABC) + // … then the "good" defaults are retrieved: we get the ones for key "ABCD" + const goodDefaults = queryClient.getMutationDefaults(keyABCD) + expect(goodDefaults).toBe(defaultsOfABCD) + // The warning is still raised since several defaults are matching + expect(consoleWarnMock).toHaveBeenCalledTimes(1) + + // Let's create another queryClient and change the order of registration + const newQueryClient = new QueryClient() + // The defaults for key ABC (more generic) are registered **before** the ones of key ABCD… + newQueryClient.setMutationDefaults(keyABC, defaultsOfABC) + newQueryClient.setMutationDefaults(keyABCD, defaultsOfABCD) + // … then the "wrong" defaults are retrieved: we get the ones for key "ABC" + const badDefaults = newQueryClient.getMutationDefaults(keyABCD) + expect(badDefaults).not.toBe(defaultsOfABCD) + expect(badDefaults).toBe(defaultsOfABC) + expect(consoleWarnMock).toHaveBeenCalledTimes(2) + + consoleWarnMock.mockRestore() + }) }) describe('setQueryData', () => { From 71f9fa5ee1c20a4f2d0249a8c6959acb4270d3a7 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sun, 6 Feb 2022 16:43:32 +0100 Subject: [PATCH 13/15] Revert "feat(utils): add assert helper" This reverts commit 05c3fe19cd3277bd99740564c2e771412b7cf109. --- src/core/tests/utils.test.tsx | 21 --------------------- src/core/utils.ts | 10 ---------- 2 files changed, 31 deletions(-) diff --git a/src/core/tests/utils.test.tsx b/src/core/tests/utils.test.tsx index 57e9d6ac47..af8b618a82 100644 --- a/src/core/tests/utils.test.tsx +++ b/src/core/tests/utils.test.tsx @@ -5,7 +5,6 @@ import { parseMutationArgs, matchMutation, scheduleMicrotask, - assert, } from '../utils' import { QueryClient, QueryCache, setLogger, Logger } from '../..' import { queryKey } from '../../reactjs/tests/utils' @@ -390,24 +389,4 @@ describe('core/utils', () => { jest.useRealTimers() }) }) - - describe('assert', () => { - it('should assert acccording to condition', () => { - const logger: Logger = { - error: jest.fn(), - log: jest.fn(), - warn: jest.fn(), - } - setLogger(logger) - - assert(true, '') - expect(logger.warn).not.toHaveBeenCalled() - assert(false, '') - expect(logger.warn).toHaveBeenCalledTimes(1) - assert(false, '') - expect(logger.warn).toHaveBeenCalledTimes(2) - - setLogger(console) - }) - }) }) diff --git a/src/core/utils.ts b/src/core/utils.ts index c2e9ae59c2..0c4c9b0fcf 100644 --- a/src/core/utils.ts +++ b/src/core/utils.ts @@ -1,4 +1,3 @@ -import { getLogger } from './logger' import type { Mutation } from './mutation' import type { Query } from './query' import type { @@ -75,15 +74,6 @@ export function noop(): undefined { return undefined } -/** - * Raise a dev warning message when the condition is not fulfilled - */ -export function assert(condition: boolean, message: string): void { - if (process.env.NODE_ENV !== 'production' && !condition) { - getLogger().warn(message) - } -} - export function functionalUpdate( updater: Updater, input: TInput From 96d74f871b30704e1ed5e027ff7cd5a083d86399 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sun, 6 Feb 2022 17:03:16 +0100 Subject: [PATCH 14/15] refactor(QueryClient): error when several defaults Review how the check for multiple defaults on a key is raised. Ensure it remains fast in release build. --- src/core/queryClient.ts | 61 +++++++++++++++++++---------- src/core/tests/queryClient.test.tsx | 24 ++++++------ 2 files changed, 52 insertions(+), 33 deletions(-) diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index 224ca9d740..958d342a28 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -8,7 +8,6 @@ import { partialMatchKey, hashQueryKeyByOptions, MutationFilters, - assert, } from './utils' import type { QueryClientConfig, @@ -39,6 +38,7 @@ import { onlineManager } from './onlineManager' import { notifyManager } from './notifyManager' import { infiniteQueryBehavior } from './infiniteQueryBehavior' import { CancelOptions, DefaultedQueryObserverOptions } from './types' +import { getLogger } from './logger' // TYPES @@ -526,19 +526,29 @@ export class QueryClient { return undefined } - // First retrieve all matching defaults for the given key - const matchingDefaults = this.queryDefaults.filter(x => + // Get the first matching defaults + const firstMatchingDefaults = this.queryDefaults.find(x => partialMatchKey(queryKey, x.queryKey) ) - // It is ok not having defaults, but it is error prone to have more than 1 default for a given key - assert( - matchingDefaults.length <= 1, - `[QueryClient] Several query defaults match with key '${JSON.stringify( - queryKey - )}'. The first matching query defaults are used. Please check how query defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetquerydefaults.` - ) + + // Additional checks and error in dev mode + if (process.env.NODE_ENV !== 'production') { + // Retrieve all matching defaults for the given key + const matchingDefaults = this.queryDefaults.filter(x => + partialMatchKey(queryKey, x.queryKey) + ) + // It is ok not having defaults, but it is error prone to have more than 1 default for a given key + if (matchingDefaults.length > 1) { + getLogger().error( + `[QueryClient] Several query defaults match with key '${JSON.stringify( + queryKey + )}'. The first matching query defaults are used. Please check how query defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetquerydefaults.` + ) + } + } + // Explicitly returns the first one - return matchingDefaults[0]?.defaultOptions + return firstMatchingDefaults?.defaultOptions } findFirstMatchingMutationDefaults( @@ -548,19 +558,28 @@ export class QueryClient { return undefined } - // First retrieve all matching defaults for the given key - const matchingDefaults = this.mutationDefaults.filter(x => + // Get the first matching defaults + const firstMatchingDefaults = this.mutationDefaults.find(x => partialMatchKey(mutationKey, x.mutationKey) ) - // It is ok not having defaults, but it is error prone to have more than 1 default for a given key - assert( - matchingDefaults.length <= 1, - `[QueryClient] Several mutation defaults match with key '${JSON.stringify( - mutationKey - )}'. The first matching mutation defaults are used. Please check how mutation defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetmutationdefaults.` - ) + + // Additional checks and error in dev mode + if (process.env.NODE_ENV !== 'production') { + // Retrieve all matching defaults for the given key + const matchingDefaults = this.mutationDefaults.filter(x => + partialMatchKey(mutationKey, x.mutationKey) + ) + // It is ok not having defaults, but it is error prone to have more than 1 default for a given key + if (matchingDefaults.length > 1) { + getLogger().error( + `[QueryClient] Several mutation defaults match with key '${JSON.stringify( + mutationKey + )}'. The first matching mutation defaults are used. Please check how mutation defaults are registered. Order does matter here. cf. https://react-query.tanstack.com/reference/QueryClient#queryclientsetmutationdefaults.` + ) + } + } // Explicitly returns the first one - return matchingDefaults[0]?.defaultOptions + return firstMatchingDefaults?.defaultOptions } setQueryDefaults( diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx index 2d786c0edf..c32cf97cc0 100644 --- a/src/core/tests/queryClient.test.tsx +++ b/src/core/tests/queryClient.test.tsx @@ -133,8 +133,8 @@ describe('queryClient', () => { test('should warn in dev if several query defaults match a given key', () => { // Check discussion here: https://github.com/tannerlinsley/react-query/discussions/3199 - const consoleWarnMock = jest.spyOn(console, 'warn') - consoleWarnMock.mockImplementation(() => true) + const consoleErrorMock = jest.spyOn(console, 'error') + consoleErrorMock.mockImplementation(() => true) const keyABCD = [ { @@ -171,7 +171,7 @@ describe('queryClient', () => { // No defaults, no warning const noDefaults = queryClient.getQueryDefaults(keyABCD) expect(noDefaults).toBeUndefined() - expect(consoleWarnMock).not.toHaveBeenCalled() + expect(consoleErrorMock).not.toHaveBeenCalled() // If defaults for key ABCD are registered **before** the ones of key ABC (more generic)… queryClient.setQueryDefaults(keyABCD, defaultsOfABCD) @@ -180,7 +180,7 @@ describe('queryClient', () => { const goodDefaults = queryClient.getQueryDefaults(keyABCD) expect(goodDefaults).toBe(defaultsOfABCD) // The warning is still raised since several defaults are matching - expect(consoleWarnMock).toHaveBeenCalledTimes(1) + expect(consoleErrorMock).toHaveBeenCalledTimes(1) // Let's create another queryClient and change the order of registration const newQueryClient = new QueryClient() @@ -191,15 +191,15 @@ describe('queryClient', () => { const badDefaults = newQueryClient.getQueryDefaults(keyABCD) expect(badDefaults).not.toBe(defaultsOfABCD) expect(badDefaults).toBe(defaultsOfABC) - expect(consoleWarnMock).toHaveBeenCalledTimes(2) + expect(consoleErrorMock).toHaveBeenCalledTimes(2) - consoleWarnMock.mockRestore() + consoleErrorMock.mockRestore() }) test('should warn in dev if several mutation defaults match a given key', () => { // Check discussion here: https://github.com/tannerlinsley/react-query/discussions/3199 - const consoleWarnMock = jest.spyOn(console, 'warn') - consoleWarnMock.mockImplementation(() => true) + const consoleErrorMock = jest.spyOn(console, 'error') + consoleErrorMock.mockImplementation(() => true) const keyABCD = [ { @@ -232,7 +232,7 @@ describe('queryClient', () => { // No defaults, no warning const noDefaults = queryClient.getMutationDefaults(keyABCD) expect(noDefaults).toBeUndefined() - expect(consoleWarnMock).not.toHaveBeenCalled() + expect(consoleErrorMock).not.toHaveBeenCalled() // If defaults for key ABCD are registered **before** the ones of key ABC (more generic)… queryClient.setMutationDefaults(keyABCD, defaultsOfABCD) @@ -241,7 +241,7 @@ describe('queryClient', () => { const goodDefaults = queryClient.getMutationDefaults(keyABCD) expect(goodDefaults).toBe(defaultsOfABCD) // The warning is still raised since several defaults are matching - expect(consoleWarnMock).toHaveBeenCalledTimes(1) + expect(consoleErrorMock).toHaveBeenCalledTimes(1) // Let's create another queryClient and change the order of registration const newQueryClient = new QueryClient() @@ -252,9 +252,9 @@ describe('queryClient', () => { const badDefaults = newQueryClient.getMutationDefaults(keyABCD) expect(badDefaults).not.toBe(defaultsOfABCD) expect(badDefaults).toBe(defaultsOfABC) - expect(consoleWarnMock).toHaveBeenCalledTimes(2) + expect(consoleErrorMock).toHaveBeenCalledTimes(2) - consoleWarnMock.mockRestore() + consoleErrorMock.mockRestore() }) }) From 1de843c4c9cdee3591d7da0e2b8c97862da16220 Mon Sep 17 00:00:00 2001 From: Guillaume Labat Date: Sun, 6 Feb 2022 18:29:31 +0100 Subject: [PATCH 15/15] refactor(QueryClient): inline code --- src/core/queryClient.ts | 81 +++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 48 deletions(-) diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index 958d342a28..d09461ef83 100644 --- a/src/core/queryClient.ts +++ b/src/core/queryClient.ts @@ -519,9 +519,23 @@ export class QueryClient { this.defaultOptions = options } - findFirstMatchingQueryDefaults( + setQueryDefaults( + queryKey: QueryKey, + options: QueryObserverOptions + ): void { + const result = this.queryDefaults.find( + x => hashQueryKey(queryKey) === hashQueryKey(x.queryKey) + ) + if (result) { + result.defaultOptions = options + } else { + this.queryDefaults.push({ queryKey, defaultOptions: options }) + } + } + + getQueryDefaults( queryKey?: QueryKey - ): QueryOptions | undefined { + ): QueryObserverOptions | undefined { if (!queryKey) { return undefined } @@ -547,13 +561,26 @@ export class QueryClient { } } - // Explicitly returns the first one return firstMatchingDefaults?.defaultOptions } - findFirstMatchingMutationDefaults( + setMutationDefaults( + mutationKey: MutationKey, + options: MutationObserverOptions + ): void { + const result = this.mutationDefaults.find( + x => hashQueryKey(mutationKey) === hashQueryKey(x.mutationKey) + ) + if (result) { + result.defaultOptions = options + } else { + this.mutationDefaults.push({ mutationKey, defaultOptions: options }) + } + } + + getMutationDefaults( mutationKey?: MutationKey - ): MutationOptions | undefined { + ): MutationObserverOptions | undefined { if (!mutationKey) { return undefined } @@ -578,50 +605,8 @@ export class QueryClient { ) } } - // Explicitly returns the first one - return firstMatchingDefaults?.defaultOptions - } - setQueryDefaults( - queryKey: QueryKey, - options: QueryObserverOptions - ): void { - const result = this.queryDefaults.find( - x => hashQueryKey(queryKey) === hashQueryKey(x.queryKey) - ) - if (result) { - result.defaultOptions = options - } else { - this.queryDefaults.push({ queryKey, defaultOptions: options }) - } - } - - getQueryDefaults( - queryKey?: QueryKey - ): QueryObserverOptions | undefined { - const queryDefaults = this.findFirstMatchingQueryDefaults(queryKey) - return queryDefaults - } - - setMutationDefaults( - mutationKey: MutationKey, - options: MutationObserverOptions - ): void { - const result = this.mutationDefaults.find( - x => hashQueryKey(mutationKey) === hashQueryKey(x.mutationKey) - ) - if (result) { - result.defaultOptions = options - } else { - this.mutationDefaults.push({ mutationKey, defaultOptions: options }) - } - } - - getMutationDefaults( - mutationKey?: MutationKey - ): MutationObserverOptions | undefined { - const mutationDefaults = this.findFirstMatchingMutationDefaults(mutationKey) - return mutationDefaults + return firstMatchingDefaults?.defaultOptions } defaultQueryOptions<