Convert Provider into function component with hooks (reduxjs#1377)

* 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 reduxjs#1370

* Remove onStateChange from subscription when unsubscribing

Co-Authored-By: Mark Erikson <[email protected]>

Author: mpeyper

src/components/Provider.js

@@ -1,62 +1,36 @@
-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()
+      subscription.onStateChange = null
     }
-  }
-
-  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 = {

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')
+    })
   })
 })