2964 changes to on success callback (#2969)

* feat(useQuery): onSuccess callback

do not call onSuccess if update was done manually from setQueryData

* feat(useQuery): onSuccess callback

test that onSuccess is not called when setQueryData is used

* feat(useQuery): onSuccess callback

docs changes

* feat(useQuery): onSuccess callback

options spread is wrong - `updatedAt` is actually `dataUpdatedAt`. Oddly we didn't have a test, so I added one

docs/src/pages/guides/migrating-to-react-query-4.md

@@ -132,6 +132,19 @@ If you were importing anything from `'react-query/react'` directly in your proje
 + import { QueryClientProvider } from 'react-query/reactjs';
 ```
 
+### `onSuccess` is no longer called from `setQueryData`
+
+This was confusing to many and also created infinite loops if `setQueryData` was called from within `onSuccess`. It was also a frequent source of error when combined with `staleTime`, because if data was read from the cache only, `onSuccess` was _not_ called.
+
+Similar to `onError` and `onSettled`, the `onSuccess` callback is now tied to a request being made. No request -> no callback.
+
+If you want to listen to changes of the `data` field, you can best do this with a `useEffect`, where `data` is part of the dependency Array. Since react-query ensures stable data through structural sharing, the effect will not execute with every background refetch, but only if something within data has changed:
+
+```
+const { data } = useQuery({ queryKey, queryFn })
+React.useEffect(() => mySideEffectHere(data), [data])
+```
+
 ### `persistQueryClient` and the corresponding persister plugins are no longer experimental and have been renamed
 
 The plugins `createWebStoragePersistor` and `createAsyncStoragePersistor` have been renamed to [`createWebStoragePersister`](/plugins/createWebStoragePersister) and [`createAsyncStoragePersister`](/plugins/createAsyncStoragePersister) respectively. The interface `Persistor` in `persistQueryClient` has also been renamed to `Persister`. Checkout [this stackexchange](https://english.stackexchange.com/questions/206893/persister-or-persistor) for the motivation of this change.

docs/src/pages/reference/QueryClient.md

@@ -205,8 +205,6 @@ This distinction is more a "convenience" for ts devs that know which structure w
 
 `setQueryData` is a synchronous function that can be used to immediately update a query's cached data. If the query does not exist, it will be created. **If the query is not utilized by a query hook in the default `cacheTime` of 5 minutes, the query will be garbage collected**.
 
-After successful changing query's cached data via `setQueryData`, it will also trigger `onSuccess` callback from that query.
-
 > The difference between using `setQueryData` and `fetchQuery` is that `setQueryData` is sync and assumes that you already synchronously have the data available. If you need to fetch the data asynchronously, it's suggested that you either refetch the query key or use `fetchQuery` to handle the asynchronous fetch.
 
 ```js

docs/src/pages/reference/useQuery.md

@@ -137,7 +137,7 @@ const result = useQuery({
   - If set to `['isStale']` for example, the component will not re-render when the `isStale` property changes.
 - `onSuccess: (data: TData) => void`
   - Optional
-  - This function will fire any time the query successfully fetches new data or the cache is updated via `setQueryData`.
+  - This function will fire any time the query successfully fetches new data.
 - `onError: (error: TError) => void`
   - Optional
   - This function will fire if the query encounters an error and will be passed the error.

src/core/query.ts

@@ -100,6 +100,7 @@ interface SuccessAction<TData> {
   data: TData | undefined
   type: 'success'
   dataUpdatedAt?: number
+  notifySuccess?: boolean
 }
 
 interface ErrorAction<TError> {
@@ -202,7 +203,7 @@ export class Query<
 
   setData(
     updater: Updater<TData | undefined, TData>,
-    options?: SetDataOptions
+    options?: SetDataOptions & { notifySuccess: boolean }
   ): TData {
     const prevData = this.state.data
 
@@ -222,6 +223,7 @@ export class Query<
       data,
       type: 'success',
       dataUpdatedAt: options?.updatedAt,
+      notifySuccess: options?.notifySuccess,
     })
 
     return data

src/core/queryClient.ts

@@ -136,7 +136,7 @@ export class QueryClient {
     const defaultedOptions = this.defaultQueryOptions(parsedOptions)
     return this.queryCache
       .build(this, defaultedOptions)
-      .setData(updater, options)
+      .setData(updater, { ...options, notifySuccess: false })
   }
 
   setQueriesData<TData>(

src/core/queryObserver.ts

@@ -679,7 +679,7 @@ export class QueryObserver<
     const notifyOptions: NotifyOptions = {}
 
     if (action.type === 'success') {
-      notifyOptions.onSuccess = true
+      notifyOptions.onSuccess = action.notifySuccess ?? true
     } else if (action.type === 'error' && !isCancelledError(action.error)) {
       notifyOptions.onError = true
     }

src/core/tests/queryClient.test.tsx

@@ -1,5 +1,15 @@
-import { sleep, queryKey, mockConsoleError } from '../../reactjs/tests/utils'
+import { waitFor } from '@testing-library/react'
+import '@testing-library/jest-dom'
+import React from 'react'
+
+import {
+  sleep,
+  queryKey,
+  mockConsoleError,
+  renderWithClient,
+} from '../../reactjs/tests/utils'
 import {
+  useQuery,
   InfiniteQueryObserver,
   QueryCache,
   QueryClient,
@@ -219,6 +229,62 @@ describe('queryClient', () => {
 
       expect(queryCache.find(key)!.state.data).toBe(newData)
     })
+
+    test('should not call onSuccess callback of active observers', async () => {
+      const key = queryKey()
+      const onSuccess = jest.fn()
+
+      function Page() {
+        const state = useQuery(key, () => 'data', { onSuccess })
+        return (
+          <div>
+            <div>data: {state.data}</div>
+            <button onClick={() => queryClient.setQueryData(key, 'newData')}>
+              setQueryData
+            </button>
+          </div>
+        )
+      }
+
+      const rendered = renderWithClient(queryClient, <Page />)
+
+      await waitFor(() => rendered.getByText('data: data'))
+      rendered.getByRole('button', { name: /setQueryData/i }).click()
+      await waitFor(() => rendered.getByText('data: newData'))
+
+      expect(onSuccess).toHaveBeenCalledTimes(1)
+      expect(onSuccess).toHaveBeenCalledWith('data')
+    })
+
+    test('should respect updatedAt', async () => {
+      const key = queryKey()
+
+      function Page() {
+        const state = useQuery(key, () => 'data')
+        return (
+          <div>
+            <div>data: {state.data}</div>
+            <div>dataUpdatedAt: {state.dataUpdatedAt}</div>
+            <button
+              onClick={() =>
+                queryClient.setQueryData(key, 'newData', { updatedAt: 100 })
+              }
+            >
+              setQueryData
+            </button>
+          </div>
+        )
+      }
+
+      const rendered = renderWithClient(queryClient, <Page />)
+
+      await waitFor(() => rendered.getByText('data: data'))
+      rendered.getByRole('button', { name: /setQueryData/i }).click()
+      await waitFor(() => rendered.getByText('data: newData'))
+      await waitFor(() => {
+        expect(rendered.getByText('dataUpdatedAt: 100')).toBeInTheDocument()
+      })
+    })
   })
 
   describe('setQueriesData', () => {