Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use new Context API in React 16.3 #10868

Closed
oliviertassinari opened this issue Mar 31, 2018 · 18 comments
Closed

Use new Context API in React 16.3 #10868

oliviertassinari opened this issue Mar 31, 2018 · 18 comments
Labels
core Infrastructure work going on behind the scenes

Comments

@oliviertassinari
Copy link
Member

There's a new render prop/children-as-function API for context that just came in React 16.3.0.
The big thing that's relevant to us: It works across shouldComponentUpdate boundaries without the need for subscriptions.

It will require some refactoring. And backward compatibility is going to be...interesting. We may have to drop backward compatibility to make the implementation clean. FWIW, React Router and React Redux is considering this too, so we're not alone.

@leMaik
Copy link
Member

leMaik commented Apr 1, 2018

Regarding backward compatibility: https://github.com/jamiebuilds/create-react-context

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Apr 1, 2018

@leMaik I'm not sure that using create-react-context will be a wise call to make.

  • The issues are disabled, we have no "indicator" of how much "buggy" it's nor how bugs have been fixed to date.
  • The library doesn't have many downloads. It might only get traction in the long run from nerv, preact & co. users.
  • It's 1.4 kB gzipped extra in the bundle.

I start to believe the best approach would be to rely on the old context API up until React 17.0.0 land and introduce a breaking change to move to the new API. Let's wait and see what the community is doing. I don't think that we need to be bleeding edge on this topic.

@leMaik
Copy link
Member

leMaik commented Apr 1, 2018

I would prefer dropping support for React < 16.3 in 1.0.0. People will hate us, but they'll need to effectively rewrite their app anyway when they update. Also, React 16.x is very stable, API-wise, so upgrading from React 16.0 to 16.3 to 16.x should be easy.

Regarding the metrics of the package: Didn't look at that, missing issues and 1.4 kB overhead are a pretty big no-go.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Apr 1, 2018

I would prefer dropping support for React < 16.3 in 1.0.0.

@leMaik We will at least drop React < 16.0 support between 1.0.0-beta.x and 1.0.0.
It's a matter of timing. If the react team release React 17.0 in one year from now, I think that we can wait Material-UI 2.0.0 otherwise, as you say.

@dmtrKovalenko
Copy link
Member

@oliviertassinari Any updates on that? Are there any plans on dropping support of React < 16.3 for 1.0.0. Just considering moving to new context api for material-ui-pickers. But don't want to force the community migrate to React 16.3 till material-ui not.

@oliviertassinari
Copy link
Member Author

@dmtrKovalenko We will drop support for React 15.x in Material-UI v1. There is no plan for dropping React 16.0. I think that we will wait for Material-UI v2.

@TrySound
Copy link
Contributor

TrySound commented May 4, 2018

Be careful. React 16.4 will start warn about deprecated api. Waiting react 17 is a case only for application. All third party libraries has only 16.3 - 16.4 migration period.

create react context is used across many libs before dropping react < 16.3 support. PRs are open, so all possible problems are fixable.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 4, 2018

@TrySound I believe that we have fixed all the deprecation warnings introduced in React 16.3. Unless the React team plans on adding new ones, we should be good on waiting React 17. If they do, we will release Material-UI v2 sooner than expected and drop < React 16.3 support. I wish we can wait +6 months between v1 and v2. We plan on releasing Material-UI v1 May 17th. The timing is short.

@willopez
Copy link

willopez commented May 4, 2018

Hi @oliviertassinari I am using React 16.3 and MUI next beta.44 in a project and I seed the following warnings coming from ButtonBase and Modal.
warnigs

@oliviertassinari
Copy link
Member Author

@willopez Make sure you use the most up to date version of react-hot-loader.

@jedwards1211
Copy link
Contributor

jedwards1211 commented Jun 12, 2018

@oliviertassinari I'm not sure how #11361 helps. Whenever you get rid of the polyfill and use the new API, won't you have to release it as a breaking change anyway? Because if people have version 1.0.0, which doesn't use the new API, and the upgrade to a semver-compatible version that does use the API (1.something) without upgrading React, it will break their app, and the only warnings they will get are are peer dependency mismatches.

Nevermind, I didn't notice the dates on this thread, I see that React 16.3.0 is required as of version 1.0.0. Very good!

@oliviertassinari
Copy link
Member Author

@eps1lon We are pretty much done with this task no? I think that we can close.

@eps1lon
Copy link
Member

eps1lon commented Dec 1, 2018

core/styles/withStyles is still open. As far as I understood it this will require cssinjs/jss#924?

@oliviertassinari
Copy link
Member Author

@eps1lon People can already use @material-ui/styles today. It's using DI to completely replace the usage of core/styles/withStyles.

@eps1lon
Copy link
Member

eps1lon commented Dec 2, 2018

That's still alpha though. I guess it's ok since we want to migrate mui/styles anyway I suppose. We can close then.

@jedwards1211
Copy link
Contributor

@oliviertassinari I'm confused, do you consider the new React context API a form of dependency injection? Or is there a branch of material-ui/styles that uses something different?

@oliviertassinari
Copy link
Member Author

@jedwards1211 @material-ui/styles uses the new context API, that's all.

@jedwards1211
Copy link
Contributor

Cool, i was nervous when I saw "DI" 🤣

@oliviertassinari oliviertassinari added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 20, 2022
@zannager zannager added core Infrastructure work going on behind the scenes and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

No branches or pull requests

8 participants