Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc0a14f39b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <QueryParamProvider adapter={ReactRouter6Adapter}> | ||
| <Routes> |
There was a problem hiding this comment.
This JSX now references ReactRouter6Adapter, Routes, and Navigate, but index.js still imports ReactRouter5Adapter and does not import the new router components; as written, rendering AppHTML will hit a ReferenceError and prevent the dashboard from loading at all. Please update the imports in this file to match the new identifiers used in the routing tree.
Useful? React with 👍 / 👎.
| replace: (path, state) => navigate(path, { replace: true, state }), | ||
| location, | ||
| }} | ||
| match={{ params }} |
There was a problem hiding this comment.
The compatibility wrapper only passes match={{ params }}, but legacy components still rely on match.url; for example ResourceDetail calls match.url.match(...) in getResourceFromUrl and compares prevProps.match.url in componentDidUpdate, so navigating to resource detail routes will throw once match.url is missing. The shim needs to provide a v5-compatible match shape (at least including url) to avoid breaking these pages.
Useful? React with 👍 / 👎.
React Router v6 Upgrade UpdatesAdded a few specific corrections to ensure the dashboard reliably functions after the
|
Summary
This PR upgrades
react-router-domfrom v5 to v6 in the dashboard web app (#7252).Upgrading to React Router v6 introduces many breaking changes (like the removal of
<Switch>,<Redirect>, and thewithRouterHOC, as well as breaking changes to props inside<Route>components). To minimize churn in the codebase and keep the transition clean for legacy components, we have implemented a backward-compatible HOC (withRouter.jsx) that wraps v6 hooks (useNavigate,useLocationanduseParams) and translates them into thehistory,locationandmatchprops that the existing components expect.Changes Made
react-router-domto^6.22.3.use-query-paramsto^3.2.1to support the newReactRouter6Adapter.historyandreact-router-prop-types).web/app/js/index.js):<Switch>with<Routes>.<Redirect>to<Navigate replace>.renderprop in<Route>for theelementprop.web/app/js/components/util/withRouter.jsx):withRouterhigh-order component. This bridges v6 hooks into legacy v5 props so the existing class components (likeNavigationBase) do not need heavy refactoring.testHelpers.jsxand individual component test suites (Navigation.test.jsx,BreadcrumbHeader.test.jsx) to useMemoryRouterwithinitialEntriesinstead of deprecated custom contexts built fromcreateMemoryHistory.Testing
yarn testwas run directly. All 15 suites (95 tests) pass successfully, restoring the complete test coverage./to/namespaces), and parameter extractions worked seamlessly.