-
Notifications
You must be signed in to change notification settings - Fork 35
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
MB-15974 - upgrades react-router and react-router-dom versions #10805
Conversation
Bundle StatsHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
c91655c
to
6205619
Compare
JIRA Ticket: MB-15974
We can't update to the latest versions because it breaks our react-admin site. Per their troubleshooting docs, https://marmelab.com/react-admin/Routing.html#troubleshooting, we need to match the versions they're using, so we need to set resolutions. A bit of a pain because now we'll need to keep them in sync. JIRA Ticket: MB-15974
Needed for the react-router update because of [ #9506 - Fix inadvertent support for partial dynamic parameters](remix-run/react-router#9506). JIRA Ticket: MB-15974 Co-authored-by: Kyle Hill <[email protected]>
6205619
to
18ef91c
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.
I tested that the URL update is working as intended in the office app and that admin is working as well. Both LGTM!
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. I pulled down the branch and ran officelocal and adminlocal. The new route shows in Officelocal and adminlocal works fine.
Jira ticket
Summary
Upgrades react-router and react-router-dom versions. Replaces #10777 and #10754.
We can't update to the latest versions because it breaks our
react-admin
site. Perreact-admin
's troubleshooting docs, we need to match the versions they're using, so we need to set resolutions. A bit of a pain because now we'll need to keep them in sync.This update also requires an update to some of our FE routes because the maintainers removed support for partial dynamic parameters. Kyle wrote up some info in a post to inform the design and product teams (post was the same and as of now, only the design one has non-emoji comments on it). Here it is for ease:
Release Notes & Changelog
Release notes
Sourced from react-router's releases.
... (truncated)
Changelog
Sourced from react-router's changelog.
... (truncated)
Commits
0815c96
chore: Update version for release (#10500)f1e4db4
chore: Update version for release (pre) (#10495)e665a46
Fix basename duplication in RouterProvider descendant routes (#10492)db696c1
chore: Update version for release (#10443)b725c3b
chore: Update version for release (pre) (#10436)c4e9607
Fix usage of Navigate in strict mode when using a data router (#10435)5e195ec
Fix useNAvigate when called from <Routes> inside a <RouterProvider> (#10432)666d962
Fix usage of Component API within descendant routes (#10434)7ff51c0
chore: Update version for release (#10414)5ec9f8e
chore: Update version for release (pre) (#10410)Verification Steps for Reviewers
These are to be checked by a reviewer.
ephemeral deployment links:
• https://admin-milmove-pr-10805.mymove.sandbox.truss.coffee/
• https://office-milmove-pr-10805.mymove.sandbox.truss.coffee/
Setup to Run the Code Locally
How to test
Route changes
/new-shipment/:shipmentType
rather than the old pattern of/new-:shipmentType
. I.e. you should see something like/new-shipment/PPM
rather than/new-PPM
Ensuring Admin works
react-router
andreact-router-dom
.Frontend
officeApp
class or custommin-width
styling is used to hide any states the would not be visible to the user.Screenshots