Reinvoke mapState and mapDispatch if a second argument (assumed to be…

… props) is passed in Fixes #52
reduxjs · Aug 16, 2015 · db38380 · db38380
1 parent 27aebae
commit db38380
Show file tree

Hide file tree

Showing 3 changed files with 231 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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 <div {...this.props}/>;
+        }
+      }
+
+      class OuterComponent extends Component {
+        constructor() {
+          super();
+          this.state = { foo: 'FOO' };
+        }
+
+        setFoo(foo) {
+          this.setState({ foo });
+        }
+
+        render() {
+          return (
+            <div>
+              <WithoutProps {...this.state} />
+            </div>
+          );
+        }
+      }
+
+      const tree = TestUtils.renderIntoDocument(
+        <Provider store={store}>
+          {() => (
+            <OuterComponent ref='outerComponent' />
+          )}
+        </Provider>
+      );
+
+      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 <div {...this.props}/>;
+        }
+      }
+
+      class OuterComponent extends Component {
+        constructor() {
+          super();
+          this.state = { foo: 'FOO' };
+        }
+
+        setFoo(foo) {
+          this.setState({ foo });
+        }
+
+        render() {
+          return (
+            <div>
+              <WithProps {...this.state} />
+            </div>
+          );
+        }
+      }
+
+      const tree = TestUtils.renderIntoDocument(
+        <Provider store={store}>
+          {() => (
+            <OuterComponent ref='outerComponent' />
+          )}
+        </Provider>
+      );
+
+      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 <div {...this.props}/>;
+        }
+      }
+
+      class OuterComponent extends Component {
+        constructor() {
+          super();
+          this.state = { foo: 'FOO' };
+        }
+
+        setFoo(foo) {
+          this.setState({ foo });
+        }
+
+        render() {
+          return (
+            <div>
+              <WithoutProps {...this.state} />
+            </div>
+          );
+        }
+      }
+
+      const tree = TestUtils.renderIntoDocument(
+        <Provider store={store}>
+          {() => (
+            <OuterComponent ref='outerComponent' />
+          )}
+        </Provider>
+      );
+
+      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 <div {...this.props}/>;
+        }
+      }
+
+      class OuterComponent extends Component {
+        constructor() {
+          super();
+          this.state = { foo: 'FOO' };
+        }
+
+        setFoo(foo) {
+          this.setState({ foo });
+        }
+
+        render() {
+          return (
+            <div>
+              <WithProps {...this.state} />
+            </div>
+          );
+        }
+      }
+
+      const tree = TestUtils.renderIntoDocument(
+        <Provider store={store}>
+          {() => (
+            <OuterComponent ref='outerComponent' />
+          )}
+        </Provider>
+      );
+
+      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'