fix: apply CR requested changes reduxjs#2245

Feedback:

- reduxjs#2245 (comment)
- reduxjs#2245 (comment)
- reduxjs#2245 (comment)
- reduxjs#2245 (comment)
- reduxjs#2245 (comment)

Author: FaberVitale

packages/toolkit/src/query/react/buildHooks.ts

@@ -79,9 +79,7 @@ export interface MutationHooks<
   useMutation: UseMutation<Definition>
 }
 
-type IdleState<Arg> = Arg extends SkipToken
-  ? { isSkipped: true }
-  : { isSkipped: boolean }
+type SkippedState<Skipped extends boolean> = { isSkipped: Skipped }
 
 /**
  * A React hook that automatically triggers fetches of data from an endpoint, 'subscribes' the component to the cached data, and reads the request status and cached data from the Redux store. The component will re-render as the loading status changes and the data becomes available.
@@ -98,16 +96,43 @@ type IdleState<Arg> = Arg extends SkipToken
  * - Returns the latest request status and cached data from the Redux store
  * - Re-renders as the request status changes and data becomes available
  */
-export type UseQuery<D extends QueryDefinition<any, any, any, any>> = <
-  R extends Record<string, any> = UseQueryStateDefaultResult<D>,
-  Arg extends QueryArgFrom<D> | SkipToken = QueryArgFrom<D> | SkipToken
->(
-  arg: QueryArgFrom<D> | SkipToken,
-  options?: UseQuerySubscriptionOptions & UseQueryStateOptions<D, R>
-) => UseQueryStateResult<D, R> &
-  ReturnType<UseQuerySubscription<D>> &
-  Suspendable &
-  IdleState<Arg>
+export interface UseQuery<D extends QueryDefinition<any, any, any, any>> {
+  // arg provided
+  <R extends Record<string, any> = UseQueryStateDefaultResult<D>>(
+    arg: QueryArgFrom<D>,
+    options?: UseQuerySubscriptionOptions & UseQueryStateOptions<D, R>
+  ): UseQueryStateResult<D, R> &
+    ReturnType<UseQuerySubscription<D>> &
+    Suspendable &
+    SkippedState<false>
+  // skipped query
+  <R extends Record<string, any> = UseQueryStateDefaultResult<D>>(
+    arg: SkipToken,
+    options?: UseQuerySubscriptionOptions & UseQueryStateOptions<D, R>
+  ): UseQueryStateResult<D, R> &
+    ReturnType<UseQuerySubscription<D>> &
+    Suspendable &
+    SkippedState<true>
+  <R extends Record<string, any> = UseQueryStateDefaultResult<D>>(
+    arg: QueryArgFrom<D> | SkipToken,
+    options?: UseQuerySubscriptionOptions & UseQueryStateOptions<D, R>
+  ): UseQueryStateResult<D, R> &
+    ReturnType<UseQuerySubscription<D>> &
+    Suspendable &
+    SkippedState<boolean>
+}
+
+/**
+ * @internal
+ */
+type UseQueryParams<D extends QueryDefinition<any, any, any, any>> = Parameters<
+  UseQuery<D>
+>
+
+/**
+ * @internal
+ */
+type AnyQueryDefinition = QueryDefinition<any, any, any, any, any>
 
 interface UseQuerySubscriptionOptions extends SubscriptionOptions {
   /**
@@ -551,7 +576,7 @@ const createSuspendablePromise = <
   Definitions,
   Key
 >): Suspendable['getSuspendablePromise'] => {
-  const retry = () => {
+  const fetchOnce = () => {
     prefetch(args, {
       force: true,
     })
@@ -565,27 +590,19 @@ const createSuspendablePromise = <
         let pendingPromise = api.util.getRunningOperationPromise(name, args)
 
         if (!pendingPromise) {
-          prefetch(args, {
-            force: true,
-          })
+          fetchOnce()
 
           pendingPromise = api.util.getRunningOperationPromise(
             name as any,
             args
           )
-
-          if (!pendingPromise) {
-            throw new Error(
-              `[rtk-query][react]: invalid state error, expected getRunningOperationPromise(${name}, ${queryStateResults.requestId}) to be defined`
-            )
-          }
         }
         return pendingPromise
       } else if (queryStateResults.isError && !queryStateResults.isFetching) {
         throw new SuspenseQueryError(
           queryStateResults.error,
           queryStateResults.endpointName + '',
-          retry
+          fetchOnce
         )
       }
     }
@@ -938,7 +955,10 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
           [trigger, queryStateResults, info]
         )
       },
-      useQuery(arg, options) {
+      useQuery(
+        arg: UseQueryParams<AnyQueryDefinition>['0'],
+        options: UseQueryParams<AnyQueryDefinition>['1']
+      ) {
         const isSkipped: boolean = arg === skipToken || !!options?.skip
         const querySubscriptionResults = useQuerySubscription(arg, options)
         const queryStateResults = useQueryState(arg, {

packages/toolkit/src/query/react/exceptions.ts

@@ -4,11 +4,13 @@ const computeErrorMessage = (reason: any, queryKey: string) => {
   if (reason instanceof Error) {
     message += reason
   } else if (typeof reason === 'object' && reason !== null) {
-    ;[reason?.status, reason?.code, reason?.error].forEach((value) => {
-      if (value) {
-        message += ` ${value}`
+    const relevantProperties = [reason?.status, reason?.code, reason?.error]
+
+    for (const property of relevantProperties) {
+      if (property) {
+        message += ` ${property}`
       }
-    })
+    }
   } else {
     message += reason
   }
@@ -25,5 +27,8 @@ export class SuspenseQueryError extends Error {
     super(computeErrorMessage(reason, endpointName))
     this.reason = reason
     this.name = 'SuspenseQueryError'
+
+    // https://www.typescriptlang.org/docs/handbook/2/classes.html#inheriting-built-in-types
+    Object.setPrototypeOf(this, SuspenseQueryError.prototype)
   }
 }

packages/toolkit/src/query/react/suspense-utils.ts

@@ -1,3 +1,5 @@
+import { isPromiseLike } from '../utils/isPromiseLike'
+
 export interface Resource<Data> {
   data?: Data | undefined
   isLoading?: boolean
@@ -34,26 +36,13 @@ export type UseSuspendAllOutput<Sus extends readonly unknown[]> = {
     : never
 }
 
-function isPromiseLike(val: unknown): val is PromiseLike<unknown> {
-  return (
-    !!val && typeof val === 'object' && typeof (val as any).then === 'function'
-  )
-}
-
-function getSuspendable(suspendable: Suspendable) {
+const getSuspendable = (suspendable: Suspendable) => {
   return suspendable.getSuspendablePromise()
 }
 
 export function useSuspendAll<
-  G extends SuspendableResource<any>,
-  T extends SuspendableResource<any>[]
->(
-  ...suspendables: readonly [G, ...T]
-): UseSuspendAllOutput<readonly [G, ...T]> {
-  if (!suspendables.length) {
-    throw new TypeError('useSuspendAll: requires one or more arguments')
-  }
-
+  T extends ReadonlyArray<SuspendableResource<any>>
+>(...suspendables: T): UseSuspendAllOutput<T> {
   let promises = suspendables
     .map(getSuspendable)
     .filter(isPromiseLike) as Promise<unknown>[]
@@ -62,5 +51,5 @@ export function useSuspendAll<
     throw Promise.all(promises)
   }
 
-  return suspendables as UseSuspendAllOutput<readonly [G, ...T]>
+  return suspendables as UseSuspendAllOutput<T>
 }

packages/toolkit/src/query/tests/buildHooks.test.tsx

@@ -1542,7 +1542,7 @@ describe('hooks tests', () => {
       baseQuery: fetchBaseQuery({ baseUrl: 'https://example.com' }),
       tagTypes: ['User'],
       endpoints: (build) => ({
-        checkSession: build.query<any, void>({
+        checkSession: build.query<any, void | undefined>({
           query: () => '/me',
           providesTags: ['User'],
         }),
@@ -1837,7 +1837,7 @@ describe('hooks with createApi defaults set', () => {
       baseQuery: fetchBaseQuery({ baseUrl: 'https://example.com/' }),
       tagTypes: ['Posts'],
       endpoints: (build) => ({
-        getPosts: build.query<PostsResponse, void>({
+        getPosts: build.query<PostsResponse, void | undefined>({
           query: () => ({ url: 'posts' }),
           providesTags: (result) =>
             result ? result.map(({ id }) => ({ type: 'Posts', id })) : [],
@@ -2134,9 +2134,9 @@ describe('hooks with createApi defaults set', () => {
 
     test('useQuery with selectFromResult option has a type error if the result is not an object', async () => {
       function SelectedPost() {
+        // @ts-expect-error
         const _res1 = api.endpoints.getPosts.useQuery(undefined, {
           // selectFromResult must always return an object
-          // @ts-expect-error
           selectFromResult: ({ data }) => data?.length ?? 0,
         })
 
@@ -2434,18 +2434,16 @@ describe('suspense', () => {
   describe('useSuspendAll', () => {
     const consoleErrorSpy = jest.spyOn(console, 'error')
 
-    function ThrowsBecauseNoArgs() {
+    function ExceptionCausedByAnInvalidArg() {
       const tuple = [
         {
-          getSuspendablePromise() {
+          invalid() {
             return undefined
           },
         },
       ] as const
 
-      ;(tuple as unknown as any[]).splice(0, tuple.length)
-
-      useSuspendAll(...tuple)
+      useSuspendAll(...(tuple as any))
       return <div></div>
     }
 
@@ -2457,7 +2455,7 @@ describe('suspense', () => {
             <div data-testid="error-fallback">{String(error)}</div>
           )}
         >
-          <ThrowsBecauseNoArgs />
+          <ExceptionCausedByAnInvalidArg />
         </ErrorBoundary>
       )
 

packages/toolkit/src/query/tests/unionTypes.test.ts

@@ -358,7 +358,10 @@ describe.skip('TS only tests', () => {
       getSuspendablePromise,
       ...useQueryResultWithoutMethods
     } = useQueryResult
-    expectExactType(useQueryStateResult)(useQueryResultWithoutMethods)
+    expectExactType(useQueryStateResult)(
+      // @ts-expect-error
+      useQueryResultWithoutMethods
+    )
     expectExactType(useQueryStateWithSelectFromResult)(
       // @ts-expect-error
       useQueryResultWithoutMethods
@@ -411,10 +414,12 @@ describe.skip('TS only tests', () => {
           isFetching,
           isError,
           isSuccess,
+          isSkipped: false,
           isUninitialized,
         }
       },
     })
+
     expectExactType({
       getSuspendablePromise: expect.any(Function),
       data: '' as string | number,
@@ -423,6 +428,7 @@ describe.skip('TS only tests', () => {
       isFetching: true,
       isSuccess: false,
       isError: false,
+      isSkipped: false,
       refetch: () => {},
     })(result)
   })

packages/toolkit/src/query/utils/isPromiseLike.ts

@@ -0,0 +1,9 @@
+/**
+ * Thenable type guard.
+ * @internal
+ */
+export const isPromiseLike = (val: unknown): val is PromiseLike<unknown> => {
+  return (
+    !!val && typeof val === 'object' && typeof (val as any).then === 'function'
+  )
+}