Update dependencies to support react@16.0.0

[damianobarbati]

As title says! :)

[kof]

I think we should not have react in dependencies at all, it should be peerDependencies

[kof]

@iamstarkov do you know why we use it in dependencies?

[hburrows]

ThemeProvider extends React.Component... otherwise React is only needed for the tests. So... I think it's needed as a dependency.

I created a branch and updated the dependencies to the latest version and updated enzyme to use its adapter (enzyme-adapter-react-16). All the tests are passing. The only issues are:

• some tests throw Errors and no error boundaries are defined with componentDidCatch. I think this might be fine but worthy of discussion.
• React 16 requires requestAnimationFrame which results in a warning. This is easy to shim but I can't figure out how to get ava to honor a global test setup like you can do with jest. This warning is harmless (because this package doesn't depend on raf) but it might be something that is desired to cleanup.

[kof]

ThemeProvider extends React.Component... otherwise React is only needed for the tests. So... I think it's needed as a dependency.

No, tests need devDependency, code needs peerDependency, they both can co-exist.

[hburrows]

That's correct. I was thinking peerDependency but wrote dependency. I need another cup of coffee!!

[hburrows]

@kof What needs to be done to get someone to pay attention to this? I'm happy to help out but can only do so if the maintainers are receptive. I see all the great work you're trying to do with react-jss but not keeping up with the latest version of React sends a bad signal and having a critical dependency on a project that appears abandoned doubles the concern. You can always fork this project if the maintainers have no interest in keeping it current.

[hburrows]

@iamstarkov @oliviertassinari I'm happy to help make this repo compatible with React 16. Please let me know how I can contribute. @oliviertassinari don't you care about this for material-ui v1 ?

[oliviertassinari]

@hburrows Material-UI doesn't rely on the package.

[hburrows]

@oliviertassinari material-ui v1-beta depends on react-jss@7.2.0 and that package dependents on theming@1.1.0. Doesn't that mean material-ui indirectly depends on theming? Maybe I'm misunderstanding something.... but when I build my project using react-jss with react 16 I end up with 2 version of react - 16 and 15.6.2.

[oliviertassinari]

@hburrows Right, Material-UI indirectly dependent on this package at the installation time, nothing at the runtime.

@kof I think that the best path going forward is to remove this dependency from react-jss. Allowing more flexibility and ownership on this critical part of the styling solution.

[hburrows]

@oliviertassinari Thanks for the quick response. I get what you're saying. From react-jss I use JssProvider, SheetsRegistry, and injectSheet. When I build using React 16 I get 2 versions of React - maybe JssProvider or SheetsRegistry is the culprit (I'll investigate their dependencies). I'm using JssProvider and SheetsRegistry to create printable versions of pages -- something I would continue to do with material-ui v1 unless you're providing an alternative way of getting styles after calling ReactDOMServer.renderToStaticMarkup.

@kof I'm 👍 to removing theming as a react-jss dependency and taking ownership of it directly. That would at least allow me to fork theming without also having to fork react-jss to work around this until an alternative is found.

[kof]

Theming is a separate package for a reason. It is a common effort from multiple cssinjs projects in order to unify theming. It is used by emotion and the plan was we all agreed upon to use it in styled-components and glamorous and hopefully more.

I guess the problem here is that nobody did the work of migrating as of right now.

I have publishing rights in this package, so I will fix this at some point if nobody else does. In the meantime everyone is free to fork both projects and use them temporarily.

[oliviertassinari]

@kof I can't see the win for react-jss. I'm very grateful to @iamstarkov for sharing his implementation. This saved me a lot of time with implementing a custom version for Material-UI. I have soon reached the limitation of the abstraction. By the way, I have opened three issues to provide feedback on those limitations. This plan doesn't seem to move forward (while the importance of any problem tends to increase over time):

• https://github.com/styled-components/styled-components/pull/1051
• https://github.com/paypal/glamorous/issues/175

I think that it's time to move on, it's just my humble opinion.

[kof]

I will try to get the admin rights for the repo and add more people so we can all move it forward. Otherwise we will just fork and move forward.

[iamstarkov]

i'll take care of everything

[iamstarkov]

i moved repo to cssinjs org

[iamstarkov]

so @kof is the admin now

[iamstarkov]

and ask him to add active contributors

[iamstarkov]

i think i cant handle long term oss, and im deeply sorry for you. i wish i would delegate the project earlier

[kof]

Don't worry @iamstarkov, get better and come back any time.

[hburrows]

@kof Submitted a PR (https://github.com/cssinjs/theming/pull/45) to upgrade dependencies and make compatible with React 16.