Repository: reduxjs/redux-toolkit
Pull request #1035 (2021-05-01 17:04:24)

simplify `selectFromResult` usage

phryneas: So with that, there should be a lot less confusing about using `selectFromResult`, but it's a small breaking change
codesandbox[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/reduxjs/redux-toolkit/pr/1035/builds/125904) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 739adbf0d75272d1a896929bc30d2bea15733e6a:

|Sandbox| Source |
|--|--|
|[Vanilla](https://codesandbox.io/s/vanilla-7sl8w)| Configuration |
|[Vanilla Typescript](https://codesandbox.io/s/vanilla-typescript-nw9ii)| Configuration |
|[rsk-github-issues-example](https://codesandbox.io/s/rsk-github-issues-example-3k7me)| Configuration |

netlify[bot]: Deploy preview for *redux-starter-kit-docs* ready!

Built with commit 739adbf0d75272d1a896929bc30d2bea15733e6a

https://deploy-preview-1035--redux-starter-kit-docs.netlify.app
markerikson: Still only partly understand what all is going on here :)  but seems okay at first glance.

Patch:
diff --git a/src/query/react/buildHooks.ts b/src/query/react/buildHooks.ts
index 8fd65d3b..4bc1e4ce 100644
--- a/src/query/react/buildHooks.ts
+++ b/src/query/react/buildHooks.ts
@@ -4,7 +4,14 @@ import {
   ThunkAction,
   ThunkDispatch,
 } from '@reduxjs/toolkit'
-import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
+import {
+  useCallback,
+  useEffect,
+  useLayoutEffect,
+  useMemo,
+  useRef,
+  useState,
+} from 'react'
 import {
   QueryStatus,
   QuerySubState,
@@ -40,6 +47,7 @@ import {
 import { ReactHooksModuleOptions } from './module'
 import { useShallowStableValue } from './useShallowStableValue'
 import { UninitializedValue, UNINITIALIZED_VALUE } from '../constants'
+import { useStore } from 'react-redux'
 
 export interface QueryHooks<
   Definition extends QueryDefinition<any, any, any, any, any>
@@ -194,18 +202,7 @@ export type UseLazyQuerySubscription<
 export type QueryStateSelector<
   R extends Record<string, any>,
   D extends QueryDefinition<any, any, any, any>
-> = (
-  state: QueryResultSelectorResult<D>,
-  lastResult: R | undefined,
-  defaultQueryStateSelector: DefaultQueryStateSelector<D>
-) => R
-
-export type DefaultQueryStateSelector<
-  D extends QueryDefinition<any, any, any, any>
-> = (
-  state: QueryResultSelectorResult<D>,
-  lastResult: Pick<UseQueryStateDefaultResult<D>, 'data'>
-) => UseQueryStateDefaultResult<D>
+> = (state: UseQueryStateDefaultResult<D>) => R
 
 /**
  * A React hook that 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.
@@ -356,14 +353,7 @@ type UseQueryStateDefaultResult<
 export type MutationStateSelector<
   R extends Record<string, any>,
   D extends MutationDefinition<any, any, any, any>
-> = (
-  state: MutationResultSelectorResult<D>,
-  defaultMutationStateSelector: DefaultMutationStateSelector<D>
-) => R
-
-export type DefaultMutationStateSelector<
-  D extends MutationDefinition<any, any, any, any>
-> = (state: MutationResultSelectorResult<D>) => MutationResultSelectorResult<D>
+> = (state: MutationResultSelectorResult<D>) => R
 
 export type UseMutationStateOptions<
   D extends MutationDefinition<any, any, any, any>,
@@ -396,14 +386,13 @@ export type UseMutation<D extends MutationDefinition<any, any, any, any>> = <
   UseMutationStateResult<D, R>
 ]
 
-const defaultMutationStateSelector: DefaultMutationStateSelector<any> = (
-  currentState
-) => currentState
+const defaultQueryStateSelector: QueryStateSelector<any, any> = (x) => x
+const defaultMutationStateSelector: MutationStateSelector<any, any> = (x) => x
 
-const defaultQueryStateSelector: DefaultQueryStateSelector<any> = (
-  currentState,
-  lastResult
-) => {
+const queryStatePreSelector = (
+  currentState: QueryResultSelectorResult<any>,
+  lastResult: UseQueryStateDefaultResult<any>
+): UseQueryStateDefaultResult<any> => {
   // data is the last known good request result we have tracked - or if none has been tracked yet the last good result for the current args
   const data =
     (currentState.isSuccess ? currentState.data : lastResult?.data) ??
@@ -431,11 +420,9 @@ const defaultQueryStateSelector: DefaultQueryStateSelector<any> = (
  * `{ isUninitialized: false, isFetching: true, isLoading: true }`
  * to prevent that the library user has to do an additional check for `isUninitialized`/
  */
-const noPendingQueryStateSelector: DefaultQueryStateSelector<any> = (
-  currentState,
-  lastResult
+const noPendingQueryStateSelector: QueryStateSelector<any, any> = (
+  selected
 ) => {
-  const selected = defaultQueryStateSelector(currentState, lastResult)
   if (selected.isUninitialized) {
     return {
       ...selected,
@@ -443,7 +430,7 @@ const noPendingQueryStateSelector: DefaultQueryStateSelector<any> = (
       isFetching: true,
       isLoading: true,
       status: QueryStatus.pending,
-    }
+    } as any
   }
   return selected
 }
@@ -633,13 +620,7 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
 
     const useQueryState: UseQueryState<any> = (
       arg: any,
-      {
-        skip = false,
-        selectFromResult = defaultQueryStateSelector as QueryStateSelector<
-          any,
-          any
-        >,
-      } = {}
+      { skip = false, selectFromResult = defaultQueryStateSelector } = {}
     ) => {
       const { select } = api.endpoints[name] as ApiEndpointQuery<
         QueryDefinition<any, any, any, any, any>,
@@ -649,17 +630,21 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
 
       const lastValue = useRef<any>()
 
-      const querySelector = useMemo(
+      const selectDefaultResult = useMemo(
         () =>
           createSelector(
             [
               select(skip ? skipSelector : stableArg),
               (_: any, lastResult: any) => lastResult,
             ],
-            (subState, lastResult) =>
-              selectFromResult(subState, lastResult, defaultQueryStateSelector)
+            queryStatePreSelector
           ),
-        [select, skip, stableArg, selectFromResult]
+        [select, skip, stableArg]
+      )
+
+      const querySelector = useMemo(
+        () => createSelector([selectDefaultResult], selectFromResult),
+        [selectDefaultResult, selectFromResult]
       )
 
       const currentState = useSelector(
@@ -668,9 +653,14 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
         shallowEqual
       )
 
-      useEffect(() => {
-        lastValue.current = currentState
-      }, [currentState])
+      const store = useStore()
+      const newLastValue = selectDefaultResult(
+        store.getState(),
+        lastValue.current
+      )
+      useLayoutEffect(() => {
+        lastValue.current = newLastValue
+      }, [newLastValue])
 
       return currentState
     }
@@ -698,7 +688,7 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
         const queryStateResults = useQueryState(arg, {
           selectFromResult: options?.skip
             ? undefined
-            : (noPendingQueryStateSelector as QueryStateSelector<any, any>),
+            : noPendingQueryStateSelector,
           ...options,
         })
         return useMemo(
@@ -710,12 +700,7 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
   }
 
   function buildMutationHook(name: string): UseMutation<any> {
-    return ({
-      selectFromResult = defaultMutationStateSelector as MutationStateSelector<
-        any,
-        any
-      >,
-    } = {}) => {
+    return ({ selectFromResult = defaultMutationStateSelector } = {}) => {
       const { select, initiate } = api.endpoints[name] as ApiEndpointMutation<
         MutationDefinition<any, any, any, any, any>,
         Definitions
@@ -749,7 +734,7 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
       const mutationSelector = useMemo(
         () =>
           createSelector([select(requestId || skipSelector)], (subState) =>
-            selectFromResult(subState, defaultMutationStateSelector)
+            selectFromResult(subState)
           ),
         [select, requestId, selectFromResult]
       )


Test patch:
diff --git a/src/query/tests/buildHooks.test.tsx b/src/query/tests/buildHooks.test.tsx
index e3395dcc..1f02a2db 100644
--- a/src/query/tests/buildHooks.test.tsx
+++ b/src/query/tests/buildHooks.test.tsx
@@ -79,15 +79,14 @@ afterEach(() => {
   amount = 0
 })
 
+let getRenderCount: () => number = () => 0
+
 describe('hooks tests', () => {
   describe('useQuery', () => {
-    let getRenderCount: () => number = () => 0
-
     test('useQuery hook basic render count assumptions', async () => {
       function User() {
-        getRenderCount = useRenderCounter()
-
         const { isFetching } = api.endpoints.getUser.useQuery(1)
+        getRenderCount = useRenderCounter()
 
         return (
           <div>
@@ -107,12 +106,12 @@ describe('hooks tests', () => {
 
     test('useQuery hook sets isFetching=true whenever a request is in flight', async () => {
       function User() {
-        getRenderCount = useRenderCounter()
         const [value, setValue] = React.useState(0)
 
         const { isFetching } = api.endpoints.getUser.useQuery(1, {
           skip: value < 1,
         })
+        getRenderCount = useRenderCounter()
 
         return (
           <div>
@@ -545,13 +544,13 @@ describe('hooks tests', () => {
           { data: hookData, isFetching, isUninitialized },
         ] = api.endpoints.getUser.useLazyQuery()
         getRenderCount = useRenderCounter()
+
         data = hookData
 
         return (
           <div>
             <div data-testid="isUninitialized">{String(isUninitialized)}</div>
             <div data-testid="isFetching">{String(isFetching)}</div>
-
             <button data-testid="fetchButton" onClick={() => fetchUser(1)}>
               fetchUser
             </button>
@@ -1545,12 +1544,12 @@ describe('hooks with createApi defaults set', () => {
       }
 
       function SelectedPost() {
-        const [renderCount, setRenderCount] = React.useState(0)
         const { post } = api.endpoints.getPosts.useQueryState(undefined, {
           selectFromResult: ({ data }) => ({
             post: data?.find((post) => post.id === 1),
           }),
         })
+        getRenderCount = useRenderCounter()
 
         /**
          * Notes on the renderCount behavior
@@ -1561,11 +1560,7 @@ describe('hooks with createApi defaults set', () => {
          * on rendering.
          */
 
-        React.useEffect(() => {
-          setRenderCount((prev) => prev + 1)
-        }, [post])
-
-        return <div data-testid="renderCount">{String(renderCount)}</div>
+        return <div />
       }
 
       render(
@@ -1576,27 +1571,92 @@ describe('hooks with createApi defaults set', () => {
         { wrapper: storeRef.wrapper }
       )
 
-      expect(screen.getByTestId('renderCount').textContent).toBe('1')
+      expect(getRenderCount()).toBe(1)
 
       const addBtn = screen.getByTestId('addPost')
 
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
 
       fireEvent.click(addBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
       // We fire off a few requests that would typically cause a rerender as JSON.parse() on a request would always be a new object.
       fireEvent.click(addBtn)
       fireEvent.click(addBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
       // Being that it didn't rerender, we can be assured that the behavior is correct
     })
 
+    /**
+     * This test shows that even though a user can select a specific post, the fetching/loading flags
+     * will still cause rerenders for the query. This should show that if you're using selectFromResult,
+     * the 'performance' value comes with selecting _only_ the data.
+     */
+    test('useQuery with selectFromResult with all flags destructured rerenders like the default useQuery behavior', async () => {
+      function Posts() {
+        const { data: posts } = api.endpoints.getPosts.useQuery()
+        const [addPost] = api.endpoints.addPost.useMutation()
+        getRenderCount = useRenderCounter()
+        return (
+          <div>
+            <button
+              data-testid="addPost"
+              onClick={() =>
+                addPost({
+                  name: `some text ${posts?.length}`,
+                  fetched_at: new Date().toISOString(),
+                })
+              }
+            >
+              Add random post
+            </button>
+          </div>
+        )
+      }
+
+      function SelectedPost() {
+        getRenderCount = useRenderCounter()
+
+        const { post } = api.endpoints.getPosts.useQuery(undefined, {
+          selectFromResult: ({
+            data,
+            isUninitialized,
+            isLoading,
+            isFetching,
+            isSuccess,
+            isError,
+          }) => ({
+            post: data?.find((post) => post.id === 1),
+            isUninitialized,
+            isLoading,
+            isFetching,
+            isSuccess,
+            isError,
+          }),
+        })
+
+        return <div />
+      }
+
+      render(
+        <div>
+          <Posts />
+          <SelectedPost />
+        </div>,
+        { wrapper: storeRef.wrapper }
+      )
+      expect(getRenderCount()).toBe(2)
+
+      const addBtn = screen.getByTestId('addPost')
+
+      await waitFor(() => expect(getRenderCount()).toBe(3))
+
+      fireEvent.click(addBtn)
+      await waitFor(() => expect(getRenderCount()).toBe(5))
+      fireEvent.click(addBtn)
+      fireEvent.click(addBtn)
+      await waitFor(() => expect(getRenderCount()).toBe(9))
+    })
+
     test('useQuery with selectFromResult option serves a deeply memoized value and does not rerender unnecessarily', async () => {
       function Posts() {
         const { data: posts } = api.endpoints.getPosts.useQuery()
@@ -1619,18 +1679,14 @@ describe('hooks with createApi defaults set', () => {
       }
 
       function SelectedPost() {
-        const [renderCount, setRenderCount] = React.useState(0)
+        getRenderCount = useRenderCounter()
         const { post } = api.endpoints.getPosts.useQuery(undefined, {
           selectFromResult: ({ data }) => ({
             post: data?.find((post) => post.id === 1),
           }),
         })
 
-        React.useEffect(() => {
-          setRenderCount((prev) => prev + 1)
-        }, [post])
-
-        return <div data-testid="renderCount">{String(renderCount)}</div>
+        return <div />
       }
 
       render(
@@ -1640,23 +1696,17 @@ describe('hooks with createApi defaults set', () => {
         </div>,
         { wrapper: storeRef.wrapper }
       )
-      expect(screen.getByTestId('renderCount').textContent).toBe('1')
+      expect(getRenderCount()).toBe(1)
 
       const addBtn = screen.getByTestId('addPost')
 
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
 
       fireEvent.click(addBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
       fireEvent.click(addBtn)
       fireEvent.click(addBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
     })
 
     test('useQuery with selectFromResult option serves a deeply memoized value, then ONLY updates when the underlying data changes', async () => {
@@ -1690,22 +1740,20 @@ describe('hooks with createApi defaults set', () => {
       }
 
       function SelectedPost() {
-        const [renderCount, setRenderCount] = React.useState(0)
         const { post } = api.endpoints.getPosts.useQuery(undefined, {
           selectFromResult: ({ data }) => ({
             post: data?.find((post) => post.id === 1),
           }),
         })
+        getRenderCount = useRenderCounter()
 
         React.useEffect(() => {
-          setRenderCount((prev) => prev + 1)
           expectablePost = post
         }, [post])
 
         return (
           <div>
             <div data-testid="postName">{post?.name}</div>
-            <div data-testid="renderCount">{String(renderCount)}</div>
           </div>
         )
       }
@@ -1717,31 +1765,23 @@ describe('hooks with createApi defaults set', () => {
         </div>,
         { wrapper: storeRef.wrapper }
       )
-      expect(screen.getByTestId('renderCount').textContent).toBe('1')
+      expect(getRenderCount()).toBe(1)
 
       const addBtn = screen.getByTestId('addPost')
       const updateBtn = screen.getByTestId('updatePost')
 
       fireEvent.click(addBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
       fireEvent.click(addBtn)
       fireEvent.click(addBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('2')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(2))
 
       fireEvent.click(updateBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('3')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(3))
       expect(expectablePost?.name).toBe('supercoooll!')
 
       fireEvent.click(addBtn)
-      await waitFor(() =>
-        expect(screen.getByTestId('renderCount').textContent).toBe('3')
-      )
+      await waitFor(() => expect(getRenderCount()).toBe(3))
     })
 
     test('useQuery with selectFromResult option has a type error if the result is not an object', async () => {
@@ -1805,8 +1845,6 @@ describe('hooks with createApi defaults set', () => {
       ...actionsReducer,
     })
 
-    let getRenderCount: () => number = () => 0
-
     it('causes no more than one rerender when using selectFromResult with an empty object', async () => {
       function Counter() {
         const [increment] = api.endpoints.increment.useMutation({
diff --git a/src/query/tests/unionTypes.test.ts b/src/query/tests/unionTypes.test.ts
index 95d4692e..bd9dbc83 100644
--- a/src/query/tests/unionTypes.test.ts
+++ b/src/query/tests/unionTypes.test.ts
@@ -352,25 +352,61 @@ describe.skip('TS only tests', () => {
 
   test('useQueryState (with selectFromResult)', () => {
     const result = api.endpoints.test.useQueryState(undefined, {
-      selectFromResult({ data, isLoading }) {
-        return { data: data ?? 1, isLoading }
+      selectFromResult({
+        data,
+        isLoading,
+        isFetching,
+        isError,
+        isSuccess,
+        isUninitialized,
+      }) {
+        return {
+          data: data ?? 1,
+          isLoading,
+          isFetching,
+          isError,
+          isSuccess,
+          isUninitialized,
+        }
       },
     })
     expectExactType({
       data: '' as string | number,
-      isLoading: true as boolean,
+      isUninitialized: false,
+      isLoading: true,
+      isFetching: true,
+      isSuccess: false,
+      isError: false,
     })(result)
   })
 
   test('useQuery (with selectFromResult)', () => {
     const result = api.endpoints.test.useQuery(undefined, {
-      selectFromResult({ data, isLoading }) {
-        return { data: data ?? 1, isLoading }
+      selectFromResult({
+        data,
+        isLoading,
+        isFetching,
+        isError,
+        isSuccess,
+        isUninitialized,
+      }) {
+        return {
+          data: data ?? 1,
+          isLoading,
+          isFetching,
+          isError,
+          isSuccess,
+          isUninitialized,
+        }
       },
     })
     expectExactType({
       data: '' as string | number,
-      isLoading: true as boolean,
+      isUninitialized: false,
+      isLoading: true,
+      isFetching: true,
+      isSuccess: false,
+      isError: false,
       refetch: () => {},
     })(result)
   })
@@ -426,6 +462,33 @@ describe.skip('TS only tests', () => {
     }
   })
 
+  test('useMutation (with selectFromResult)', () => {
+    const [_trigger, result] = api.endpoints.mutation.useMutation({
+      selectFromResult({
+        data,
+        isLoading,
+        isError,
+        isSuccess,
+        isUninitialized,
+      }) {
+        return {
+          data: data ?? 'hi',
+          isLoading,
+          isError,
+          isSuccess,
+          isUninitialized,
+        }
+      },
+    })
+    expectExactType({
+      data: '' as string,
+      isUninitialized: false,
+      isLoading: true,
+      isSuccess: false,
+      isError: false,
+    })(result)
+  })
+
   // pre41-remove-start
   test('useMutation TS4.1 union', () => {
     const [_trigger, result] = api.useMutationMutation()