feat: add deprecation warnings

TkDodo · web-flow · commit c376158afadd · 2022-11-01T11:55:49.000+01:00
* feat: deprecate isDataEqual

* feat: deprecate contextSharing

* feat: deprecate custom loggers

* test: fix ssr-hydration tests

we need to compare against actual console.log here (and not our own logger), because react hydration mismatches are also logged to the console if they exist. This has surfaced another issue because of how we import react for react17/18 differential testing.

* fix: remove useEffect

that would wind up empty in production, and since this is just during development mode, we can just log during render

* test: adapt tests

we can remove most of these in v5

* chore: fix console.mock declaration

* feat: deprecate placeholder data function

* chore: scope test execution to src directories

if we do `pnpm test` locally, it first does typechecking, which produces output in `build` (including test files). Those will then be tested as well and the svelte tests fail with that.

* test: fix vue query test

* un-deprecate placeholderData function

we might still need it after all - to be discussed

* un-deprecate placeholderData function

* fix auto import
diff --git a/docs/guides/custom-logger.md b/docs/guides/custom-logger.md
@@ -20,3 +20,8 @@ const queryClient = new QueryClient({
   },
 })
 ```
+
+**Deprecated**
+
+Custom loggers have been deprecated and will be removed in the next major version.
+Logging only has an effect in development mode, where passing a custom logger is not necessary.
diff --git a/docs/guides/placeholder-query-data.md b/docs/guides/placeholder-query-data.md
@@ -28,9 +28,9 @@ function Todos() {
 }
 ```
 
-### Placeholder Data as a Function
+### Placeholder Data Memoization
 
-If the process for accessing a query's placeholder data is intensive or just not something you want to perform on every render, you can memoize the value or pass a memoized function as the `placeholderData` value:
+If the process for accessing a query's placeholder data is intensive or just not something you want to perform on every render, you can memoize the value:
 
 ```tsx
 function Todos() {
diff --git a/docs/reference/QueryClientProvider.md b/docs/reference/QueryClientProvider.md
@@ -21,6 +21,7 @@ function App() {
   - **Required**
   - the QueryClient instance to provide
 - `contextSharing: boolean`
+  - **Deprecated**
   - defaults to `false`
   - Set this to `true` to enable context sharing, which will share the first and at least one instance of the context across the window to ensure that if React Query is used across different bundles or microfrontends they will all use the same **instance** of context, regardless of module scoping.
 - `context?: React.Context<QueryClient | undefined>`
diff --git a/docs/reference/useQuery.md b/docs/reference/useQuery.md
@@ -36,7 +36,6 @@ const {
   networkMode,
   initialData,
   initialDataUpdatedAt,
-  isDataEqual,
   keepPreviousData,
   meta,
   notifyOnChangeProps,
@@ -171,6 +170,8 @@ const {
   - Defaults to `false`
   - If set, any previous `data` will be kept when fetching new data because the query key changed.
 - `isDataEqual: (oldData: TData | undefined, newData: TData) => boolean`
+  - **Deprecated**. You can achieve the same functionality by passing a function to `structuralSharing` instead:
+    - structuralSharing: (oldData, newData) => isDataEqual(oldData, newData) ? oldData : replaceEqualDeep(oldData, newData)
   - Optional
   - This function should return boolean indicating whether to use previous `data` (`true`) or new data (`false`) as a resolved data for the query.
 - `structuralSharing: boolean | ((oldData: TData | undefined, newData: TData) => TData)`
diff --git a/jest-preset.js b/jest-preset.js
@@ -24,7 +24,7 @@ const moduleNameMapper = {
 module.exports = {
   collectCoverage: true,
   coverageReporters: ['json', 'lcov', 'text', 'clover', 'text-summary'],
-  testMatch: ['<rootDir>/**/*.test.[jt]s?(x)'],
+  testMatch: ['<rootDir>/**/src/**/*.test.[jt]s?(x)'],
   transform: { '^.+\\.(ts|tsx)$': 'ts-jest' },
   clearMocks: true,
   testEnvironment: 'jsdom',
diff --git a/packages/query-core/src/index.ts b/packages/query-core/src/index.ts
@@ -13,6 +13,7 @@ export { focusManager } from './focusManager'
 export { onlineManager } from './onlineManager'
 export {
   hashQueryKey,
+  replaceEqualDeep,
   isError,
   isServer,
   parseQueryArgs,
diff --git a/packages/query-core/src/queryClient.ts b/packages/query-core/src/queryClient.ts
@@ -71,6 +71,12 @@ export class QueryClient {
     this.defaultOptions = config.defaultOptions || {}
     this.queryDefaults = []
     this.mutationDefaults = []
+
+    if (process.env.NODE_ENV !== 'production' && config.logger) {
+      this.logger.error(
+        `Passing a custom logger has been deprecated and will be removed in the next major version.`,
+      )
+    }
   }
 
   mount(): void {
diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts
@@ -155,6 +155,17 @@ export class QueryObserver<
 
     this.options = this.client.defaultQueryOptions(options)
 
+    if (
+      process.env.NODE_ENV !== 'production' &&
+      typeof options?.isDataEqual !== 'undefined'
+    ) {
+      this.client
+        .getLogger()
+        .error(
+          `The isDataEqual option has been deprecated and will be removed in the next major version. You can achieve the same functionality by passing a function as the structuralSharing option`,
+        )
+    }
+
     if (!shallowEqualObjects(prevOptions, this.options)) {
       this.client.getQueryCache().notify({
         type: 'observerOptionsUpdated',
diff --git a/packages/query-core/src/tests/queriesObserver.test.tsx b/packages/query-core/src/tests/queriesObserver.test.tsx
@@ -50,7 +50,13 @@ describe('queriesObserver', () => {
     unsubscribe()
     expect(observerResult).toMatchObject([{ data: 1 }, { data: 2 }])
 
-    expect(mockLogger.error).toHaveBeenCalledTimes(1)
+    expect(mockLogger.error).toHaveBeenCalledTimes(2)
+    expect(mockLogger.error).toHaveBeenCalledWith(
+      'Passing a custom logger has been deprecated and will be removed in the next major version.',
+    )
+    expect(mockLogger.error).toHaveBeenCalledWith(
+      'Passing a custom logger has been deprecated and will be removed in the next major version.',
+    )
   })
 
   test('should update when a query updates', async () => {
diff --git a/packages/query-core/src/tests/queryClient.test.tsx b/packages/query-core/src/tests/queryClient.test.tsx
@@ -160,7 +160,7 @@ describe('queryClient', () => {
       // No defaults, no warning
       const noDefaults = queryClient.getQueryDefaults(keyABCD)
       expect(noDefaults).toBeUndefined()
-      expect(mockLogger.error).not.toHaveBeenCalled()
+      expect(mockLogger.error).toHaveBeenCalledTimes(1)
 
       // If defaults for key ABCD are registered **before** the ones of key ABC (more generic)…
       queryClient.setQueryDefaults(keyABCD, defaultsOfABCD)
@@ -169,7 +169,7 @@ describe('queryClient', () => {
       const goodDefaults = queryClient.getQueryDefaults(keyABCD)
       expect(goodDefaults).toBe(defaultsOfABCD)
       // The warning is still raised since several defaults are matching
-      expect(mockLogger.error).toHaveBeenCalledTimes(1)
+      expect(mockLogger.error).toHaveBeenCalledTimes(2)
 
       // Let's create another queryClient and change the order of registration
       const newQueryClient = createQueryClient()
@@ -180,7 +180,7 @@ describe('queryClient', () => {
       const badDefaults = newQueryClient.getQueryDefaults(keyABCD)
       expect(badDefaults).not.toBe(defaultsOfABCD)
       expect(badDefaults).toBe(defaultsOfABC)
-      expect(mockLogger.error).toHaveBeenCalledTimes(2)
+      expect(mockLogger.error).toHaveBeenCalledTimes(4)
     })
 
     test('should warn in dev if several mutation defaults match a given key', () => {
@@ -216,7 +216,10 @@ describe('queryClient', () => {
       // No defaults, no warning
       const noDefaults = queryClient.getMutationDefaults(keyABCD)
       expect(noDefaults).toBeUndefined()
-      expect(mockLogger.error).not.toHaveBeenCalled()
+      expect(mockLogger.error).toHaveBeenNthCalledWith(
+        1,
+        'Passing a custom logger has been deprecated and will be removed in the next major version.',
+      )
 
       // If defaults for key ABCD are registered **before** the ones of key ABC (more generic)…
       queryClient.setMutationDefaults(keyABCD, defaultsOfABCD)
@@ -225,7 +228,7 @@ describe('queryClient', () => {
       const goodDefaults = queryClient.getMutationDefaults(keyABCD)
       expect(goodDefaults).toBe(defaultsOfABCD)
       // The warning is still raised since several defaults are matching
-      expect(mockLogger.error).toHaveBeenCalledTimes(1)
+      expect(mockLogger.error).toHaveBeenCalledTimes(2)
 
       // Let's create another queryClient and change the order of registration
       const newQueryClient = createQueryClient()
@@ -236,7 +239,7 @@ describe('queryClient', () => {
       const badDefaults = newQueryClient.getMutationDefaults(keyABCD)
       expect(badDefaults).not.toBe(defaultsOfABCD)
       expect(badDefaults).toBe(defaultsOfABC)
-      expect(mockLogger.error).toHaveBeenCalledTimes(2)
+      expect(mockLogger.error).toHaveBeenCalledTimes(4)
     })
   })
 
diff --git a/packages/query-core/src/tests/queryObserver.test.tsx b/packages/query-core/src/tests/queryObserver.test.tsx
@@ -690,7 +690,7 @@ describe('queryObserver', () => {
       },
     })
 
-    expect(mockLogger.error).toHaveBeenNthCalledWith(1, new Error('error'))
+    expect(mockLogger.error).toHaveBeenNthCalledWith(2, new Error('error'))
   })
 
   test('should not use replaceEqualDeep for select value when structuralSharing option is true and placeholderdata is defined', () => {
diff --git a/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx b/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx
@@ -416,8 +416,8 @@ describe('PersistQueryClientProvider', () => {
 
     await waitFor(() => rendered.getByText('fetched'))
     expect(removeClient).toHaveBeenCalledTimes(1)
-    expect(mockLogger.error).toHaveBeenCalledTimes(1)
-    expect(mockLogger.error).toHaveBeenCalledWith(error)
+    expect(mockLogger.error).toHaveBeenCalledTimes(2)
+    expect(mockLogger.error).toHaveBeenNthCalledWith(2, error)
   })
 
   test('should be able to persist into multiple clients', async () => {
diff --git a/packages/react-query/src/QueryClientProvider.tsx b/packages/react-query/src/QueryClientProvider.tsx
@@ -80,6 +80,14 @@ export const QueryClientProvider = ({
     }
   }, [client])
 
+  if (process.env.NODE_ENV !== 'production' && contextSharing) {
+    client
+      .getLogger()
+      .error(
+        `The contextSharing option has been deprecated and will be removed in the next major version`,
+      )
+  }
+
   const Context = getQueryClientContext(context, contextSharing)
 
   return (
diff --git a/packages/react-query/src/__tests__/ssr-hydration.test.tsx b/packages/react-query/src/__tests__/ssr-hydration.test.tsx
@@ -12,7 +12,7 @@ import {
   dehydrate,
   hydrate,
 } from '..'
-import { createQueryClient, mockLogger, setIsServer, sleep } from './utils'
+import { createQueryClient, setIsServer, sleep } from './utils'
 
 const isReact18 = () => (process.env.REACTJS_VERSION || '18') === '18'
 
@@ -55,6 +55,9 @@ describe('Server side rendering with de/rehydration', () => {
     globalThis.IS_REACT_ACT_ENVIRONMENT = previousIsReactActEnvironment
   })
   it('should not mismatch on success', async () => {
+    const consoleMock = jest.spyOn(console, 'error')
+    consoleMock.mockImplementation(() => undefined)
+
     if (!isReact18()) {
       return
     }
@@ -118,15 +121,23 @@ describe('Server side rendering with de/rehydration', () => {
     )
 
     // Check that we have no React hydration mismatches
-    expect(mockLogger.error).not.toHaveBeenCalled()
+    // this should be zero calls and can be changed once we drop react17 support
+    expect(consoleMock).toHaveBeenNthCalledWith(
+      1,
+      'Warning: You are importing hydrateRoot from "react-dom" which is not supported. You should instead import it from "react-dom/client".',
+    )
     expect(fetchDataSuccess).toHaveBeenCalledTimes(2)
     expect(el.innerHTML).toBe(expectedMarkup)
 
     unmount()
     queryClient.clear()
+    consoleMock.mockRestore()
   })
 
   it('should not mismatch on error', async () => {
+    const consoleMock = jest.spyOn(console, 'error')
+    consoleMock.mockImplementation(() => undefined)
+
     if (!isReact18()) {
       return
     }
@@ -187,7 +198,7 @@ describe('Server side rendering with de/rehydration', () => {
     )
 
     // We expect exactly one console.error here, which is from the
-    expect(mockLogger.error).toHaveBeenCalledTimes(1)
+    expect(consoleMock).toHaveBeenCalledTimes(1)
     expect(fetchDataError).toHaveBeenCalledTimes(2)
     expect(el.innerHTML).toBe(expectedMarkup)
     await sleep(50)
@@ -198,9 +209,13 @@ describe('Server side rendering with de/rehydration', () => {
 
     unmount()
     queryClient.clear()
+    consoleMock.mockRestore()
   })
 
   it('should not mismatch on queries that were not prefetched', async () => {
+    const consoleMock = jest.spyOn(console, 'error')
+    consoleMock.mockImplementation(() => undefined)
+
     if (!isReact18()) {
       return
     }
@@ -254,7 +269,11 @@ describe('Server side rendering with de/rehydration', () => {
     )
 
     // Check that we have no React hydration mismatches
-    expect(mockLogger.error).not.toHaveBeenCalled()
+    // this should be zero calls and can be changed once we drop react17 support
+    expect(consoleMock).toHaveBeenNthCalledWith(
+      1,
+      'Warning: You are importing hydrateRoot from "react-dom" which is not supported. You should instead import it from "react-dom/client".',
+    )
     expect(fetchDataSuccess).toHaveBeenCalledTimes(1)
     expect(el.innerHTML).toBe(expectedMarkup)
     await sleep(50)
@@ -265,5 +284,6 @@ describe('Server side rendering with de/rehydration', () => {
 
     unmount()
     queryClient.clear()
+    consoleMock.mockRestore()
   })
 })
diff --git a/packages/solid-query/src/QueryClientProvider.tsx b/packages/solid-query/src/QueryClientProvider.tsx
@@ -80,7 +80,17 @@ export const QueryClientProvider = (
     },
     props,
   )
-  onMount(() => mergedProps.client.mount())
+  onMount(() => {
+    mergedProps.client.mount()
+
+    if (process.env.NODE_ENV !== 'production' && mergedProps.contextSharing) {
+      mergedProps.client
+        .getLogger()
+        .error(
+          `The contextSharing option has been deprecated and will be removed in the next major version`,
+        )
+    }
+  })
   onCleanup(() => mergedProps.client.unmount())
 
   const QueryClientContext = getQueryClientContext(
diff --git a/packages/vue-query/src/__tests__/vueQueryPlugin.test.ts b/packages/vue-query/src/__tests__/vueQueryPlugin.test.ts
@@ -245,7 +245,12 @@ describe('VueQueryPlugin', () => {
     })
 
     test('should use existing context', () => {
-      const customClient = { mount: jest.fn() } as unknown as QueryClient
+      const customClient = {
+        mount: jest.fn(),
+        getLogger: () => ({
+          error: jest.fn(),
+        }),
+      } as unknown as QueryClient
       window.__VUE_QUERY_CONTEXT__ = customClient
       const appMock = getAppMock()
       VueQueryPlugin.install(appMock, { contextSharing: true })
diff --git a/packages/vue-query/src/vueQueryPlugin.ts b/packages/vue-query/src/vueQueryPlugin.ts
@@ -59,6 +59,14 @@ export const VueQueryPlugin = {
 
     client.mount()
 
+    if (process.env.NODE_ENV !== 'production' && options.contextSharing) {
+      client
+        .getLogger()
+        .error(
+          `The contextSharing option has been deprecated and will be removed in the next major version`,
+        )
+    }
+
     const cleanup = () => {
       client.unmount()
     }

Original file line number	Diff line number	Diff line change
`@@ -28,9 +28,9 @@ function Todos() {`
`28`	`28`	`}`
`29`	`29`	```
`30`	`30`
`31`		`-### Placeholder Data as a Function`
	`31`	`+### Placeholder Data Memoization`
`32`	`32`
`33`		-If the process for accessing a query's placeholder data is intensive or just not something you want to perform on every render, you can memoize the value or pass a memoized function as the `placeholderData` value:
	`33`	`+If the process for accessing a query's placeholder data is intensive or just not something you want to perform on every render, you can memoize the value:`
`34`	`34`
`35`	`35`	```tsx
`36`	`36`	`function Todos() {`