Suggestion: @connect's shouldComponentUpdate could be optional or opt-in

[dallonf]

The @connect decorator, by default, appears to add a very aggressive shouldComponentUpdate hook to the decorated component. This is great if Redux and props are the only sources of statefulness in your app - but that's not always true and leads to some seriously confusing behavior with non-obvious workarounds when it's not.

This is a known issue, of course, with react-router 0.13, but I'm having the same issue with react-intl's IntlMixin, which adds localization data to context. And I get the feeling that this is going to keep happening as long as the React ecosystem is as fragmented as it is right now.

It would be really nice to just be able to throw an option on the @connected components that I know are impure (or, similar to React core's PureRenderMixin, throw an option on the ones I know are pure; there's certainly room for debate as to whether opt-in or opt-out is the right choice here).

[gaearon]

Fair. I'm open to opt-out behavior with a { pure: false } options parameter. Because all parameters are functions, we can let it always come last (e.g. you can skip mergeProps or mapDispatchToProps). A PR is welcome.

[gnoff]

@dallonf yup this is a tricky situation. My thought is that in keeping with the paradigm that redux assumes state is not mutated we should guide people towards making more pure Components and leave the aggressive shouldComponentUpdate as is.

Given that you won't always have control over this situation though i think there are a couple of workarounds that might be worth exploring (some more hack-ish than others). I haven't tried any of these but in theory they should work

1) make your mapStateToProps, mapDispatchToProps, or mergeProps arguments return a purposely 'dirty' props object. something like

(state, ownProps) => ({
  //whatever you had before +
  forceUpdate: {}
})

This will eliminate all preformance optimizations from shouldComponentUpdate since there will always be a prop that has a new object reference and therefore should fail the shallowEqual test
this doesn't work per @dallonf 's testing.

2. develop your own 'impure' Component decorator that wraps your connected component and have it call forceUpdate on the connected component child whenever it's props change.

maybe something like this

export default function wrapWithImpure(WrappedComponent) {
  class Impure extends Component {
    componentWillUpdate() {  
      this.refs.wrappedInstance.forceUpdate();
    }
    render() {
      return <WrappedComponent ref="wrappedInstance" {...this.props} />
    }
  }
  return Impure;
}

btw, i never actually use forceUpdate so not sure if it would need to go there or in componeDidUpdate or somewhere else but essentially this should force the connected component to update on every prop change

you would use it like this

import impure from //wherever...
import connect //....

impure(connect(mapState, mapDispatch)(Component));

[gnoff]

Because all parameters are functions, we can let it always come last (e.g. you can skip mergeProps or mapDispatchToProps)

dealing with a second arg of an object of functions complicates this slightly but yeah this solution is certainly workable.

[dallonf]

Cool, I'll see if I can add an options argument (defaulting to pure: true) over the weekend. I'm actually rather fond of the forceUpdate: {} workaround (and slightly embarrassed I didn't think of it myself 😉), but probably better to have something a bit more readable in the long run. And it opens the door for more configuration options on @connect, should you ever need them...

[gnoff]

Sounds good.

[dallonf]

FYI to anyone looking at this issue, the forceUpdate: {} hack actually doesn't have the intended effect; the shouldComponentUpdate definition compares the props that were already calculated the last time there was a store update or new props were received, so it won't be able to handle updates outside the Redux flow.

Also, after diving into the implementation of a pure: false option, I think that the options argument is going to always need to be the fourth argument to @connect; trying to make it the last argument regardless of which others are present would probably lead to madness.

Anyways, I've got a PR incoming soon.