Skip to content

[core] Support React 17#23311

Merged
eps1lon merged 3 commits into
mui:nextfrom
eps1lon:feat/react-17-allow
Nov 2, 2020
Merged

[core] Support React 17#23311
eps1lon merged 3 commits into
mui:nextfrom
eps1lon:feat/react-17-allow

Conversation

@eps1lon

@eps1lon eps1lon commented Oct 29, 2020

Copy link
Copy Markdown
Member

Allows React 17 as a peer dependency in all packages.

Installing it in the main repo needs a bit more upstream/downstream work. Since we test the next release channel daily, it should be fairly safe. Most of the compat issues have already been resolved before 17 was released.

Closes #23306

Test plan

  1. fresh install with
    • codesandbox
    • npm (6 and 7)
    • yarn (1 and 2 v2 does not allow installs from urls i.e. can't be tested with csb deploys)

@eps1lon eps1lon added the type: new feature Expand the scope of the product to solve a new problem. label Oct 29, 2020
Comment thread package.json
"**/@babel/types": "^7.10.2",
"**/cross-fetch": "^3.0.5",
"**/dot-prop": "^5.2.0",
"**/react-is": "^16.13.1",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This avoids pulling in 16 and 17. But I haven't actually verified if yarn or yarn deduplicate are that aggressive so I'll catch up on that.

@mui-pr-bot

mui-pr-bot commented Oct 29, 2020

Copy link
Copy Markdown

No bundle size changes

Generated by 🚫 dangerJS against e9a9cdb

"@types/react": "^16.8.6",
"react": "^16.8.0",
"react-dom": "^16.8.0"
"@types/react": "^16.8.6 || ^17.0.0",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These aren't released yet. If npm and yarn don't complain on a fresh install we should get this in as well. @types/react@17 is already proposed in DefinitelyTyped/DefinitelyTyped#48971

"clsx": "^1.0.4",
"prop-types": "^15.7.2",
"react-is": "^16.8.0"
"react-is": "^16.8.0 || ^17.0.0"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This feels a bit awkward as a direct dependency but should "just work" in practice. More details why this is technically wrong can be found in react/react#20099.

@oliviertassinari

Copy link
Copy Markdown
Member

On a related note, did you have a look at the React 17 issue in #23215? I could only find one solution, but I'm not 💯% happy with it. Maybe you have some better ideas.

@eps1lon

eps1lon commented Oct 29, 2020

Copy link
Copy Markdown
Member Author

On a related note, did you have a look at the React 17 issue in #23215?

I'll take a look. Though I would've preferred if the first advise is to not use React 17 yet instead of some timer workarounds. They likely break in more subtle ways.

@eps1lon eps1lon added the on hold There is a blocker, we need to wait. label Oct 29, 2020
@eps1lon

eps1lon commented Oct 29, 2020

Copy link
Copy Markdown
Member Author

I'll investigate #23215 first. Hopefully we can write a test that fails with 17 so we can rely on our nightly tests.

@oliviertassinari oliviertassinari removed the on hold There is a blocker, we need to wait. label Oct 30, 2020
@eps1lon
eps1lon merged commit c465fb6 into mui:next Nov 2, 2020
@eps1lon
eps1lon deleted the feat/react-17-allow branch November 2, 2020 10:12
@oliviertassinari oliviertassinari changed the title [core] Allow React 17 [core] Support React 17 Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: new feature Expand the scope of the product to solve a new problem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot install @material-ui/core with npm 7 and React 17

3 participants