[docs] Fix migration feedback #35232
Conversation
|
| @@ -71,19 +71,36 @@ If you need to support IE 11, check out our [legacy bundle](/material-ui/guides/ | |||
|
|
|||
| ## Update React & TypeScript version | |||
|
|
|||
| ### Update React | |||
|
|
|||
| The minimum supported version of React has been increased from v16.8.0 to v17.0.0. | |||
There was a problem hiding this comment.
Off-topic
@mui/core I almost wonder if we shouldn't have moved in this direction: #25464. Most seems to keep the support of React 16.8.0
- https://github.com/ant-design/ant-design/blob/c2f75a2ba3054f67041810aa8d172ab5965b21b6/package.json#L309
- https://github.com/react-bootstrap/react-bootstrap/blob/bd016aa330b005073f79ec1439ad3fef5eb021b4/package.json#L150
- https://github.com/radix-ui/primitives/blob/f0a7495bcd00faa14164b2f80721149238c85d9d/packages/react/context-menu/package.json#L28
- https://github.com/adobe/react-spectrum/blob/810579b671791f1593108f62cdc1893de3a220e3/packages/react-aria/package.json#L63
There was a problem hiding this comment.
Honestly, if we support 17, I don't see why we shouldn't support 16.8. But we would need to probably add additional tests for it. I wonder if people still use it honestly
There was a problem hiding this comment.
I wonder if people still use it
@mnajdova based on https://www.npmjs.com/package/react-dom?activeTab=versions I could cover an extra 7797292 weekly downloads of React DOM (downloads >=v16.8.0 && < v17.0.0). So with Material UI v5, we exclude 45% of the today's React DOM downloads.
But we would need to probably add additional tests for it.
We have tests for React 17 in
material-ui/.circleci/config.yml
Line 832 in 90d6a3b
There was a problem hiding this comment.
Which have been broken for at least 3 months https://app.circleci.com/insights/github/mui/material-ui/workflows/react-17/overview?branch=master&reporting-window=last-90-days
Probably this means that it was a bad decision on my side to create this check as "non blocking" on the PR CI.
@mnajdova based on https://www.npmjs.com/package/react-dom?activeTab=versions I could cover an extra 7797292 weekly downloads of React DOM (downloads >=v16.8.0 && < v17.0.0). So with Material UI v5, we exclude 45% of the today's React DOM downloads.
I am not sure that this is that much black/white. If someone is not aiming to update React, it's likely that they are not going to update their UI library dependency either, so they may be depending on v4. But agree, it is a big percentage, so it may be worth checking what would take for us to support v16.8.0.
There was a problem hiding this comment.
Probably this means that it was a bad decision on my side to create this check as "non blocking" on the PR CI.
@mnajdova I think that a nice workaround could be to run this step randomly on PRs. This way, we get the feedback every now and then, without slowing down the CI too much.
so they may be depending on v4.
This could be why v4 is still this high in downloads, too many have to first upgrade React, but they might not because some other libraries are not compatible with React 17 yet. I have created a live app with Toolpad to better visualize v16.8 use, with live data: https://master--toolpad.mui.com/deploy/clcdobvof0008n99jpx86fede/pages/331kqzd
It seems to have been down during the holidays period.
There was a problem hiding this comment.
Ok, let's bring this up with the team.
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
| With npm: | ||
|
|
||
| ```sh | ||
| npm update @material-ui/core@^4.11.2 react@^17.0.0 |
There was a problem hiding this comment.
This comment doesn't run
I think that it should be
| npm update @material-ui/core@^4.11.2 react@^17.0.0 | |
| npm install @material-ui/core@^4.11.2 react@^17.0.0 |
Fix doc feedback about https://mui.com/material-ui/migration/migration-v4/
From @oliviertassinari
https://deploy-preview-35232--material-ui.netlify.app/material-ui/migration/migration-v4/#update-react-amp-typescript-version