2

Suppose a stateless, functional UserProfile component that displays user data for the given url. Suppose it is being wrapped with connect(mapStateToProps, mapDispatchToProps)(UserProfile). Finally, suppose a reducer that reduces into state.userProfile. Anytime the url changes, I need to re-initialize the state.userProfile, so a solution that comes to mind is to do so from within the mapDispatchToProps like so:

function mapDispatchToProps(dispatch, ownProps) {
  dispatch(fetchUser(ownProps.userId))
  return {
    ...
  }
}

Provided that the thunked fetchUser ignores repeated calls by comparing with current state, is this an acceptable practice? Or are there problems associated with calling dispatch immediately from this map function?

Dan Abramov
  • 264,556
  • 84
  • 409
  • 511
Bijou Trouvaille
  • 8,794
  • 4
  • 39
  • 42

1 Answers1

5

This is unsupported and can break at any time.
mapDispatchToProps itself should not have side effects.

If you need to dispatch actions in response to prop changes, you can create a component class and use lifecycle methods for this:

class UserProfile extends Component {
  componentDidMount() {
    this.props.fetchUser(this.props.id)
  }

  componentDidUpdate(prevProps) {
    if (prevProps.id !== this.props.id) {
      this.props.fetchUser(this.props.id)
    }
  }

  // ...

}
Dan Abramov
  • 264,556
  • 84
  • 409
  • 511
  • Technically it is still a pure function, as far I understand, but thinking more about it I can imagine circular dispatch situations. Thanks for the confirmation. – Bijou Trouvaille Apr 15 '16 at 20:10
  • 1
    If it calls `dispatch`, it isn’t pure—that’s a side effect. – Dan Abramov Apr 15 '16 at 20:13
  • I suppose it's an question of terminology, but if you go by wikipedia's definition, a pure function "always evaluates the same result value given the same argument value(s)." Perhaps there's another definition that you have in mind. – Bijou Trouvaille Apr 15 '16 at 20:17
  • 2
    From Wikipedia: “Evaluation of the result does not cause any semantically observable side effect”. Dispatching is a side effect because it changes the global (atomically mutable) state of the app. – Dan Abramov Apr 15 '16 at 20:19
  • For example, `alert()` or `xhr.send()` are not pure even though they return the same value (`undefined`) on every call. – Dan Abramov Apr 15 '16 at 20:20
  • alert modifies the ui state, and xhr modifies the state of the network. dispatch is just a parameter. But you are still correct, if you take to the "semantic" definition. Semantically we expect dispatch to be a specific function, even though it doesn't have to be. – Bijou Trouvaille Apr 15 '16 at 20:34
  • 1
    Another reason not to dispatch in `mapDispatchToProps` is that you now coupled your component to redux entirely and made it impossible to run unit tests on. If you were to test the non-connected component, it would be missing the bit of logic about mounting and unmounting. – ZekeDroid Apr 15 '16 at 20:54
  • 1
    I don't mean to be argumentative. I appreciate very much the input, but this has been something that I keep coming back to in my code. In my unit tests I don't want to test _that_ "bit of logic". I want the stateless component to be agnostic of where it's getting its data from. I almost wish for a third map function that exposes the lifecycle events from the `connect`'s internal component. – Bijou Trouvaille Apr 15 '16 at 21:02
  • 1
    Well in those cases you should be making a container component that can do the fetch, and then have a presentational component that's not connected at all. Then it's easy to unit test without that bit of logic since the container is the only one containing it. So just ignore it. – ZekeDroid Apr 15 '16 at 21:18