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.

Sign up for GitHub

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

[docs] Fix migration feedback #35232

Merged
merged 3 commits into from
Nov 24, 2022
Merged

[docs] Fix migration feedback #35232

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
737e5f8
[docs] Fix migration feedback
alexfauquette Nov 22, 2022
e771311
feedbacks
alexfauquette Nov 22, 2022
c3c48ac
off topic: this feels more like an info
oliviertassinari Nov 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions docs/data/material/migration/migration-v4/migration-v4.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

@oliviertassinari oliviertassinari Nov 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@oliviertassinari oliviertassinari Dec 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

react-17:
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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Screenshot 2023-01-01 at 19 57 07

It seems to have been down during the holidays period.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's bring this up with the team.


If you are using a React version below 17.0.0, update your packages to at least v14.11.2 for Material UI and v17.0.0 for React.

With npm:

```sh
npm update @material-ui/core@^4.11.2 react@^17.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't run

Screenshot 2023-02-13 at 20 43 15

I think that it should be

Suggested change
npm update @material-ui/core@^4.11.2 react@^17.0.0
npm install @material-ui/core@^4.11.2 react@^17.0.0

```

With yarn:

```sh
yarn upgrade @material-ui/core@^4.11.2 react@^17.0.0
```

### Update TypeScript

The minimum supported version of TypeScript has been increased from v3.2 to v3.5.

:::warning
:::info
We try to align with types released by [DefinitelyTyped](https://github.com/DefinitelyTyped/DefinitelyTyped) (i.e. packages published on npm under the `@types` namespace).

We will not change the minimum supported version in a major version of Material UI.

We will not change the minimum supported version in a minor version of Material UI.
However, we generally recommend not to use a TypeScript version older than the lowest supported version of DefinitelyTyped.
:::

If your project includes these packages, you'll need to update them to the `latest` version:
If your project includes these packages, you'll need to update them:

- `react-scripts`
- `@types/react`
Expand Down