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

[withWidth] Remove the API #17350

Closed
Bouncue opened this issue Sep 7, 2019 · 14 comments · Fixed by #26136
Closed

[withWidth] Remove the API #17350

Bouncue opened this issue Sep 7, 2019 · 14 comments · Fixed by #26136

Comments

@Bouncue
Copy link

Bouncue commented Sep 7, 2019

The docs says:

withWidth()

This higher-order component will be deprecated for the useMediaQuery hook.

Will this really happen? when and in what version?

I have many class components which mean cannot use useMediaQuery hook. But I really need to keep watch on screen size changes.

Also if Material-UI decides to get rid of withWidth() then why still use it in the demo? as in Responsive Drawer

This makes me confuse unsure to decide, should I just use withWidth() in my class components or should I convert them into functional component -which means a lot of work- to use useMediaQuery.

Does Material-UI plans to drop support for the class component?

@oliviertassinari oliviertassinari added this to the v5 milestone Sep 7, 2019
@oliviertassinari
Copy link
Member

@Bouncue You can find more detail on this topic under #14101. To sum up the discussion, the most likely path forward is to remove withWidth() and to introduce a withMediaQuery() API.

Does Material-UI plans to drop support for the class component?

We will support class components as long as React do so. Note that moving a hook to a higher-order component is relatively straightforward:

import useMediaQuery from '@material-ui/core/useMediaQuery';

const withMediaQuery = (...args) => Component => props => {
  const mediaQuery = useMediaQuery(...args);
  return <Component mediaQuery={mediaQuery} {...props} />;
}

export default withMediaQuery;

Does it help?

Also if Material-UI decides to get rid of withWidth() then why still use it in the demo? as in Responsive Drawer

I can't find any demo on the drawer page that uses this API. Where did you spot it? We should remove it.

@Bouncue
Copy link
Author

Bouncue commented Sep 7, 2019

Thanks I'll try it.

I can't find any demo on the drawer page that uses this API. Where did you spot it? We should remove it.

In Responsive Drawer, the Hidden component is used:

import Hidden from '@material-ui/core/Hidden';

As far as I understand the component uses withWidth() where the docs says:

responsively hiding content based on using the withWidth() higher-order component that watches screen size.

@oliviertassinari
Copy link
Member

@Bouncue Thanks for highlighting this issue, the Hidden component is no longer using withWidth, it was migrated to useMediaQuery some time ago. We will need to solve this inconsistency.

@tlenex
Copy link

tlenex commented Apr 14, 2020

I have a question for this. As far as I understand, built-in react hooks are not meant to be used with class components. Will useMediaQuery be usable in class components? What will be the alternative if not?

@oliviertassinari
Copy link
Member

@tlenex If there is enough request, we can make either:

  • withMediaQuery high-order component
const withMediaQuery("(prefers-color-scheme: dark)")(MyComponent);
  • or a render prop MediaQuery component
<MediaQuery query="(prefers-color-scheme: dark)">
  {(matches) => matches}
</MediaQuery>

@tlenex
Copy link

tlenex commented Apr 14, 2020

Thank you. I'd like to vote for support of both react component formats. Pretty please!

Personally, I like to use class components for business logic (controllers/containers) and functional components for visualizations. My point is that sometimes business logic can depend on media queries - in that case material-ui will force me to rewrite classes to functions. More over, react team does not plan to deprecate class components as far as I know.

@NMinhNguyen
Copy link
Contributor

in that case material-ui will force me to rewrite classes to functions.

This is just my opinion, but you wouldn't need to rewrite class components to function components - you could just define a function component wrapper that uses the useMediaQuery hook and passes props to your class component as shown in #17350 (comment)

@eps1lon
Copy link
Member

eps1lon commented Apr 8, 2021

What are some use cases for knowing the current breakpoint other than our internal usage in Hidden?

@oliviertassinari
Copy link
Member

@eps1lon I believe we will need an internal useBreakpoint() hook (returning the current breakpoint) in order to implement: useBreakpointValue (#23885).

@eps1lon
Copy link
Member

eps1lon commented Apr 8, 2021

@eps1lon I believe we will need an internal useBreakpoint() hook (returning the current breakpoint) in order to implement: useBreakpointValue (#23885).

So can we close one of these issues?

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 8, 2021

What do you mean by "these issues"? From what I understand:

@eps1lon
Copy link
Member

eps1lon commented Apr 8, 2021

So if #17350 and #23885 are different issues, why did you bring up #23885 here?

These are either coupled or they're not.

in order to close #17350, the best course of action is to introduce a new useBreakpoint() hook. It will completely solve the migration concern.

I'm asking what is supposed to be migrated. How is this function used (I'm not interested in internal usage since that is separate from a public API)?

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 11, 2021

why did you bring up #23885 here?

@eps1lon The objective was to raise awareness of the potential dependency between the two issues. Considering the problem holistically could yield a better solution. #23885 might need #17350 to be fixed first.


Regarding this issue and the core problem we might solve for developers. So far, I have seen 3 prior-art with the useBreakpoint (same as useWidth):

We have been deprecating withWidth for a long-time, I imagine developers have been looking for alternatives since they get a warning in the docs. Browsing and commenting on #17350 is one of these alternatives. We didn't get many interactions from developers in this issue. Also, I believe that useMediaQuery and #23885 can cover the same use cases, with a more powerful API.

A new proposal for the course of action:

  1. We don't introduce useBreakpoint/useWidth in the public API, if we need it, it will stay private.
  2. We solve Add useBreakpointValue hook #23885, we could have "no arguments" return the current breakpoint.
  3. We remove withWidth and https://next.material-ui.com/customization/breakpoints/#withwidth
  4. We update the migration guide to encourage useMediaQuery or useBreakpointValue.

cc @mui-org/core

@oliviertassinari oliviertassinari changed the title Deprecation plan of withWidth() for the useMediaQuery hook. [withWidth] Remove the API Apr 11, 2021
@oliviertassinari
Copy link
Member

Done in #26136 by @m4theushw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants