[docs] Fix migration v5 docs #28530
[docs] Fix migration v5 docs #28530
Conversation
| import { ThemeProvider, createMuiTheme, makeStyles } from '@mui/material/styles'; | ||
| import { ThemeProvider, createMuiTheme, makeStyles } from '@material-ui/core/styles'; |
There was a problem hiding this comment.
This is the confusion pointed out in #28496. At this point, the developer is still using v4 so it should be @material-ui/core/styles instead of the new package.
| @@ -66,7 +66,7 @@ const useStyles = makeStyles((theme) => { | |||
| }); | |||
|
|
|||
| function App() { | |||
| const classes = useStyles(); // ❌ If you have this, consider moving <ThemeProvider> to HOC and wrap the App | |||
| const classes = useStyles(); // ❌ If you have this, consider moving it inside a component that wrapped with <ThemeProvider> | |||
There was a problem hiding this comment.
Make it consistent to what we suggest in troubleshooting
| ```sh | ||
| npm install @mui/material @mui/styles | ||
|
|
||
| // or with `yarn` | ||
| yarn add @mui/material @mui/styles | ||
| ``` | ||
|
|
||
| **Optional**: if you have one these packages, install the new package separately | ||
| - For `@material-ui/lab`, install `@mui/lab` | ||
| - For `@material-ui/icons`, install `@mui/icons-material` | ||
|
|
||
| <details> | ||
| <summary>See all packages change</summary> | ||
|
|
There was a problem hiding this comment.
Before the change it is very hard to know what should be the update. I think this is what most developer are looking for, the installation part with npm install or yarn add.
| > 📝 Please make sure that your application is still **running** without errors and **commit** the change before continuing the next step. | ||
|
|
There was a problem hiding this comment.
Because v5 packages are separated from v4, the project should not crash after the installation. Add this note to remind developer to commit before running the codemod.
| @@ -2737,3 +2762,17 @@ function App(props) { | |||
|
|
|||
| export default App; | |||
| ``` | |||
|
|
|||
| ### TypeError: Cannot read properties of undefined (reading 'pxToRem') | |||
There was a problem hiding this comment.
What's the difference with this case https://deploy-preview-28530--material-ui.netlify.app/guides/migration-v4/#makestyles-typeerror-cannot-read-property-drawer-of-undefined? It seems to be the same root issue, with different symptoms. I guess we won't add a h3 for each instance of where theme.x.y breaks 😁
There was a problem hiding this comment.
If there are more similar issues open, I will group them together. The point that I added here is to help people see the error that match their case (cover as much as possible). I don't have strong evidence to prove my thought but I also don't see a downside of adding it.
| - For `@material-ui/icons`, install `@mui/icons-material` | ||
|
|
||
| <details> | ||
| <summary>See all packages change</summary> |
There was a problem hiding this comment.
With this collapse, a developer wouldn't find what happened with @material-ui/core by searching on the page. I would advocate that @material-ui/core -> @mui/material should be made very clear, not implicit.
I don't think that it was clear before either:
"you first need to update the package" Ok, but what goes where?
There was a problem hiding this comment.
With this collapse, a developer wouldn't find what happened with @material-ui/core by searching on the page
Fair enough, I can remove the collapse.
"you first need to update the package" Ok, but what goes where?
I think this should change to "To use the v5 version of MUI Core, install these packages"
There was a problem hiding this comment.
and mention that the package names have changed (I'm sure that some people will be surprised)
|
I think |
@iamchathu No need, the lab is always in an alpha state. We don't use I have run |
Preview: https://deploy-preview-28530--material-ui.netlify.app/guides/migration-v4/
close #28477
close #28496
pxToRemof undefined #28496<summary>tag