From 7f2ca907ba9d590c7a813d93b5253214be469a4f 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.js | 37 ++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/test/hooks/useSelector.spec.js b/test/hooks/useSelector.spec.js
index 17d7a80aa..4da793c1b 100644
--- a/test/hooks/useSelector.spec.js
+++ b/test/hooks/useSelector.spec.js
@@ -45,14 +45,14 @@ describe('React', () => {
           })
 
           expect(result.current).toEqual(0)
-          expect(selector).toHaveBeenCalledTimes(2)
+          expect(selector).toHaveBeenCalledTimes(1)
 
           act(() => {
             store.dispatch({ type: '' })
           })
 
           expect(result.current).toEqual(1)
-          expect(selector).toHaveBeenCalledTimes(3)
+          expect(selector).toHaveBeenCalledTimes(2)
         })
       })
 
@@ -246,6 +246,39 @@ describe('React', () => {
 
           expect(renderedItems.length).toBe(1)
         })
+
+        it('calls selector exactly once on mount and on update', () => {
+          store = createStore(({ count } = { count: 0 }) => ({
+            count: count + 1,
+          }))
+
+          let numCalls = 0
+          const selector = (s) => {
+            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 75b2fba2e2f87e48da9c8ff582ee05c4c140cf5f Mon Sep 17 00:00:00 2001
From: Sufian Rhazi <srhazi@slack-corp.com>
Date: Thu, 19 Aug 2021 18:57:52 -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.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/hooks/useSelector.js b/src/hooks/useSelector.js
index 85e85d13e..652bcef33 100644
--- a/src/hooks/useSelector.js
+++ b/src/hooks/useSelector.js
@@ -65,6 +65,11 @@ function useSelectorWithStoreAndSubscription(
     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 2ac8bfc73e948f6913852e90941d723c3b027e49 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.js | 40 ++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/test/hooks/useSelector.spec.js b/test/hooks/useSelector.spec.js
index 4da793c1b..8eb4bddf4 100644
--- a/test/hooks/useSelector.spec.js
+++ b/test/hooks/useSelector.spec.js
@@ -279,6 +279,46 @@ describe('React', () => {
           expect(numCalls).toBe(2)
           expect(renderedItems.length).toEqual(2)
         })
+
+        it('calls selector twice once on mount when state changes during render', () => {
+          store = createStore(({ count } = { count: 0 }) => ({
+            count: count + 1,
+          }))
+
+          let numCalls = 0
+          const selector = (s) => {
+            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', () => {