From 9ecceb2a78e5ebd4b84d8830ca41983a8c4a3e52 Mon Sep 17 00:00:00 2001
From: Sufian Rhazi <srhazi@slack-corp.com>
Date: Thu, 19 Aug 2021 18:14:45 -0400
Subject: [PATCH 1/3] Add a failing tests (and update one test)

Currently, useSelector(selector) will call the passed selector twice on
mount when it only needs to be called once on mount. This is
unnecessary, and in the case of expensive selectors, can be a
performance concern.
---
 test/hooks/useSelector.spec.tsx | 40 +++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/test/hooks/useSelector.spec.tsx b/test/hooks/useSelector.spec.tsx
index a9d48f11c..2c85a88f1 100644
--- a/test/hooks/useSelector.spec.tsx
+++ b/test/hooks/useSelector.spec.tsx
@@ -72,14 +72,14 @@ describe('React', () => {
           })
 
           expect(result.current).toEqual(0)
-          expect(selector).toHaveBeenCalledTimes(2)
+          expect(selector).toHaveBeenCalledTimes(1)
 
           act(() => {
             normalStore.dispatch({ type: '' })
           })
 
           expect(result.current).toEqual(1)
-          expect(selector).toHaveBeenCalledTimes(3)
+          expect(selector).toHaveBeenCalledTimes(2)
         })
       })
 
@@ -283,6 +283,42 @@ describe('React', () => {
 
           expect(renderedItems.length).toBe(1)
         })
+
+        it('calls selector exactly once on mount and on update', () => {
+          interface StateType {
+            count: number
+          }
+          const store = createStore(({ count }: StateType = { count: 0 }) => ({
+            count: count + 1,
+          }))
+
+          let numCalls = 0
+          const selector = (s: StateType) => {
+            numCalls += 1
+            return s.count
+          }
+          const renderedItems = []
+
+          const Comp = () => {
+            const value = useSelector(selector)
+            renderedItems.push(value)
+            return <div />
+          }
+
+          rtl.render(
+            <ProviderMock store={store}>
+              <Comp />
+            </ProviderMock>
+          )
+
+          expect(numCalls).toBe(1)
+          expect(renderedItems.length).toEqual(1)
+
+          store.dispatch({ type: '' })
+
+          expect(numCalls).toBe(2)
+          expect(renderedItems.length).toEqual(2)
+        })
       })
 
       it('uses the latest selector', () => {

From 9a28e78e68c2c0c5251c72e6a2e3359177e650dd Mon Sep 17 00:00:00 2001
From: Sufian Rhazi <srhazi@slack-corp.com>
Date: Tue, 24 Aug 2021 18:44:36 -0400
Subject: [PATCH 2/3] Reduce the number of calls to the provided selector by
 one

By checking if the store state has differed prior to recalculating a
selector, we can avoid unnecessary selector recalculation in most cases
for components on mount.
---
 src/hooks/useSelector.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/hooks/useSelector.ts b/src/hooks/useSelector.ts
index ee072d9f1..567ef1872 100644
--- a/src/hooks/useSelector.ts
+++ b/src/hooks/useSelector.ts
@@ -71,6 +71,11 @@ function useSelectorWithStoreAndSubscription<TStoreState, TSelectedState>(
     function checkForUpdates() {
       try {
         const newStoreState = store.getState()
+        // Avoid calling selector multiple times if the store's state has not changed
+        if (newStoreState === latestStoreState.current) {
+          return
+        }
+
         const newSelectedState = latestSelector.current!(newStoreState)
 
         if (equalityFn(newSelectedState, latestSelectedState.current)) {

From afa7c8e4f5afc549c614caa87f635b3d697d9d8d Mon Sep 17 00:00:00 2001
From: Sufian Rhazi <srhazi@slack-corp.com>
Date: Wed, 25 Aug 2021 10:30:19 -0400
Subject: [PATCH 3/3] Add test to ensure we are calling selector on state
 change

---
 test/hooks/useSelector.spec.tsx | 43 +++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/test/hooks/useSelector.spec.tsx b/test/hooks/useSelector.spec.tsx
index 2c85a88f1..038bd4ae2 100644
--- a/test/hooks/useSelector.spec.tsx
+++ b/test/hooks/useSelector.spec.tsx
@@ -319,6 +319,49 @@ describe('React', () => {
           expect(numCalls).toBe(2)
           expect(renderedItems.length).toEqual(2)
         })
+
+        it('calls selector twice once on mount when state changes during render', () => {
+          interface StateType {
+            count: number
+          }
+          const store = createStore(({ count }: StateType = { count: 0 }) => ({
+            count: count + 1,
+          }))
+
+          let numCalls = 0
+          const selector = (s: StateType) => {
+            numCalls += 1
+            return s.count
+          }
+          const renderedItems = []
+
+          const Child = () => {
+            useLayoutEffect(() => {
+              store.dispatch({ type: '', count: 1 })
+            }, [])
+            return <div />
+          }
+
+          const Comp = () => {
+            const value = useSelector(selector)
+            renderedItems.push(value)
+            return (
+              <div>
+                <Child />
+              </div>
+            )
+          }
+
+          rtl.render(
+            <ProviderMock store={store}>
+              <Comp />
+            </ProviderMock>
+          )
+
+          // Selector first called on Comp mount, and then re-invoked after mount due to useLayoutEffect dispatching event
+          expect(numCalls).toBe(2)
+          expect(renderedItems.length).toEqual(2)
+        })
       })
 
       it('uses the latest selector', () => {