feat: Bail out if query data undefined (#3271)

* Bail out if query data undefined * Fix failing test * docs: migration guide for undefined data * docs: update setQueryData reference * Update docs/src/pages/guides/migrating-to-react-query-4.md Co-authored-by: Louis Law <[email protected]> Co-authored-by: Dominik Dorfmeister <[email protected]>
TanStack · Feb 9, 2022 · 7ee784f · 7ee784f
1 parent 2b5c337
commit 7ee784f
Show file tree

Hide file tree

Showing 13 changed files with 204 additions and 82 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
@@ -245,6 +245,21 @@ Types now require using TypeScript v4.1 or greater
 Starting with v4, react-query will no longer log errors (e.g. failed fetches) to the console in production mode, as this was confusing to many.
 Errors will still show up in development mode.
 
+### Undefined is an illegale cache value for successful queries
+
+In order to make bailing out of updates possible by returning `undefined`, we had to make `undefined` an illegal cache value. This is in-line with other concepts of react-query, for example, returning `undefined` from the [initialData function](guides/initial-query-data#initial-data-function) will also _not_ set data.
+
+Further, it is an easy bug to produce `Promise<void>` by adding logging in the queryFn:
+
+```js
+useQuery(
+  ['key'],
+  () => axios.get(url).then(result => console.log(result.data))
+)
+```
+
+This is now disallowed on type level; at runtime, `undefined` will be transformed to a _failed Promise_, which means you will get an `error`, which will also be logged to the console in development mode.
+
 ## New Features 🚀
 
 ### Proper offline support
@@ -265,3 +280,14 @@ Mutations can now also be garbage collected automatically, just like queries. Th
 ### Tracked Queries per default
 
 React Query defaults to "tracking" query properties, which should give you a nice boost in render optimization. The feature has existed since [v3.6.0](https://github.com/tannerlinsley/react-query/releases/tag/v3.6.0) and has now become the default behavior with v4.
+
+### Bailing out of updates with setQueryData
+
+When using the [functional updater form of setQueryData](../reference/QueryClient#queryclientsetquerydata), you can now bail out of the update by returning `undefined`. This is helpful if `undefined` is given to you as `previousValue`, which means that currently, no cached entry exists and you don't want to / cannot create one, like in the example of toggling a todo:
+
+```js
+ queryClient.setQueryData(
+   ['todo', id],
+   (previousTodo) => previousTodo ? { ...previousTodo, done: true } : undefined
+)
+ ```
diff --git a/docs/src/pages/reference/QueryClient.md b/docs/src/pages/reference/QueryClient.md
@@ -214,7 +214,7 @@ queryClient.setQueryData(queryKey, updater)
 **Options**
 
 - `queryKey: QueryKey`: [Query Keys](../guides/query-keys)
-- `updater: TData | (oldData: TData | undefined) => TData`
+- `updater: TData | (oldData: TData | undefined) => TData | undefined`
   - If non-function is passed, the data will be updated to this value
   - If a function is passed, it will receive the old data value and be expected to return a new one.
 
@@ -224,6 +224,8 @@ queryClient.setQueryData(queryKey, updater)
 setQueryData(queryKey, newData)
 ```
 
+If the value is `undefined`, the query data is not updated.
+
 **Using an updater function**
 
 For convenience in syntax, you can also pass an updater function which receives the current data value and returns the new one:
@@ -232,6 +234,8 @@ For convenience in syntax, you can also pass an updater function which receives
 setQueryData(queryKey, oldData => newData)
 ```
 
+If the updater function returns `undefined`, the query data will not be updated. If the updater function receives `undefined` as input, you can return `undefined` to bail out of the update and thus _not_ create a new cache entry.
+
 ## `queryClient.getQueryState`
 
 `getQueryState` is a synchronous function that can be used to get an existing query's state. If the query does not exist, `undefined` will be returned.

diff --git a/docs/src/pages/reference/useQuery.md b/docs/src/pages/reference/useQuery.md
@@ -68,7 +68,7 @@ const result = useQuery({
 
 **Options**
 
-- `queryKey:  unknown[]`
+- `queryKey: unknown[]`
   - **Required**
   - The query key to use for this query.
   - The query key will be hashed into a stable hash. See [Query Keys](../guides/query-keys) for more information.
@@ -77,7 +77,7 @@ const result = useQuery({
   - **Required, but only if no default query function has been defined** See [Default Query Function](../guides/default-query-function) for more information.
   - The function that the query will use to request data.
   - Receives a [QueryFunctionContext](../guides/query-functions#queryfunctioncontext)
-  - Must return a promise that will either resolve data or throw an error.
+  - Must return a promise that will either resolve data or throw an error. The data cannot be `undefined`.
 - `enabled: boolean`
   - Set this to `false` to disable this query from automatically running.
   - Can be used for [Dependent Queries](../guides/dependent-queries).

diff --git a/src/core/query.ts b/src/core/query.ts
@@ -1,7 +1,5 @@
 import {
   getAbortController,
-  Updater,
-  functionalUpdate,
   noop,
   replaceEqualDeep,
   timeUntilStale,
@@ -195,14 +193,11 @@ export class Query<
   }
 
   setData(
-    updater: Updater<TData | undefined, TData>,
+    data: TData,
     options?: SetDataOptions & { notifySuccess: boolean }
   ): TData {
     const prevData = this.state.data
 
-    // Get the new data
-    let data = functionalUpdate(updater, prevData)
-
     // Use prev data if an isDataEqual function is defined and returns `true`
     if (this.options.isDataEqual?.(prevData, data)) {
       data = prevData as TData
@@ -438,11 +433,41 @@ export class Query<
       this.dispatch({ type: 'fetch', meta: context.fetchOptions?.meta })
     }
 
+    const onError = (error: TError | { silent?: boolean }) => {
+      // Optimistically update state if needed
+      if (!(isCancelledError(error) && error.silent)) {
+        this.dispatch({
+          type: 'error',
+          error: error as TError,
+        })
+      }
+
+      if (!isCancelledError(error)) {
+        // Notify cache callback
+        this.cache.config.onError?.(error, this as Query<any, any, any, any>)
+
+        if (process.env.NODE_ENV !== 'production') {
+          getLogger().error(error)
+        }
+      }
+
+      if (!this.isFetchingOptimistic) {
+        // Schedule query gc after fetching
+        this.scheduleGc()
+      }
+      this.isFetchingOptimistic = false
+    }
+
     // Try to fetch the data
     this.retryer = createRetryer({
       fn: context.fetchFn as () => TData,
       abort: abortController?.abort?.bind(abortController),
       onSuccess: data => {
+        if (typeof data === 'undefined') {
+          onError(new Error('Query data cannot be undefined') as any)
+          return
+        }
+
         this.setData(data as TData)
 
         // Notify cache callback
@@ -454,30 +479,7 @@ export class Query<
         }
         this.isFetchingOptimistic = false
       },
-      onError: (error: TError | { silent?: boolean }) => {
-        // Optimistically update state if needed
-        if (!(isCancelledError(error) && error.silent)) {
-          this.dispatch({
-            type: 'error',
-            error: error as TError,
-          })
-        }
-
-        if (!isCancelledError(error)) {
-          // Notify cache callback
-          this.cache.config.onError?.(error, this as Query<any, any, any, any>)
-
-          if (process.env.NODE_ENV !== 'production') {
-            getLogger().error(error)
-          }
-        }
-
-        if (!this.isFetchingOptimistic) {
-          // Schedule query gc after fetching
-          this.scheduleGc()
-        }
-        this.isFetchingOptimistic = false
-      },
+      onError,
       onFail: () => {
         this.dispatch({ type: 'failed' })
       },

diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts
@@ -8,6 +8,7 @@ import {
   partialMatchKey,
   hashQueryKeyByOptions,
   MutationFilters,
+  functionalUpdate,
 } from './utils'
 import type {
   QueryClientConfig,
@@ -125,14 +126,22 @@ export class QueryClient {
 
   setQueryData<TData>(
     queryKey: QueryKey,
-    updater: Updater<TData | undefined, TData>,
+    updater: Updater<TData | undefined, TData> | undefined,
     options?: SetDataOptions
-  ): TData {
+  ): TData | undefined {
+    const query = this.queryCache.find<TData>(queryKey)
+    const prevData = query?.state.data
+    const data = functionalUpdate(updater, prevData)
+
+    if (typeof data === 'undefined') {
+      return undefined
+    }
+
     const parsedOptions = parseQueryArgs(queryKey)
     const defaultedOptions = this.defaultQueryOptions(parsedOptions)
     return this.queryCache
       .build(this, defaultedOptions)
-      .setData(updater, { ...options, notifySuccess: false })
+      .setData(data, { ...options, notifySuccess: false })
   }
 
   setQueriesData<TData>(
@@ -151,7 +160,7 @@ export class QueryClient {
     queryKeyOrFilters: QueryKey | QueryFilters,
     updater: Updater<TData | undefined, TData>,
     options?: SetDataOptions
-  ): [QueryKey, TData][] {
+  ): [QueryKey, TData | undefined][] {
     return notifyManager.batch(() =>
       this.getQueryCache()
         .findAll(queryKeyOrFilters)

diff --git a/src/core/tests/query.test.tsx b/src/core/tests/query.test.tsx
@@ -12,6 +12,7 @@ import {
   isError,
   onlineManager,
   QueryFunctionContext,
+  QueryObserverResult,
 } from '../..'
 import { waitFor } from '@testing-library/react'
 
@@ -787,6 +788,7 @@ describe('query', () => {
     let signalTest: any
     await queryClient.prefetchQuery(key, ({ signal }) => {
       signalTest = signal
+      return 'data'
     })
 
     expect(signalTest).toBeUndefined()
@@ -814,6 +816,31 @@ describe('query', () => {
     consoleMock.mockRestore()
   })
 
+  test('fetch should dispatch an error if the queryFn returns undefined', async () => {
+    const key = queryKey()
+
+    const observer = new QueryObserver(queryClient, {
+      queryKey: key,
+      queryFn: (() => undefined) as any,
+      retry: false,
+    })
+
+    let observerResult: QueryObserverResult<unknown, unknown> | undefined
+
+    const unsubscribe = observer.subscribe(result => {
+      observerResult = result
+    })
+
+    await sleep(10)
+
+    expect(observerResult).toMatchObject({
+      isError: true,
+      error: new Error('Query data cannot be undefined'),
+    })
+
+    unsubscribe()
+  })
+
   test('fetch should dispatch fetch if is fetching and current promise is undefined', async () => {
     const key = queryKey()