nest data structure for cacheDataLoaded and queryFulfilled (#1078)

phryneas · web-flow · commit 77b576c0a188 · 2021-05-22T17:06:33.000+02:00
to prepare for adding more information in the resolved/rejected values in future releases
diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts
@@ -9,7 +9,11 @@ import type {
   QueryResultSelectorResult,
 } from '../buildSelectors'
 import type { PatchCollection, Recipe } from '../buildThunks'
-import type { SubMiddlewareApi, SubMiddlewareBuilder } from './types'
+import type {
+  PromiseWithKnownReason,
+  SubMiddlewareApi,
+  SubMiddlewareBuilder,
+} from './types'
 
 export type ReferenceCacheLifecycle = never
 
@@ -88,7 +92,15 @@ declare module '../../endpointDefinitions' {
      *
      * If you don't interact with this promise, it will not throw.
      */
-    cacheDataLoaded: Promise<ResultType>
+    cacheDataLoaded: PromiseWithKnownReason<
+      {
+        /**
+         * The (transformed) query result.
+         */
+        data: ResultType
+      },
+      typeof neverResolvedError
+    >
     /**
      * Promise that allows you to wait for the point in time when the cache entry
      * has been removed from the cache, by not being used/subscribed to any more
@@ -150,6 +162,12 @@ declare module '../../endpointDefinitions' {
   }
 }
 
+const neverResolvedError = new Error(
+  'Promise never resolved before cacheEntryRemoved.'
+) as Error & {
+  message: 'Promise never resolved before cacheEntryRemoved.'
+}
+
 export const build: SubMiddlewareBuilder = ({
   api,
   reducerPath,
@@ -163,7 +181,7 @@ export const build: SubMiddlewareBuilder = ({
 
   return (mwApi) => {
     type CacheLifecycle = {
-      valueResolved?(value: unknown): unknown
+      valueResolved?(value: { data: unknown }): unknown
       cacheEntryRemoved(): void
     }
     const lifecycleMap: Record<string, CacheLifecycle> = {}
@@ -201,7 +219,7 @@ export const build: SubMiddlewareBuilder = ({
       } else if (isFullfilledThunk(action)) {
         const lifecycle = lifecycleMap[cacheKey]
         if (lifecycle?.valueResolved) {
-          lifecycle.valueResolved(action.payload.result)
+          lifecycle.valueResolved({ data: action.payload.result })
           delete lifecycle.valueResolved
         }
       } else if (
@@ -244,16 +262,16 @@ export const build: SubMiddlewareBuilder = ({
       const onCacheEntryAdded = endpointDefinition?.onCacheEntryAdded
       if (!onCacheEntryAdded) return
 
-      const neverResolvedError = new Error(
-        'Promise never resolved before cacheEntryRemoved.'
-      )
       let lifecycle = {} as CacheLifecycle
 
       const cacheEntryRemoved = new Promise<void>((resolve) => {
         lifecycle.cacheEntryRemoved = resolve
       })
-      const cacheDataLoaded = Promise.race([
-        new Promise<void>((resolve) => {
+      const cacheDataLoaded: PromiseWithKnownReason<
+        { data: unknown },
+        typeof neverResolvedError
+      > = Promise.race([
+        new Promise<{ data: unknown }>((resolve) => {
           lifecycle.valueResolved = resolve
         }),
         cacheEntryRemoved.then(() => {
diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts
@@ -1,13 +1,23 @@
 import { isPending, isRejected, isFulfilled } from '@reduxjs/toolkit'
-import { BaseQueryFn } from '../../baseQueryTypes'
-import { DefinitionType } from '../../endpointDefinitions'
+import { BaseQueryError, BaseQueryFn } from '../../baseQueryTypes'
+import {
+  DefinitionType,
+  QueryFulfilledRejectionReason,
+} from '../../endpointDefinitions'
 import { Recipe } from '../buildThunks'
-import { SubMiddlewareBuilder } from './types'
+import {
+  SubMiddlewareBuilder,
+  PromiseWithKnownReason,
+  PromiseConstructorWithKnownReason,
+} from './types'
 
 export type ReferenceQueryLifecycle = never
 
 declare module '../../endpointDefinitions' {
-  export interface QueryLifecyclePromises<ResultType> {
+  export interface QueryLifecyclePromises<
+    ResultType,
+    BaseQuery extends BaseQueryFn
+  > {
     /**
      * Promise that will resolve with the (transformed) query result.
      *
@@ -17,9 +27,34 @@ declare module '../../endpointDefinitions' {
      *
      * If you don't interact with this promise, it will not throw.
      */
-    queryFulfilled: Promise<ResultType>
+    queryFulfilled: PromiseWithKnownReason<
+      {
+        /**
+         * The (transformed) query result.
+         */
+        data: ResultType
+      },
+      QueryFulfilledRejectionReason<BaseQuery>
+    >
   }
 
+  type QueryFulfilledRejectionReason<BaseQuery extends BaseQueryFn> =
+    | {
+        error: BaseQueryError<BaseQuery>
+        /**
+         * If this is `false`, that means this error was returned from the `baseQuery` or `queryFn` in a controlled manner.
+         */
+        isUnhandledError: false
+      }
+    | {
+        error: unknown
+        /**
+         * If this is `true`, that means that this error is the result of `baseQueryFn`, `queryFn` or `transformResponse` throwing an error instead of handling it properly.
+         * There can not be made any assumption about the shape of `error`.
+         */
+        isUnhandledError: true
+      }
+
   interface QueryExtraOptions<
     TagTypes extends string,
     ResultType,
@@ -52,7 +87,7 @@ declare module '../../endpointDefinitions' {
     ResultType,
     ReducerPath extends string = string
   > extends QueryBaseLifecycleApi<QueryArg, BaseQuery, ResultType, ReducerPath>,
-      QueryLifecyclePromises<ResultType> {}
+      QueryLifecyclePromises<ResultType, BaseQuery> {}
 
   export interface MutationLifecycleApi<
     QueryArg,
@@ -65,7 +100,7 @@ declare module '../../endpointDefinitions' {
         ResultType,
         ReducerPath
       >,
-      QueryLifecyclePromises<ResultType> {}
+      QueryLifecyclePromises<ResultType, BaseQuery> {}
 }
 
 export const build: SubMiddlewareBuilder = ({
@@ -80,8 +115,8 @@ export const build: SubMiddlewareBuilder = ({
 
   return (mwApi) => {
     type CacheLifecycle = {
-      resolve(value: unknown): unknown
-      reject(value: unknown): unknown
+      resolve(value: { data: unknown }): unknown
+      reject(value: { error: unknown; isUnhandledError: boolean }): unknown
     }
     const lifecycleMap: Record<string, CacheLifecycle> = {}
 
@@ -97,7 +132,10 @@ export const build: SubMiddlewareBuilder = ({
         const onQueryStarted = endpointDefinition?.onQueryStarted
         if (onQueryStarted) {
           const lifecycle = {} as CacheLifecycle
-          const queryFulfilled = new Promise((resolve, reject) => {
+          const queryFulfilled = new (Promise as PromiseConstructorWithKnownReason)<
+            { data: unknown },
+            QueryFulfilledRejectionReason<any>
+          >((resolve, reject) => {
             lifecycle.resolve = resolve
             lifecycle.reject = reject
           })
@@ -133,11 +171,14 @@ export const build: SubMiddlewareBuilder = ({
         }
       } else if (isFullfilledThunk(action)) {
         const { requestId } = action.meta
-        lifecycleMap[requestId]?.resolve(action.payload.result)
+        lifecycleMap[requestId]?.resolve({ data: action.payload.result })
         delete lifecycleMap[requestId]
       } else if (isRejectedThunk(action)) {
-        const { requestId } = action.meta
-        lifecycleMap[requestId]?.reject(action.payload ?? action.error)
+        const { requestId, rejectedWithValue } = action.meta
+        lifecycleMap[requestId]?.reject({
+          error: action.payload ?? action.error,
+          isUnhandledError: !rejectedWithValue,
+        })
         delete lifecycleMap[requestId]
       }
 
diff --git a/src/query/core/buildMiddleware/types.ts b/src/query/core/buildMiddleware/types.ts
@@ -52,3 +52,50 @@ export type SubMiddlewareBuilder = (
   RootState<EndpointDefinitions, string, string>,
   ThunkDispatch<any, any, AnyAction>
 >
+
+export interface PromiseConstructorWithKnownReason {
+  /**
+   * Creates a new Promise with a known rejection reason.
+   * @param executor A callback used to initialize the promise. This callback is passed two arguments:
+   * a resolve callback used to resolve the promise with a value or the result of another promise,
+   * and a reject callback used to reject the promise with a provided reason or error.
+   */
+  new <T, R>(
+    executor: (
+      resolve: (value: T | PromiseLike<T>) => void,
+      reject: (reason?: R) => void
+    ) => void
+  ): PromiseWithKnownReason<T, R>
+}
+
+export interface PromiseWithKnownReason<T, R>
+  extends Omit<Promise<T>, 'then' | 'catch'> {
+  /**
+   * Attaches callbacks for the resolution and/or rejection of the Promise.
+   * @param onfulfilled The callback to execute when the Promise is resolved.
+   * @param onrejected The callback to execute when the Promise is rejected.
+   * @returns A Promise for the completion of which ever callback is executed.
+   */
+  then<TResult1 = T, TResult2 = never>(
+    onfulfilled?:
+      | ((value: T) => TResult1 | PromiseLike<TResult1>)
+      | undefined
+      | null,
+    onrejected?:
+      | ((reason: R) => TResult2 | PromiseLike<TResult2>)
+      | undefined
+      | null
+  ): Promise<TResult1 | TResult2>
+
+  /**
+   * Attaches a callback for only the rejection of the Promise.
+   * @param onrejected The callback to execute when the Promise is rejected.
+   * @returns A Promise for the completion of the callback.
+   */
+  catch<TResult = never>(
+    onrejected?:
+      | ((reason: R) => TResult | PromiseLike<TResult>)
+      | undefined
+      | null
+  ): Promise<T | TResult>
+}
diff --git a/src/query/createApi.ts b/src/query/createApi.ts
@@ -23,7 +23,7 @@ export interface CreateApiOptions<
    *
    * ```ts
    * import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query'
-   * 
+   *
    * const api = createApi({
    *   // highlight-start
    *   baseQuery: fetchBaseQuery({ baseUrl: '/' }),
@@ -37,12 +37,12 @@ export interface CreateApiOptions<
   baseQuery: BaseQuery
   /**
    * An array of string tag type names. Specifying tag types is optional, but you should define them so that they can be used for caching and invalidation. When defining an tag type, you will be able to [provide](../../usage/rtk-query/cached-data#providing-tags) them with `provides` and [invalidate](../../usage/rtk-query/cached-data#invalidating-tags) them with `invalidates` when configuring [endpoints](#endpoints).
-   * 
+   *
    * @example
-   * 
+   *
    * ```ts
    * import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query'
-   * 
+   *
    * const api = createApi({
    *   baseQuery: fetchBaseQuery({ baseUrl: '/' }),
    *   // highlight-start
@@ -229,15 +229,14 @@ export function buildCreateApi<Modules extends [Module<any>, ...Module<any>[]]>(
                 '`onStart`, `onSuccess` and `onError` have been replaced by `onQueryStarted`, please change your code accordingly'
               )
             }
-            x.onQueryStarted ??= async (arg, { queryFulfilled, ...api }) => {
+            x.onQueryStarted ??= (arg, { queryFulfilled, ...api }) => {
               const queryApi = { ...api, context: {} }
               x.onStart?.(arg, queryApi)
-              try {
-                const result = await queryFulfilled
-                x.onSuccess?.(arg, queryApi, result, undefined)
-              } catch (error) {
-                x.onError?.(arg, queryApi, error, undefined)
-              }
+              return queryFulfilled.then(
+                (result) =>
+                  x.onSuccess?.(arg, queryApi, result.data, undefined),
+                (reason) => x.onError?.(arg, queryApi, reason.error, undefined)
+              )
             }
           }
           return { ...x, type: DefinitionType.query } as any
@@ -253,15 +252,14 @@ export function buildCreateApi<Modules extends [Module<any>, ...Module<any>[]]>(
                 '`onStart`, `onSuccess` and `onError` have been replaced by `onQueryStarted`, please change your code accordingly'
               )
             }
-            x.onQueryStarted ??= async (arg, { queryFulfilled, ...api }) => {
+            x.onQueryStarted ??= (arg, { queryFulfilled, ...api }) => {
               const queryApi = { ...api, context: {} }
               x.onStart?.(arg, queryApi)
-              try {
-                const result = await queryFulfilled
-                x.onSuccess?.(arg, queryApi, result, undefined)
-              } catch (error) {
-                x.onError?.(arg, queryApi, error, undefined)
-              }
+              return queryFulfilled.then(
+                (result) =>
+                  x.onSuccess?.(arg, queryApi, result.data, undefined),
+                (reason) => x.onError?.(arg, queryApi, reason.error, undefined)
+              )
             }
           }
           return { ...x, type: DefinitionType.mutation } as any
diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts
@@ -108,7 +108,7 @@ describe.each([['query'], ['mutation']] as const)(
       await fakeTimerWaitFor(() => {
         expect(gotFirstValue).toHaveBeenCalled()
       })
-      expect(gotFirstValue).toHaveBeenCalledWith({ value: 'success' })
+      expect(gotFirstValue).toHaveBeenCalledWith({ data: { value: 'success' } })
       expect(onCleanup).not.toHaveBeenCalled()
 
       promise.unsubscribe(), await waitMs()
diff --git a/src/query/tests/queryLifecycle.test.tsx b/src/query/tests/queryLifecycle.test.tsx
@@ -59,7 +59,7 @@ describe.each([['query'], ['mutation']] as const)(
       storeRef.store.dispatch(extended.endpoints.injected.initiate('arg'))
       expect(onStart).toHaveBeenCalledWith('arg')
       await waitFor(() => {
-        expect(onSuccess).toHaveBeenCalledWith({ value: 'success' })
+        expect(onSuccess).toHaveBeenCalledWith({ data: { value: 'success' } })
       })
     })
 
@@ -85,8 +85,11 @@ describe.each([['query'], ['mutation']] as const)(
       expect(onStart).toHaveBeenCalledWith('arg')
       await waitFor(() => {
         expect(onError).toHaveBeenCalledWith({
-          status: 500,
-          data: { value: 'error' },
+          error: {
+            status: 500,
+            data: { value: 'error' },
+          },
+          isUnhandledError: false,
         })
       })
       expect(onSuccess).not.toHaveBeenCalled()
