From db383800550fd822bd8364283f4cf8da558c339f Mon Sep 17 00:00:00 2001 From: James Hollingworth Date: Sun, 16 Aug 2015 11:48:16 +0100 Subject: [PATCH 1/2] Reinvoke mapState and mapDispatch if a second argument (assumed to be props) is passed in Fixes #52 --- README.md | 4 +- src/components/createConnect.js | 34 ++++-- test/components/connect.spec.js | 204 +++++++++++++++++++++++++++++++- 3 files changed, 231 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 042ca5a6d..54c5315d6 100644 --- a/README.md +++ b/README.md @@ -226,9 +226,9 @@ Connects a React component to a Redux store. #### Arguments -* [`mapStateToProps(state): stateProps`] \(*Function*): If specified, the component will subscribe to Redux store updates. Any time it updates, `mapStateToProps` will be called. Its result must be a plain object, and it will be merged into the component’s props. If you omit it, the component will not be subscribed to the Redux store. +* [`mapStateToProps(state, [props]): stateProps`] \(*Function*): If specified, the component will subscribe to Redux store updates. Any time it updates, `mapStateToProps` will be called. Its result must be a plain object, and it will be merged into the component’s props. If you omit it, the component will not be subscribed to the Redux store. If `props` is passed in as a second argument then `mapStateToProps` will be re-invoked whenever the component receives new props. -* [`mapDispatchToProps(dispatch): dispatchProps`] \(*Object* or *Function*): If an object is passed, each function inside it will be assumed to be a Redux action creator. An object with the same function names, but bound to a Redux store, will be merged into the component’s props. If a function is passed, it will be given `dispatch`. It’s up to you to return an object that somehow uses `dispatch` to bind action creators in your own way. (Tip: you may use [`bindActionCreators()`](http://gaearon.github.io/redux/docs/api/bindActionCreators.html) helper from Redux.) If you omit it, the default implementation just injects `dispatch` into your component’s props. +* [`mapDispatchToProps(dispatch, [props]): dispatchProps`] \(*Object* or *Function*): If an object is passed, each function inside it will be assumed to be a Redux action creator. An object with the same function names, but bound to a Redux store, will be merged into the component’s props. If a function is passed, it will be given `dispatch`. It’s up to you to return an object that somehow uses `dispatch` to bind action creators in your own way. (Tip: you may use [`bindActionCreators()`](http://gaearon.github.io/redux/docs/api/bindActionCreators.html) helper from Redux.) If you omit it, the default implementation just injects `dispatch` into your component’s props. If `props` is passed in as a second argument then `mapStateToProps` will be re-invoked whenever the component receives new props. * [`mergeProps(stateProps, dispatchProps, parentProps): props`] \(*Function*): If specified, it is passed the result of `mapStateToProps()`, `mapDispatchToProps()`, and the parent `props`. The plain object you return from it will be passed as props to the wrapped component. You may specify this function to select a slice of the state based on props, or to bind action creators to a particular variable from props. If you omit it, `{ ...parentProps, ...stateProps, ...dispatchProps }` is used by default. diff --git a/src/components/createConnect.js b/src/components/createConnect.js index 5923d7093..acd3b6965 100644 --- a/src/components/createConnect.js +++ b/src/components/createConnect.js @@ -30,13 +30,18 @@ export default function createConnect(React) { wrapActionCreators(mapDispatchToProps) : mapDispatchToProps || defaultMapDispatchToProps; const finalMergeProps = mergeProps || defaultMergeProps; + const shouldUpdateStateProps = finalMapStateToProps.length >= 2; + const shouldUpdateDispatchProps = finalMapDispatchToProps.length >= 2; // Helps track hot reloading. const version = nextVersion++; - function computeStateProps(store) { + function computeStateProps(store, props) { const state = store.getState(); - const stateProps = finalMapStateToProps(state); + const stateProps = shouldUpdateStateProps ? + finalMapStateToProps(state, props) : + finalMapStateToProps(state); + invariant( isPlainObject(stateProps), '`mapStateToProps` must return an object. Instead received %s.', @@ -45,9 +50,12 @@ export default function createConnect(React) { return stateProps; } - function computeDispatchProps(store) { + function computeDispatchProps(store, props) { const { dispatch } = store; - const dispatchProps = finalMapDispatchToProps(dispatch); + const dispatchProps = shouldUpdateDispatchProps ? + finalMapDispatchToProps(dispatch, props) : + finalMapDispatchToProps(dispatch); + invariant( isPlainObject(dispatchProps), '`mapDispatchToProps` must return an object. Instead received %s.', @@ -95,15 +103,15 @@ export default function createConnect(React) { `or explicitly pass "store" as a prop to "${this.constructor.displayName}".` ); - this.stateProps = computeStateProps(this.store); - this.dispatchProps = computeDispatchProps(this.store); + this.stateProps = computeStateProps(this.store, props); + this.dispatchProps = computeDispatchProps(this.store, props); this.state = { props: this.computeNextState() }; } recomputeStateProps() { - const nextStateProps = computeStateProps(this.store); + const nextStateProps = computeStateProps(this.store, this.props); if (shallowEqual(nextStateProps, this.stateProps)) { return false; } @@ -113,7 +121,7 @@ export default function createConnect(React) { } recomputeDispatchProps() { - const nextDispatchProps = computeDispatchProps(this.store); + const nextDispatchProps = computeDispatchProps(this.store, this.props); if (shallowEqual(nextDispatchProps, this.dispatchProps)) { return false; } @@ -123,6 +131,16 @@ export default function createConnect(React) { } computeNextState(props = this.props) { + const propsHaveChanged = !shallowEqual(this.props, props); + + if (shouldUpdateStateProps && propsHaveChanged) { + this.stateProps = computeStateProps(this.store, props); + } + + if (shouldUpdateDispatchProps && propsHaveChanged) { + this.dispatchProps = computeDispatchProps(this.store, props); + } + return computeNextState( this.stateProps, this.dispatchProps, diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 0e0846e5f..22cde7d33 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -225,7 +225,7 @@ describe('React', () => { expect('x' in propsAfter).toEqual(false, 'x prop must be removed'); }); - it('should remove undefined props without mapDispatchToProps', () => { + it('should remove undefined props without mapDispatch', () => { const store = createStore(() => ({})); let props = { x: true }; let container; @@ -405,6 +405,208 @@ describe('React', () => { expect(decorated.isSubscribed()).toBe(true); }); + it('should not invoke mapState when props change if it only has one argument', () => { + const store = createStore(stringBuilder); + + let invocationCount = 0; + + @connect(() => { + invocationCount++; + return {}; + }) + class WithoutProps extends Component { + render() { + return
; + } + } + + class OuterComponent extends Component { + constructor() { + super(); + this.state = { foo: 'FOO' }; + } + + setFoo(foo) { + this.setState({ foo }); + } + + render() { + return ( +
+ +
+ ); + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + tree.refs.outerComponent.setFoo('BAR'); + tree.refs.outerComponent.setFoo('DID'); + + expect(invocationCount).toEqual(2); + }); + + it('should invoke mapState every time props are changed if it has a second argument', () => { + const store = createStore(stringBuilder); + + let propsPassedIn; + let invocationCount = 0; + + @connect((state, props) => { + invocationCount++; + propsPassedIn = props; + return {}; + }) + class WithProps extends Component { + render() { + return
; + } + } + + class OuterComponent extends Component { + constructor() { + super(); + this.state = { foo: 'FOO' }; + } + + setFoo(foo) { + this.setState({ foo }); + } + + render() { + return ( +
+ +
+ ); + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + tree.refs.outerComponent.setFoo('BAR'); + tree.refs.outerComponent.setFoo('BAZ'); + + expect(invocationCount).toEqual(4); + expect(propsPassedIn).toEqual({ + foo: 'BAZ' + }); + }); + + it('should not invoke mapDispatch when props change if it only has one argument', () => { + const store = createStore(stringBuilder); + + let invocationCount = 0; + + @connect(null, () => { + invocationCount++; + return {}; + }) + class WithoutProps extends Component { + render() { + return
; + } + } + + class OuterComponent extends Component { + constructor() { + super(); + this.state = { foo: 'FOO' }; + } + + setFoo(foo) { + this.setState({ foo }); + } + + render() { + return ( +
+ +
+ ); + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + tree.refs.outerComponent.setFoo('BAR'); + tree.refs.outerComponent.setFoo('DID'); + + expect(invocationCount).toEqual(1); + }); + + it('should invoke mapDispatch every time props are changed if it has a second argument', () => { + const store = createStore(stringBuilder); + + let propsPassedIn; + let invocationCount = 0; + + @connect(null, (dispatch, props) => { + invocationCount++; + propsPassedIn = props; + return {}; + }) + class WithProps extends Component { + render() { + return
; + } + } + + class OuterComponent extends Component { + constructor() { + super(); + this.state = { foo: 'FOO' }; + } + + setFoo(foo) { + this.setState({ foo }); + } + + render() { + return ( +
+ +
+ ); + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + tree.refs.outerComponent.setFoo('BAR'); + tree.refs.outerComponent.setFoo('BAZ'); + + expect(invocationCount).toEqual(3); + expect(propsPassedIn).toEqual({ + foo: 'BAZ' + }); + }); + it('should pass dispatch and avoid subscription if arguments are falsy', () => { const store = createStore(() => ({ foo: 'bar' From 183a59528083ca0737fe73913ad4c5fe22663e6e Mon Sep 17 00:00:00 2001 From: James Hollingworth Date: Sun, 16 Aug 2015 11:56:31 +0100 Subject: [PATCH 2/2] Improved documentation and simplified logic --- README.md | 4 ++-- src/components/createConnect.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 54c5315d6..6c4d94f74 100644 --- a/README.md +++ b/README.md @@ -226,9 +226,9 @@ Connects a React component to a Redux store. #### Arguments -* [`mapStateToProps(state, [props]): stateProps`] \(*Function*): If specified, the component will subscribe to Redux store updates. Any time it updates, `mapStateToProps` will be called. Its result must be a plain object, and it will be merged into the component’s props. If you omit it, the component will not be subscribed to the Redux store. If `props` is passed in as a second argument then `mapStateToProps` will be re-invoked whenever the component receives new props. +* [`mapStateToProps(state, [ownProps]): stateProps`] \(*Function*): If specified, the component will subscribe to Redux store updates. Any time it updates, `mapStateToProps` will be called. Its result must be a plain object, and it will be merged into the component’s props. If you omit it, the component will not be subscribed to the Redux store. If `ownProps` is passed in as a second argument then `mapStateToProps` will be re-invoked whenever the component receives new props. -* [`mapDispatchToProps(dispatch, [props]): dispatchProps`] \(*Object* or *Function*): If an object is passed, each function inside it will be assumed to be a Redux action creator. An object with the same function names, but bound to a Redux store, will be merged into the component’s props. If a function is passed, it will be given `dispatch`. It’s up to you to return an object that somehow uses `dispatch` to bind action creators in your own way. (Tip: you may use [`bindActionCreators()`](http://gaearon.github.io/redux/docs/api/bindActionCreators.html) helper from Redux.) If you omit it, the default implementation just injects `dispatch` into your component’s props. If `props` is passed in as a second argument then `mapStateToProps` will be re-invoked whenever the component receives new props. +* [`mapDispatchToProps(dispatch, [ownProps]): dispatchProps`] \(*Object* or *Function*): If an object is passed, each function inside it will be assumed to be a Redux action creator. An object with the same function names, but bound to a Redux store, will be merged into the component’s props. If a function is passed, it will be given `dispatch`. It’s up to you to return an object that somehow uses `dispatch` to bind action creators in your own way. (Tip: you may use [`bindActionCreators()`](http://gaearon.github.io/redux/docs/api/bindActionCreators.html) helper from Redux.) If you omit it, the default implementation just injects `dispatch` into your component’s props. If `ownProps` is passed in as a second argument then `mapStateToProps` will be re-invoked whenever the component receives new props. * [`mergeProps(stateProps, dispatchProps, parentProps): props`] \(*Function*): If specified, it is passed the result of `mapStateToProps()`, `mapDispatchToProps()`, and the parent `props`. The plain object you return from it will be passed as props to the wrapped component. You may specify this function to select a slice of the state based on props, or to bind action creators to a particular variable from props. If you omit it, `{ ...parentProps, ...stateProps, ...dispatchProps }` is used by default. diff --git a/src/components/createConnect.js b/src/components/createConnect.js index acd3b6965..c94de50de 100644 --- a/src/components/createConnect.js +++ b/src/components/createConnect.js @@ -30,8 +30,8 @@ export default function createConnect(React) { wrapActionCreators(mapDispatchToProps) : mapDispatchToProps || defaultMapDispatchToProps; const finalMergeProps = mergeProps || defaultMergeProps; - const shouldUpdateStateProps = finalMapStateToProps.length >= 2; - const shouldUpdateDispatchProps = finalMapDispatchToProps.length >= 2; + const shouldUpdateStateProps = finalMapStateToProps.length > 1; + const shouldUpdateDispatchProps = finalMapDispatchToProps.length > 1; // Helps track hot reloading. const version = nextVersion++;