Skip to content

Upgrade React to v18.2.0#35248

Merged
ryanclark merged 13 commits intomasterfrom
ryan/react-18
Dec 6, 2023
Merged

Upgrade React to v18.2.0#35248
ryanclark merged 13 commits intomasterfrom
ryan/react-18

Conversation

@ryanclark
Copy link
Copy Markdown
Member

@ryanclark ryanclark commented Dec 1, 2023

This upgrades React to v18.2.0.

Most of this was type updates (adding PropsWithChildren everywhere) and the removal of @testing-library/react-hooks as it doesn't support React 18 (and @testing-library/react has renderHook anyway)

Breaking changes in v17 and v18:

Closes #21098

@ryanclark ryanclark added the no-changelog Indicates that a PR does not require a changelog entry label Dec 1, 2023
@github-actions github-actions Bot requested review from gzdunek and kimlisa December 1, 2023 14:27
Copy link
Copy Markdown
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

Wow, what a herculean effort.

I'll take a look on Monday, now I just submitted some quick improvements which should also help with any testing.

Comment thread yarn.lock
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

First thing that caught my attention was some dupes of babel packages because of Yarn's innacurate dedupe algorithm.

I ran npx yarn-deduplicate yarn.lock --scopes @babel and made a bunch of other small changes on my branch if you'd like to include them (#35249).

Many of those operations require running yarn again for some weird reason, I suspect yarn just has some bugs which make it fundamentally unable to be idempotent when using workspaces. 🙄

Comment thread package.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yarn install reports that packages like react-select now have an incorrect peer dependency, did you have a chance to check if those work correctly with React v18?

@ravicious
Copy link
Copy Markdown
Member

ravicious commented Dec 1, 2023

TODO list for myself:

@ravicious ravicious self-requested a review December 1, 2023 15:01
Copy link
Copy Markdown
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

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

Could you search for "react", "react-dom" and "@types/react" packages in all package.json and update their versions too? In some files the versions differ.

Comment thread web/packages/teleport/src/Discover/Shared/Header.tsx Outdated
Comment thread web/packages/teleport/src/components/Authenticated/Authenticated.tsx Outdated
Comment thread web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/ClusterLogin.story.tsx Outdated
Comment thread web/packages/teleterm/src/ui/DocumentCluster/resourcesContext.tsx Outdated
Comment thread web/packages/teleterm/src/ui/Search/SearchContext.tsx Outdated
Comment thread web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx Outdated
Copy link
Copy Markdown
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

Made it to web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/EnrollRdsDatabase.test.tsx, will continue tomorrow.

@ravicious ravicious self-requested a review December 4, 2023 16:01
@gzdunek
Copy link
Copy Markdown
Contributor

gzdunek commented Dec 5, 2023

It looks like some changes weren't committed to yarn.lock, I see updates in it after running yarn install.

Comment thread package.json
ryanclark and others added 7 commits December 6, 2023 11:01
@ryanclark ryanclark enabled auto-merge December 6, 2023 10:41
@ravicious ravicious self-requested a review December 6, 2023 10:54
Copy link
Copy Markdown
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this!

Copy link
Copy Markdown
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

Connect works properly in dev and prod modes, Storybook works without problems as well.

🚀

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from kimlisa December 6, 2023 13:24
@ryanclark ryanclark added this pull request to the merge queue Dec 6, 2023
Comment thread package.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we keep this on master only? Ofc backporting would make any future work easier, but idk how much work a backport would take. With v15 just around the corner we might be able to get away with not backporting if we wish to.

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.

Yeah I think it's best to keep it on just master, also means React 18 will go through the test plan before reaching customers

Merged via the queue into master with commit f36e075 Dec 6, 2023
@ryanclark ryanclark deleted the ryan/react-18 branch December 6, 2023 13:36
@ravicious
Copy link
Copy Markdown
Member

eslint-plugin-jest-dom update (#35926) should get backported too if we backport this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/md ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update react to v18

3 participants