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

Update frontend npm packages (admin-ui, privacy-center, cypress-e2e) #2921

Merged
merged 12 commits into from
Mar 28, 2023

Conversation

NevilleS
Copy link
Contributor

@NevilleS NevilleS commented Mar 25, 2023

Code Changes

  • First, use npm update to "safely" upgrade based on semver, etc
  • Next, use the npm-check-updates utility to selectively upgrade dependencies to latest versions, where it felt reasonable
  • Last, use npx depcheck to find unused dependencies and npm uninstall them

Steps to Confirm

  • Run all automated tests (npm test, npm run cy:run)
  • Run all CI checks
  • Manual smoke tests with nox -s "fides_env(test)"

Pre-Merge Checklist

Description Of Changes

It felt like some spring cleaning was in order, especially as we look towards some major new features in the Admin UI. This is a sweeping update of our frontend dependencies including some major version upgrades to packages like react-redux, eslint, etc.

The packages I avoided here that require more careful consideration are react, next, chakra-ui, and typescript. Some of these will likely need code migrations or breaking changes, so I didn't attempt those upgrades.

@cypress
Copy link

cypress bot commented Mar 25, 2023

Passing run #1042 ↗︎

0 3 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 39920e0 into 4e2046b...
Project: fides Commit: 5d79bc1aae ℹ️
Status: Passed Duration: 00:45 💡
Started: Mar 28, 2023 4:13 PM Ended: Mar 28, 2023 4:14 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented Mar 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4e2046b) 86.63% compared to head (39920e0) 86.63%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2921   +/-   ##
=======================================
  Coverage   86.63%   86.63%           
=======================================
  Files         299      299           
  Lines       16837    16837           
  Branches     2148     2148           
=======================================
  Hits        14587    14587           
  Misses       1841     1841           
  Partials      409      409           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TheAndrewJackson
Copy link
Contributor

I got one failure when running npm run cy:run

Screenshot 2023-03-28 at 12 38 06

User Authentication -- when the user is logged in -- lets them log out (failed) (attempt 2)

Building and checking nox -s "fides_env(test)" now

@NevilleS
Copy link
Contributor Author

That's a bit odd, because that definitely runs as part of the CI checks. Let me know what you find

@TheAndrewJackson
Copy link
Contributor

I was able to logout with the test build. I think it's probably just a flaky test. If you're not seeing/rarely seeing that failure locally I'm comfortable with merging this

@NevilleS
Copy link
Contributor Author

OK thanks- drop a review here, I'm re-running cy:run myself once more for good measure, but I think it's ship time

@NevilleS
Copy link
Contributor Author

(Side note, and maybe related... one dependency I didn't update to latest was eslint-plugin-cypress which literally released a new version a few hours ago that triggered a bunch of lint errors in our Cypress tests for "unsafe chain commands". Maybe that delinting could help... but I don't want to keep boiling the ocean here so I'm going to ship with the older linter today)

@NevilleS NevilleS merged commit de0f24f into main Mar 28, 2023
@NevilleS NevilleS deleted the ns-update-client-deps-3-25-2022 branch March 28, 2023 16:52
@allisonking allisonking mentioned this pull request Mar 28, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants