Repository: reduxjs/react-redux
Pull request #1234 (2019-04-12 09:44:14)

memoize renderedWrappedComponent separately to prevent this step from…
the following issue my @welldone-software/why-did-you-render library:
https://github.com/welldone-software/why-did-you-render/issues/18
descovered a small performance improvement possibility:

`WrappedComponent` should not be re-created for every state change. Instead it can be memoized separately.

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

Built with commit 453ae85985478d1265b13191ef6d8f8be5303c34

https://deploy-preview-1234--react-redux-docs.netlify.com
vzaidman: initial https://github.com/reduxjs/react-redux-benchmarks results:
![image](https://user-images.githubusercontent.com/5188459/56029955-4f2bff00-5d24-11e9-8c0a-16f1496bfa66.png)

vzaidman: initial https://github.com/reduxjs/react-redux-benchmarks results:

![image](https://user-images.githubusercontent.com/5188459/56030820-7d124300-5d26-11e9-903b-807f2037e53b.png)

![image](https://user-images.githubusercontent.com/5188459/56030826-81d6f700-5d26-11e9-8087-6a10e5f0b993.png)

![image](https://user-images.githubusercontent.com/5188459/56030849-8b605f00-5d26-11e9-93de-817982c274ce.png)


Patch:
diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js
index 6169fcb..c1f98f1 100644
--- a/src/components/connectAdvanced.js
+++ b/src/components/connectAdvanced.js
@@ -291,7 +291,7 @@ export default function connectAdvanced(
       })
 
       // Our re-subscribe logic only runs when the store/subscription setup changes
-      useEffect(() => {
+      useIsomorphicLayoutEffect(() => {
         // If we're not subscribed to the store, nothing to do here
         if (!shouldHandleStateChanges) return
 
@@ -375,14 +375,14 @@ export default function connectAdvanced(
 
       // Now that all that's done, we can finally try to actually render the child component.
       // We memoize the elements for the rendered child component as an optimization.
+      const renderedWrappedComponent = useMemo(
+        () => <WrappedComponent {...actualChildProps} ref={forwardedRef} />,
+        [forwardedRef, WrappedComponent, actualChildProps]
+      )
+
       // If React sees the exact same element reference as last time, it bails out of re-rendering
       // that child, same as if it was wrapped in React.memo() or returned false from shouldComponentUpdate.
       const renderedChild = useMemo(() => {
-        // Render the actual child component
-        const renderedWrappedComponent = (
-          <WrappedComponent {...actualChildProps} ref={forwardedRef} />
-        )
-
         if (shouldHandleStateChanges) {
           // If this component is subscribed to store updates, we need to pass its own
           // subscription instance down to our descendants. That means rendering the same
@@ -395,13 +395,7 @@ export default function connectAdvanced(
         }
 
         return renderedWrappedComponent
-      }, [
-        ContextToUse,
-        WrappedComponent,
-        actualChildProps,
-        forwardedRef,
-        overriddenContextValue
-      ])
+      }, [ContextToUse, renderedWrappedComponent, overriddenContextValue])
 
       return renderedChild
     }


Test patch:
diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js
index 9e8a3a9..b2bbe2a 100644
--- a/test/components/connect.spec.js
+++ b/test/components/connect.spec.js
@@ -1926,6 +1926,38 @@ describe('React', () => {
 
         expect(mapStateToProps).toHaveBeenCalledTimes(2)
       })
+
+      it('should not notify nested components after they are unmounted', () => {
+        @connect(state => ({ count: state }))
+        class Parent extends Component {
+          render() {
+            return this.props.count === 1 ? <Child /> : null
+          }
+        }
+
+        const mapStateToProps = jest.fn(state => ({ count: state }))
+        @connect(mapStateToProps)
+        class Child extends Component {
+          render() {
+            return <div>{this.props.count}</div>
+          }
+        }
+
+        const store = createStore((state = 0, action) =>
+          action.type === 'INC' ? state + 1 : state
+        )
+        rtl.render(
+          <ProviderMock store={store}>
+            <Parent />
+          </ProviderMock>
+        )
+
+        expect(mapStateToProps).toHaveBeenCalledTimes(0)
+        store.dispatch({ type: 'INC' })
+        expect(mapStateToProps).toHaveBeenCalledTimes(1)
+        store.dispatch({ type: 'INC' })
+        expect(mapStateToProps).toHaveBeenCalledTimes(1)
+      })
     })
 
     describe('Custom context and store-as-prop', () => {