-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: update react peer dependency to react@18 #1624
Conversation
00a1027
to
5f4c3bc
Compare
"@dhis2/app-runtime": "^3.9.0", | ||
"@dhis2/cli-app-scripts": "^11.7.0", | ||
"@dhis2/app-runtime": "^3.11.2", | ||
"@dhis2/cli-app-scripts": "^12.0.0-alpha.19", |
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.
not sure how to manage this - I guess we should keep it as the version in master, until it's merged and bump it to v12? or leave it as alpha and bump it later (it's a dev dependency )
🚀 Deployed on https://pr-1624--dhis2-ui.netlify.app |
5f4c3bc
to
b5f15ea
Compare
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.
LGTM
- Storybook works locally
-
yarn install && yarn setup && yarn build
works locally, even in a newly cloned repository - Tests pass
@Mohammer5 @amcgee I am re-requesting review because I did some significant changes here - the tests were failing after the upgrade, part of it could be solved by using the enzyme's unofficial adapter for React 18, but I thought we should bump testing-library as well since we're doing this .. this trickled to a lot of changes related to userEvent becoming async and the different way hooks are tested now. All these changes fall under the ones we documented for v12 migration guide here. |
sorry folks, this was on auto-merge :/ |
# [9.13.0](v9.12.0...v9.13.0) (2024-11-05) ### Bug Fixes * peer dependency issue with npm publish ([#1628](#1628)) ([1319654](1319654)) * **translations:** sync translations from transifex (master) ([7f22330](7f22330)) ### Features * update react peer dependency to react@18 ([#1624](#1624)) ([5d3c2a4](5d3c2a4))
# [10.0.0-alpha.8](v10.0.0-alpha.7...v10.0.0-alpha.8) (2024-11-21) ### Bug Fixes * peer dependency issue with npm publish ([#1628](#1628)) ([1319654](1319654)) * update calendar tests for react 18 ([98831a7](98831a7)) * update testing-library for selector-bar ([893024d](893024d)) * **translations:** sync translations from transifex (master) ([d89ce94](d89ce94)) * **translations:** sync translations from transifex (master) ([7f22330](7f22330)) ### Features * add data sharing to sharing dialog [LIBS-677] ([#1629](#1629)) ([7e15c7f](7e15c7f)) * make input field clearable and add prefix icon ([#1619](#1619)) ([7f87fb4](7f87fb4)) * update react peer dependency to react@18 ([#1624](#1624)) ([5d3c2a4](5d3c2a4))
Implements LIBS-703
Description
In preparation for bumping react version in app-platform, we need to bump the peer dependency of the UI - it works without it but generates a lot of unnecessary warnings.
Tested locally by ensuring
yarn why react
only shows react@18 in UI, and by adding the library throughyalc
, there were no warnings about the peer dependencies anymore inapp-scripts@alpha
(I had the peer dependencies as>=16.13
first, and that didn't work, but changing it to^16.13 || ^18
worked)