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.

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

Upgrade react-scripts to 5.0.1 #505

Merged
merged 15 commits into from May 2, 2023
Merged

Upgrade react-scripts to 5.0.1 #505

merged 15 commits into from May 2, 2023

Conversation

podliashanyk
Copy link
Contributor

@podliashanyk podliashanyk commented Apr 18, 2023

react-scripts 5.0.1 introduced breaking changes. Read more in create-react-app changelog.

Breaking changes relevant for Argus were:

Changes made:

NB

package.json is changed and package-lock.json is committed in this PR

@podliashanyk podliashanyk self-assigned this Apr 18, 2023
@podliashanyk podliashanyk force-pushed the bump-react-scripts branch 2 times, most recently from 58a2ef4 to b6189a4 Compare April 20, 2023 12:44
@codecov-commenter
Copy link

codecov-commenter commented Apr 20, 2023

Codecov Report

Merging #505 (cd43082) into master (7880157) will decrease coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #505      +/-   ##
==========================================
- Coverage   57.52%   57.43%   -0.09%     
==========================================
  Files          86       86              
  Lines        3597     3656      +59     
  Branches      753      812      +59     
==========================================
+ Hits         2069     2100      +31     
- Misses       1519     1547      +28     
  Partials        9        9              
Impacted Files Coverage Δ
src/api/client.ts 38.96% <ø> (-0.14%) ⬇️
src/auth.tsx 100.00% <ø> (ø)
src/components/incident/IncidentDetails.tsx 46.52% <ø> (-0.66%) ⬇️
src/components/incident/IncidentFilterToolbar.tsx 60.62% <ø> (-0.64%) ⬇️
...components/incidenttable/RealtimeIncidentTable.tsx 1.98% <ø> (-0.02%) ⬇️
...ts/notificationprofile/NotificationProfileList.tsx 43.26% <ø> (ø)
src/components/timeslot/index.tsx 89.94% <ø> (-0.19%) ⬇️
src/components/timeslotlist/index.tsx 77.41% <ø> (-0.85%) ⬇️

... and 24 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@podliashanyk podliashanyk added the dependencies Please rerun "npm install", dependencies have been changed label Apr 28, 2023
Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Curious, why so many lines with eslint-disable-next-line?

I can't get this to work against newest backend master, exception from backend when clicking on "Profiles".

Otherwise most of the changes seems to be turning off eslint.

@podliashanyk
Copy link
Contributor Author

Curious, why so many lines with eslint-disable-next-line?

Eslint rules are useful most of time, but sometimes they can be a hindrance (f.e. exhaustive deps rule in hooks makes it impossible to have a "on-mount" and "on-unmount" useEffect). But there are some eslint-disable-next-line in the code base that can be removed.

@podliashanyk podliashanyk requested a review from hmpf May 2, 2023 07:01
Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Survived quick manual test

@podliashanyk podliashanyk merged commit e0d138e into master May 2, 2023
3 checks passed
@podliashanyk podliashanyk deleted the bump-react-scripts branch May 2, 2023 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Please rerun "npm install", dependencies have been changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants