Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc(queryClient): add dev warning with queryDefaults #3249

Merged
merged 16 commits into from
Feb 6, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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.
Guillaume Labat committed Feb 6, 2022
commit 96d74f871b30704e1ed5e027ff7cd5a083d86399
61 changes: 40 additions & 21 deletions src/core/queryClient.ts
Original file line number Diff line number Diff line change
@@ -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(
24 changes: 12 additions & 12 deletions src/core/tests/queryClient.test.tsx
Original file line number Diff line number Diff line change
@@ -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()
})
})