From 5dbbd6fe6c878728c3af44ea2400baaf063bdde6 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Thu, 18 Nov 2021 17:31:38 +0100 Subject: [PATCH 1/4] feat(useQuery): onSuccess callback do not call onSuccess if update was done manually from setQueryData --- src/core/query.ts | 5 +++-- src/core/queryClient.ts | 2 +- src/core/queryObserver.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/core/query.ts b/src/core/query.ts index 643825e381..8f4ba0443a 100644 --- a/src/core/query.ts +++ b/src/core/query.ts @@ -100,6 +100,7 @@ interface SuccessAction { data: TData | undefined type: 'success' dataUpdatedAt?: number + notifySuccess?: boolean } interface ErrorAction { @@ -202,7 +203,7 @@ export class Query< setData( updater: Updater, - options?: SetDataOptions + options?: SetDataOptions & { notifySuccess: boolean } ): TData { const prevData = this.state.data @@ -221,7 +222,7 @@ export class Query< this.dispatch({ data, type: 'success', - dataUpdatedAt: options?.updatedAt, + ...options, }) return data diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts index 32cf43e160..2d837be6d1 100644 --- a/src/core/queryClient.ts +++ b/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( diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts index fceea94f37..e41d73517c 100644 --- a/src/core/queryObserver.ts +++ b/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 } From 82e88780b0f26ead8d8690d2ffd264bbb11834a9 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Thu, 18 Nov 2021 17:38:11 +0100 Subject: [PATCH 2/4] feat(useQuery): onSuccess callback test that onSuccess is not called when setQueryData is used --- src/core/tests/queryClient.test.tsx | 37 ++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx index 16fe2936f7..a758b8aef4 100644 --- a/src/core/tests/queryClient.test.tsx +++ b/src/core/tests/queryClient.test.tsx @@ -1,5 +1,14 @@ -import { sleep, queryKey, mockConsoleError } from '../../reactjs/tests/utils' +import { waitFor } from '@testing-library/react' +import React from 'react' + +import { + sleep, + queryKey, + mockConsoleError, + renderWithClient, +} from '../../reactjs/tests/utils' import { + useQuery, InfiniteQueryObserver, QueryCache, QueryClient, @@ -219,6 +228,32 @@ 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 ( +
+
data: {state.data}
+ +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + 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') + }) }) describe('setQueriesData', () => { From 0c4d3509bd5613ac16af666ac524a50d128cad6b Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Thu, 18 Nov 2021 18:05:43 +0100 Subject: [PATCH 3/4] feat(useQuery): onSuccess callback docs changes --- docs/src/pages/guides/migrating-to-react-query-4.md | 13 +++++++++++++ docs/src/pages/reference/QueryClient.md | 2 -- docs/src/pages/reference/useQuery.md | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/src/pages/guides/migrating-to-react-query-4.md b/docs/src/pages/guides/migrating-to-react-query-4.md index 01a7d79c84..b10a0db62b 100644 --- a/docs/src/pages/guides/migrating-to-react-query-4.md +++ b/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. diff --git a/docs/src/pages/reference/QueryClient.md b/docs/src/pages/reference/QueryClient.md index 42f29eadaf..ff2289d58f 100644 --- a/docs/src/pages/reference/QueryClient.md +++ b/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 diff --git a/docs/src/pages/reference/useQuery.md b/docs/src/pages/reference/useQuery.md index a2464cd2b7..e7e1ca89b9 100644 --- a/docs/src/pages/reference/useQuery.md +++ b/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. From 829254bb07e32703956631963fb3c5a518760376 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 19 Nov 2021 22:42:11 +0100 Subject: [PATCH 4/4] feat(useQuery): onSuccess callback options spread is wrong - `updatedAt` is actually `dataUpdatedAt`. Oddly we didn't have a test, so I added one --- src/core/query.ts | 3 ++- src/core/tests/queryClient.test.tsx | 31 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/core/query.ts b/src/core/query.ts index 8f4ba0443a..52ba42c6d2 100644 --- a/src/core/query.ts +++ b/src/core/query.ts @@ -222,7 +222,8 @@ export class Query< this.dispatch({ data, type: 'success', - ...options, + dataUpdatedAt: options?.updatedAt, + notifySuccess: options?.notifySuccess, }) return data diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx index a758b8aef4..79c1398b79 100644 --- a/src/core/tests/queryClient.test.tsx +++ b/src/core/tests/queryClient.test.tsx @@ -1,4 +1,5 @@ import { waitFor } from '@testing-library/react' +import '@testing-library/jest-dom' import React from 'react' import { @@ -254,6 +255,36 @@ describe('queryClient', () => { expect(onSuccess).toHaveBeenCalledTimes(1) expect(onSuccess).toHaveBeenCalledWith('data') }) + + test('should respect updatedAt', async () => { + const key = queryKey() + + function Page() { + const state = useQuery(key, () => 'data') + return ( +
+
data: {state.data}
+
dataUpdatedAt: {state.dataUpdatedAt}
+ +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + 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', () => {