From 4734a71a7baef4c9321e7646a122bdf271452c2b Mon Sep 17 00:00:00 2001
From: Michael Peyper <michael.peyper@ioof.com.au>
Date: Sun, 11 Aug 2019 22:49:14 +1000
Subject: [PATCH 1/2] Convert Provider into function component with hooks

Fixes issue where the store and children got updated in different render passes which could result in mapState errors if the new children were not compatible with the old store.
Fixes #1370
---
 src/components/Provider.js       | 61 +++++++++-----------------------
 test/components/Provider.spec.js | 38 ++++++++++++++++++++
 2 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/src/components/Provider.js b/src/components/Provider.js
index 6b3e46ba2..2c3e5b54c 100644
--- a/src/components/Provider.js
+++ b/src/components/Provider.js
@@ -1,62 +1,35 @@
-import React, { Component } from 'react'
+import React, { useMemo, useEffect } from 'react'
 import PropTypes from 'prop-types'
 import { ReactReduxContext } from './Context'
 import Subscription from '../utils/Subscription'
 
-class Provider extends Component {
-  constructor(props) {
-    super(props)
-
-    const { store } = props
-
-    this.notifySubscribers = this.notifySubscribers.bind(this)
+function Provider({ store, context, children }) {
+  const contextValue = useMemo(() => {
     const subscription = new Subscription(store)
-    subscription.onStateChange = this.notifySubscribers
-
-    this.state = {
+    subscription.onStateChange = subscription.notifyNestedSubs
+    return {
       store,
       subscription
     }
+  }, [store])
 
-    this.previousState = store.getState()
-  }
+  const previousState = useMemo(() => store.getState(), [store])
 
-  componentDidMount() {
-    this.state.subscription.trySubscribe()
+  useEffect(() => {
+    const { subscription } = contextValue
+    subscription.trySubscribe()
 
-    if (this.previousState !== this.props.store.getState()) {
-      this.state.subscription.notifyNestedSubs()
+    if (previousState !== store.getState()) {
+      subscription.notifyNestedSubs()
     }
-  }
-
-  componentWillUnmount() {
-    if (this.unsubscribe) this.unsubscribe()
-
-    this.state.subscription.tryUnsubscribe()
-  }
-
-  componentDidUpdate(prevProps) {
-    if (this.props.store !== prevProps.store) {
-      this.state.subscription.tryUnsubscribe()
-      const subscription = new Subscription(this.props.store)
-      subscription.onStateChange = this.notifySubscribers
-      this.setState({ store: this.props.store, subscription })
+    return () => {
+      subscription.tryUnsubscribe()
     }
-  }
-
-  notifySubscribers() {
-    this.state.subscription.notifyNestedSubs()
-  }
+  }, [contextValue, previousState])
 
-  render() {
-    const Context = this.props.context || ReactReduxContext
+  const Context = context || ReactReduxContext
 
-    return (
-      <Context.Provider value={this.state}>
-        {this.props.children}
-      </Context.Provider>
-    )
-  }
+  return <Context.Provider value={contextValue}>{children}</Context.Provider>
 }
 
 Provider.propTypes = {
diff --git a/test/components/Provider.spec.js b/test/components/Provider.spec.js
index 1b95ce364..c8351f621 100644
--- a/test/components/Provider.spec.js
+++ b/test/components/Provider.spec.js
@@ -312,5 +312,43 @@ describe('React', () => {
       ReactDOM.unmountComponentAtNode(div)
       expect(spy).toHaveBeenCalledTimes(1)
     })
+
+    it('should handle store and children change in a the same render', () => {
+      const reducerA = (state = { nestedA: { value: 'expectedA' } }) => state
+      const reducerB = (state = { nestedB: { value: 'expectedB' } }) => state
+
+      const storeA = createStore(reducerA)
+      const storeB = createStore(reducerB)
+
+      @connect(state => ({ value: state.nestedA.value }))
+      class ComponentA extends Component {
+        render() {
+          return <div data-testid="value">{this.props.value}</div>
+        }
+      }
+
+      @connect(state => ({ value: state.nestedB.value }))
+      class ComponentB extends Component {
+        render() {
+          return <div data-testid="value">{this.props.value}</div>
+        }
+      }
+
+      const { getByTestId, rerender } = rtl.render(
+        <Provider store={storeA}>
+          <ComponentA />
+        </Provider>
+      )
+
+      expect(getByTestId('value')).toHaveTextContent('expectedA')
+
+      rerender(
+        <Provider store={storeB}>
+          <ComponentB />
+        </Provider>
+      )
+
+      expect(getByTestId('value')).toHaveTextContent('expectedB')
+    })
   })
 })

From 67e873d692745caf01a9720ac63163e5f8d78468 Mon Sep 17 00:00:00 2001
From: Michael Peyper <mpeyper7@gmail.com>
Date: Tue, 20 Aug 2019 09:56:49 +1000
Subject: [PATCH 2/2] Remove onStateChange from subscription when unsubscribing

Co-Authored-By: Mark Erikson <mark@isquaredsoftware.com>
---
 src/components/Provider.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/components/Provider.js b/src/components/Provider.js
index 2c3e5b54c..3c5ec0ea7 100644
--- a/src/components/Provider.js
+++ b/src/components/Provider.js
@@ -24,6 +24,7 @@ function Provider({ store, context, children }) {
     }
     return () => {
       subscription.tryUnsubscribe()
+      subscription.onStateChange = null
     }
   }, [contextValue, previousState])