Material-ui: Migration to hooks

[joshwooding]

We've made a great start in the conversion to Function Components. This is just an issue to keep track of what we class components (public and internal) we have left. I haven't checked if it's feasible to convert these components, if it's necessary or if it's worth it.

Core

• [x] ScrollbarSize #15233
• [x] FormControl #15208
• [x] Collapse #15248
• [x] Tooltip #15291
• [x] Textarea #15331
• [x] Slide #15344
• [x] Ripple #15345
• [x] Popper #15405
• [x] SelectInput #15410
• [x] Snackbar #15504
• [x] Portal #15399
• [x] InputBase #15446
• [x] ButtonBase #15716
• [x] Popover #15623
• [x] SwipeableDrawer #15947
• [x] WithWidth #15678
• [x] Modal #16254
• [x] Tabs #16427
• [x] TouchRipple #16522

Lab

• [x] Slider #15703
• [x] SpeedDial #15737
• [x] SpeedDial Action #16386

Won't migrate

• [x] RootRef (soon deprecated)

Future work

The following items can be fully addressed once this effort is done:

• [x] Replace withTheme() with useTheme()
• [x] Remove withFormControlContext #16503
• [x] Remove withForwardedRef #16532
• [x] When forking refs prefer useForkRef over custom implementations
• [x] Move default props resolution from .defaultProps to function body #16542
• [ ] Remove React.Component usage from the demos.

[oliviertassinari]

Once it's done, we should be able to remove the withTheme() and withForwardedRef() higher order components.

[eps1lon]

Once it's done, we should be able to remove the withTheme() and withForwardedRef() higher order components.

withTheme is part of our public API. It's not very convincing to remove withTheme but keep withStyles.

[oliviertassinari]

withTheme is part of our public API. It's not very convincing to remove withTheme but keep withStyles.

To be more specific, we can stop using these higher order components internally. They are still valuable for our users, we should keep the export of the withTheme module. How is this a breaking change?

[eps1lon]

How is this a breaking change?

import { withTheme } from '@material-ui/styles'
- withTheme(); // no error
+ withTheme(); // TypeError

[oliviertassinari]

Oh ok. It's not what I had in mind. He are moving from classes to hooks one pull request at the time, minimizing the changes. We have kept some usage of render props and higher order components. We will be able to replace them :)

[eps1lon]

Alright. So remove internal usage not removing the implementation.

[eps1lon]

@joshwooding If you don't have any priority for specific components it would be nice to prioritize Tooltip (improves RootRef -> useForkRef switch) and other components that use setRef.

[joshwooding]

@joshwooding If you don't have any priority for specific components it would be nice to prioritize Tooltip (improves RootRef -> useForkRef switch) and other components that use setRef.

Luckily I've already done the majority of the Tooltip conversion :)

[gautam-pahuja]

Hi, @oliviertassinari I would like to try this for "Portal" component. Can I work on that if no one else has done it already? 😃

[oliviertassinari]

Unless @joshwooding has already done an effort in this direction, you are good to go :).

[joshwooding]

@gautam-relayr Feel free :)

[adeelibr]

Can I work on the InputBase & Snackbar component? @oliviertassinari @joshwooding

[oliviertassinari]

@adeelibr Good for me :). For the Snackbar, I think that we should apply the same fix we applied to the Modal and Popper first. By the way, we still have a couple of demos using the class function, we need to migrate them too to the function component API :).

[adeelibr]

For the Snackbar, I think that we should apply the same fix we applied to the Modal and Popper first.

Understood I'll have a look at the relevant PR's to understand what needs to be implemented for the Snackbar.

By the way, we still have a couple of demos using the class function, we need to migrate them too to the function component API :).

I must have missed them 😅 I'll have a look at it 👍

[austingardner]

Is the makeStyles API stable or is it subject to change? We currently use withStyles on an internal app here at WalmartLabs but we are eager to start using more hooks APIs.

[oliviertassinari]

@austingardner I have looked into it in #15023. I haven't seen any issue (aside from the default props + API generation). We haven't moved forward as it's not a priority for the project right now. I only wanted to evaluate the potential. It should be ✅. I hope we can automate the conversion post v4. Maybe with a codemod?

@eps1lon has found an issue in StrictMode. The style sheets might not be removed from the DOM. Given withStyles uses makeStyles internally, everybody is impacted. So it shouldn't be a concern when choosing between withStyles and makeStyles.

By the way, we have added WalmartLabs logo in https://next.material-ui.com/. Let us know if it's OK for your team :).

[oliviertassinari]

@austingardner To answer your question. No, we don't plan on changing the API of makeStyles.

[austingardner]

@oliviertassinari Thanks for the response! We'll start using it then.

[joshwooding]

Can I work on the InputBase & Snackbar component? @oliviertassinari @joshwooding

@adeelibr I'd prefer if people pick up one component at a time, but feel free to help. :)

[adeelibr]

@joshwooding I agree, I am working on Snackbar component right now. Once that is done. Only then i'll move to the other component.

[zhoufeng1989]

Hi, I would like to work on Modal if you guys haven't started yet @joshwooding @oliviertassinari .

[joshwooding]

@zhoufeng1989 Modal might be a quite difficult one, due to it needing to reference itself when using ModalManager.
e.g. https://github.com/mui-org/material-ui/blob/1bff0f7741fc2838f94d2143fb78ea89a49accbc/packages/material-ui/src/Modal/Modal.js#L85

Feel free to give it a go though :)

[zhoufeng1989]

@joshwooding , thanks for your reminding. I find it is quite tricky after some investigations, and as a newbie here I would like to contribute to other components for this issue, then maybe I will do this one after I get more familiar with this project. Do you have any suggestions? Thanks.

[joshwooding]

@zhoufeng1989 We’re trying to save lab for last but for a beginner, maybe SpeedDial or SpeedDialAction. InputBase in core should be okay. You could also look at swapping withTheme with useTheme :)

[zhoufeng1989]

Thanks for the guide @joshwooding , will try to work on InputBase.

[joshwooding]

@zhoufeng1989 Looks like @adeelibr is working on InputBase first instead of Snackbar, sorry! I would have a look at SpeedDialAction

[jacobbogers]

Hi, I was working on #14101, I will make PR this weekend, sorry for delay

[jacobbogers]

Overview so far
I have placed in package material-ui-styles , (if it needs to be elsewhere let me know)

• useWidth(theme) hook (analog to useTheme , but doesnt use a React.createContext)
• WidthProvider (analog to ThemeProvider, passes the width value down to child components who need width info (avoids multiple window.matchMedia usage) via React.createContext
• useWidthContext (hook to be used in conjuction with WidthProvider)

A question how do add unit tests using window.matchMedia in a headless way?

[jacobbogers]

the commit so far https://github.com/cloned-repos/material-ui/commit/58605dd8b0fb5afd2dc4b7932c7db5edd30e8bdb

demo of the components https://github.com/jacobbogers/benchmark-mobile

[jacobbogers]

ok, I analyzed your source code and use of npm module css-mediaquery used for SSR and mocha tests
I know now how to create the unit tests,

[oliviertassinari]

I'm taking care of the Slider.

[jacobbogers]

Some issues,
I am implementing this in a fork from master or should i have used next branch???

I put the hooks and contextProvider (for width) in the package material-ui-styles

running tests now, i get the following errors
Missing modules

• expect.js
• zen-observable
• detect-browser

After installing all these modules with npm , (in the project root)

I run npm run test in directory /packages/material-ui-styles
i get the error

C:\Users\jacob\repos\ui\material-ui\packages\material-ui-styles\node_modules\jss-vendor-prefixer\lib\index.test.js:34
var isIE9 = _detectBrowser2['default'].name === 'ie' && _detectBrowser2['default'].version === '9.0.0';
                                       ^

TypeError: Cannot read property 'name' of undefined

[joshwooding]

@jacobbogers next should be used.

You need to use yarn to install the dependencies. We recently updated our documentation, but it looks like the note was only added in the docs/README.md. Sorry for the inconvenience.

[jacobbogers]

Thanks, yes, I remember I came accorss this when I installed material-ui for the first time, sorry to have forgotten.

[jacobbogers]

Can someone check out my PR so far?
Havent updated the docs yet, just wanted some feedback first https://github.com/mui-org/material-ui/pull/15678

[jeongsd]

Hi, Can I work on the ButtonBase component?

[joshwooding]

@jeongsd ButtonBase is going to be one of the hardest components to migrate. I would start looking at SpeedDialAction instead

[adeelibr]

Hi, Can I work on SwipeableDrawer?

[oliviertassinari]

@adeelibr A previous effort started in https://github.com/mui-org/material-ui/pull/15469#issuecomment-492122568.

[adeelibr]

Is SpeedDial Action available to work on? 😄 @oliviertassinari

[jeongsd]

Thanks for the guide @joshwooding, can I work on SpeedDial?

[bpas247]

Remove React.Component usage from the demos.

Is this task still up for grabs? I wouldn't mind taking it, if it's open 😃

[oliviertassinari]

Unless @joshwooding has something in store for it. It's up for grab :).

[joshwooding]

@bpas247 No plans here :) Feel free!

[bpas247]

Hmm looking at the doc examples right now, it looks like most of the component examples have already migrated to function components.

Are there any that haven’t? Or am I looking in the wrong directory for what the task is relevant to?

[oliviertassinari]

This is one way to find them: https://github.com/mui-org/material-ui/search?l=JavaScript&p=1&q=%22React.Component%22.