-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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.
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
UI: Upgrade manager to react 18 #24514
Conversation
This is just the first step. Lots of stuff borked ``` yarn up react @types/react @types/react-dom @testing-library/react ``` Immediate issues with mismatched react-dom that wasn't listed explicitly in package.json before, so ``` yarn add react-dom ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-wise LGTM! Do you want me to test something in particular @ndelangen?
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: @types/react@16.14.50, @types/react-dom@16.9.21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions, LGTM! 💪
The state merging of useAddonState needed a fix because the merger function wasn't called with the correct data, due to react18 using async rendering, making this.state be the old state.
…to norbert/react18
Replaces: #21673
Closes #20378
Closes #22217
What i did:
Upgrade react and some testing deps then fix the fallout.
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:merged
orci:daily
GH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]