Skip to content

Commit

Permalink
Convert Provider into function component with hooks (#1377)
Browse files Browse the repository at this point in the history
* 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

* Remove onStateChange from subscription when unsubscribing

Co-Authored-By: Mark Erikson <mark@isquaredsoftware.com>
  • Loading branch information
mpeyper and markerikson committed Aug 20, 2019
1 parent b6b4799 commit 0c5f764
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 44 deletions.
62 changes: 18 additions & 44 deletions 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 = {
Expand Down
38 changes: 38 additions & 0 deletions test/components/Provider.spec.js
Expand Up @@ -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')
})
})
})

0 comments on commit 0c5f764

Please sign in to comment.