[styles] Remove hoisting of static properties in HOCs

[eps1lon]

BREAKING: Removes call of hoist-non-react-statics in withStyles and withTheme. This does only affect the @material-ui/styles package which is still in alpha.

Migration for hoisting requirement:

import { withStyles } from '@material-ui/styles';
- export default withStyles(styles)(Component);
+ import hoistNonReactStatics from 'hoist-non-react-statics';
+ export default hoistNonReactStatics(withStyles(styles)(Component), Component);

This might be for some people a controversial change so I'm trying to explain the reasons to the best of my ability:

1. hoist-non-react-statics is not maintained by the react team
   This is/was a major pain point when migrating to refs + React.forwardRef from findDOMNode. The react team added all these new features without considering the impact it had on a very popular pattern in the ecosystem. It caused false positives [Context Api] React 16.6.0 #13465 and hard to debug errors in our docs (hard to tell where the actual issue was but hoisting forwardRef.render into a class component was somehow bad).

   I'm fully aware that the react docs explicitly say that you should copy statics over but unless a better solution is found this part of the code needs extra maintenance for every react feature when a dependency should reduce the maintenance burden as much as possible. However the text only gives relay as an example which leads me to believe that copying statics is more of an issue in facebooks code. Exporting those statics is a much better solution in my opinion.

2. It is still doable in userland with e.g. compose(hoistNonReactStatics(UnstyledComponent), withStyles(styles))(UnstyledComponent) from the recompose package. Since I suspect that the requirement for static hoisting is not a very common use case I rather give the responsibility to the consumer than to the library. Warning: recompose still uses an outdated version of hoist-non-react-statics. Use your own implementation with the latest version of hoist-non-react-statics.

3. goes against "a function should do one thing"
   For me a good example where not following this is a code smell. withStyles did two things and one of them bad.

4. Currently type information for non-react statics is lost anyway. So everybody who used flow or TypeScript didn't have proper statics support to begin with.

5. It's targeted at an alpha release
   We can safely gauge the impact of this change and get better feedback. I'm also fully expecting that this is caused by ignorance on my part since I don't think I ever relied (consciously) on hoisting statics.

6. Reduced bundle size for everyone that does not need this feature (1KB i think)

Why not inline the dependency?
While this might improve the response time it doesn't solve the actual issue. This would also increase our workload even more beyond "just bumping a dependency version".

[oliviertassinari]

Ironically, I have just remembered: styled-components/styled-components#1411 (comment) -> #9736. More food for thoughts.

[eps1lon]

@oliviertassinari So we actually depend on hoisting. I guess we manually hoist this in withStyles.

Do we need the actual name though or would a truthy property behind a symbol suffice?

[oliviertassinari]

We can hoist it manually. It's the only case I can think of that worth it. Why do you want to use a symbol? Some people set it manually too when writing custom components.

[oliviertassinari]

Should we close this pull-request? Assuming everything is working now?

[eps1lon]

I would still drop support for this since every argument I made is still valid. There is even an open PR on react-devtools facebook/react-devtools#997 that reads certain statics and I fear this will only be getting worse.

I would much prefer whitelisting internal statics for hoisting and leaving the rest to the users.

[oliviertassinari]

Right, as you see fit :)

[oliviertassinari]

It's a 50/50 🍀 chance we will have to revert this change. I hope we won't, but we are taking some risks, we will see 😎.