-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
chore: start upgrade to React Router v6 #27185
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/caniuse-lite@1.0.30001600, npm/gud@1.0.0, npm/history@5.0.0, npm/mini-create-react-context@0.3.2, npm/react-router-dom@5.1.2, npm/react-router@5.1.2, npm/resolve-pathname@3.0.0, npm/value-equal@1.0.1 |
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is network access?This module accesses the network. Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
Builds ready [532f6c1]
Page Load Metrics (1714 ± 83 ms)
Bundle size diffs
|
de7cd6a
to
c477a4d
Compare
Quality Gate passedIssues Measures |
Builds ready [c477a4d]
Page Load Metrics (1833 ± 112 ms)
Bundle size diffs
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #27185 +/- ##
===========================================
- Coverage 70.00% 70.00% -0.01%
===========================================
Files 1445 1445
Lines 50195 50199 +4
Branches 14041 14048 +7
===========================================
- Hits 35139 35138 -1
- Misses 15056 15061 +5 ☔ View full report in Codecov by Sentry. |
a820d31
to
058ecc7
Compare
Builds ready [058ecc7]
Page Load Metrics (1811 ± 97 ms)
Bundle size diffs
|
058ecc7
to
4ee8ade
Compare
isEqualCaseInsensitive(address, asset) && id === tokenId.toString(), | ||
address && | ||
asset && | ||
isEqualCaseInsensitive(address, asset) && |
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.
Since this is our util, could we update it to accept optional arguments and check both internally?
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.
The route problem was basically that the newer version of useParams
started returning string | undefined
I did some research and found a better solution that removes the undefined
4ee8ade
to
f2c54dd
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.
Found a few unnecessary type casts
ui/components/multichain/pages/review-permissions-page/review-permissions-page.tsx
Outdated
Show resolved
Hide resolved
3013805
to
80ac2ea
Compare
Builds ready [80ac2ea]
Page Load Metrics (1840 ± 73 ms)
Bundle size diffs
|
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!
80ac2ea
to
e5b062e
Compare
Quality Gate passedIssues Measures |
Builds ready [e5b062e]
Page Load Metrics (1774 ± 114 ms)
Bundle size diffs
|
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.
Everything appears to be working as expected! Thank you!
Description
Following the Incremental Migration guide for React Router v6: remix-run/react-router#8753
All code changes are to fix
lint:tsc
errors.Related issues
Part of: https://github.com/MetaMask/MetaMask-planning/issues/2898
Part of: https://github.com/MetaMask/MetaMask-planning/issues/3302
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist