[Core] remove context-pure mixin from components

[yongxu]

This mixin appears in many components, checking if context.muiTheme has changed.

First of all, I do not see why this component is necessary besides supporting dynamic theme. This can be done by creating a new theme context instance, React will then tell it has been changed.

Moreover, Its appearance may significantly affect the performance since it does checking in shouldComponentUpdate every time react does reconciliation. Especially, this happens for every flat button and text field.

Last, but may not relate to this issue, context-pure mixin uses src/utils/shallow-equal.js, the same function has been implemented as react addon react-addons-shallow-compare. I think we should remove it from utils.

Here is the sublime search result:

Edit by @newoga (replaced @yongxu sublime search results with checklist)

• flat-button-label.jsx ([Core] Remove context-pure mixin #3331)
• card-expandable.jsx ([Core] Remove context-pure mixin #3331)
• date-picker-dialog.jsx ([Core] Remove context-pure mixin #3331)
• flat-button.jsx ([Core] Remove context-pure mixin #3331)
• icon-button.jsx ([Core] Remove context-pure mixin #3331)
• SelectField.jsx ([Core] Remove context-pure mixin #3331)
• snackbar.jsx ([Core] Remove context-pure mixin #3331)
• TextField.jsx ([Core] Remove context-pure mixin #3331)

[yongxu]

@hai-cea @oliviertassinari @shaurya947 , any idea?

[oliviertassinari]

That's related to #1176.
@yongxu You are right, we should address this issue.
I see three options:

• We could be following react-bootstrap and let users implement the pure behavior
• We can continue what we have started some time ago and implement the pure behavior at the root of all our components
• As we are breaking our components into multiple stateless components. We can let the root, stateful, component without the pure behavior and only add it the children stateless components.

[yongxu]

IMO, we should treat context.muiTheme as pure, for the components and its child that may require dynamic theme change can reload new theme in getChildContext. It is quite easy to do and we can just let users to decide. @oliviertassinari

[newoga]

First of all, I do not see why this component is necessary besides supporting dynamic theme.

@yongxu To be more precise. context-pure isn't only for context. It actually reimplements the PureRenderMixin checks in addition to checking the muiTheme in context.

@oliviertassinari I agree that those are the 3 options. Do you have a gut feeling which option is best? I suppose a hybrid option between 1 and 2 is to change the shouldComponentUpdate implementation of all the components that use ContextPure to just do a shallow compare of props and state. Then users would have to implement something for pure context if they want it.

@alitaheri Thoughts on any of this?

[alitaheri]

Well as I've said before (and we all agreed) We should try to make our components as stateless as possible. let the users and HOCs handle state. and for context, Recompose is a very good answer. We can use it to handle most of our performance issues. But unfortunately we have to get rid of imperative methods and all the code duplication with theme handling before we can do this. I've been studying Recompose for quite a while now. I think it's the naswer to many of our problems (except imperative methods which we will have to proxy through somehow). Besides if theme is gathered by an HOC and passed down as prop we won't need this mixin anymore. and if we reduce state. many of the stateless components can use Recompose's pure function 😁

[newoga]

@oliviertassinari @alitaheri What do you think the best strategy is to remove the context-pure dependency from these components? Should we simply remove it since we're moving to muiTheme as prop soon and we plan on implementing it a different way like recompose's pure?

[alitaheri]

Eradicate 👊 💥

[newoga]

😈

[oliviertassinari]

What do you think the best strategy is to remove the context-pure dependency from these components?

That definitely sounds like the best strategy 👍. Then, we can use the chain order proposed by @alitaheri #3268.

[newoga]

Sounds good, I'm gonna do a PR for the final remaining window-listenable components and then we can start removing this