react-router migration batch 1: shared components, contexts and hooks#382
Closed
blaipr wants to merge 4 commits into
Closed
react-router migration batch 1: shared components, contexts and hooks#382blaipr wants to merge 4 commits into
blaipr wants to merge 4 commits into
Conversation
Stage 2 of the React stack modernization. react-router-dom 6/7 removed
Switch, useHistory, useRouteMatch, Redirect and withRouter, which 243
files here still use, so the migration has to be incremental. This sets
up the official bridge and migrates the first screen:
- Add react-router-dom-v5-compat (ships router v6 alongside v5) and
mount CompatRouter inside the app's HashRouter, so v5 and v6 routing
contexts coexist and components can be migrated file by file.
- Wire the v6 context into testUtils/enzymeHelpers: the v5 Router keeps
its history subscription and drives re-renders, while a fully
controlled nested v6 Router (location from v5 context, navigator =
the shared history) provides the v6 context without a second
subscription - no act() warnings, and shallow rendering still works.
- Migrate screens/Login as the demonstration slice: Redirect becomes
Navigate (from the compat package), and the withRouter wrapper is
dropped (the component never used the injected props).
- Remove a test.only in Login.test.js that had silenced the other 14
Login tests since the initial import, and fix the two assertions that
had rotted while dark (the custom login screen has no LoginMainHeader;
branding-fetch errors fall back to the default logo without a modal).
The suite now runs 2869 tests, 14 more than before.
Migration recipe for follow-up PRs, per component:
useHistory().push(x) -> useNavigate(); navigate(x)
useRouteMatch().params -> useParams()
<Redirect to={x} /> -> <Navigate to={x} />
withRouter(C) -> hooks (or delete if props unused)
importing from 'react-router-dom-v5-compat'; the root Switch in App.js
migrates last, then react-router-dom flips to v6 and the compat package
is removed.
…at APIs
First batch of the incremental react-router 5 -> 6 migration started in
the v5-compat bridge PR. Migrates the shared layer every screen depends
on (31 files): useHistory becomes useNavigate, history.location reads
become useLocation(), conditional Redirect becomes Navigate, and the
seven withRouter(Lookup) wrappers plus the dead withRouter(AppContainer)
wrapper are replaced with hooks. history.replace(...) calls become
navigate(..., { replace: true }).
Deliberately left on v5 (route-tree phase, migrates last):
Switch/Route/match.path usage (components/Schedule/Schedules.js,
Schedule.js, ScreenHeader.js) and prefix-match useRouteMatch({ path })
checks (contexts/Config.js, UserAndTeamAccessAdd.js).
Session.js's history.listen(POP) logic is rewritten with
useLocation/useNavigationType, skipping the initial render to keep v5's
fire-on-change-only semantics.
Test updates: NavExpandableGroup and RoutedTabs tests move from raw
MemoryRouter mounts to mountWithContexts (migrated components need the
v6 context); the InstanceList RTL test's custom wrapper gains the same
controlled v6 Router layer the helpers use - without it the suite
error-looped on useNavigate and crashed its jest worker; a dead
useHistory mock is removed from AddResourceRole tests and a
malformed relative initialEntries path gains its leading slash (v6
normalizes paths, v5 preserved the malformed value).
This was referenced Jun 11, 2026
navigate() from react-router-dom-v5-compat changes identity with the location (unlike v5's stable history object), so including it in this effect's deps refires the query-param-clearing navigation after unrelated navigations. Same fix as the batch-3 redirect effects.
Contributor
Author
|
Pushed a follow-up commit: |
This was referenced Jun 10, 2026
test_licenses requires every package.json dependency to have a matching file in licenses/ui (caught by an integration run of the full Python suite over all open PRs combined). (cherry picked from commit c8103b1)
This was referenced Jun 11, 2026
Contributor
Author
|
Superseded by #392, which unifies the v5-compat bridge and all three migration batches into a single PR (one squashed commit, identical content, full gates green). Done as part of making the whole queue conflict-free and mergeable in any order. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📋 Merge instructions — all 13 open PRs, any order, zero conflicts
Every pair of open PRs has been verified conflict-free with
git merge-tree(0 conflicts across all 78 pairs), and the union of all 13 was integration-tested green (Python suite 3477 passed, Jest 2873 passed, lint, build, licenses). Merge in any order you like. Two recommendations, not requirements:The full queue:
SUMMARY
First batch of the incremental react-router 5 → 6 migration, building on the v5-compat bridge from #381. This migrates the shared layer every screen depends on (31 source files in
src/components,src/contexts,src/hooks):useHistory()→useNavigate();history.push(x)→navigate(x);history.replace(...)→navigate(..., { replace: true })history.locationreads →useLocation()<Redirect>→<Navigate>(ContentError, Session)withRouter(...Lookup)wrappers → hooks; thewithRouter(AppContainer)wrapper was dead (component never read the injected props) and is simply droppedcontexts/Session'shistory.listenPOP-detection is rewritten withuseLocation/useNavigationType, skipping the initial render to preserve v5's fire-on-change-only semanticsDeliberately left on v5 (route-tree phase, migrates last):
Switch/Route/match.pathfiles (Schedules.js,Schedule.js,ScreenHeader.js) and prefix-matchuseRouteMatch({ path })checks (contexts/Config.js,UserAndTeamAccessAdd.js) — v6'suseMatchhas different (full-match) semantics, so those need individual care.Test changes:
NavExpandableGroup/RoutedTabstests move from rawMemoryRoutermounts tomountWithContexts(migrated components need the v6 context); theInstanceListRTL test's custom wrapper gains the same controlled v6 Router layer the helpers use — without it the suite error-looped onuseNavigateuntil the jest worker ran out of memory; a deaduseHistorymock is removed and one malformed relativeinitialEntriespath gains its leading slash (v6 normalizes paths; v5 preserved the malformed value).Stacked on #381 (contains its commit — the CompatRouter bridge is a hard prerequisite for components using compat APIs). Merge #381 first; this PR's diff reduces to its own commit once that lands. Remaining after this batch: ~200 screen files, splittable by screen directory using the same recipe.
ISSUE TYPE
COMPONENT NAME
ASCENDER VERSION
ADDITIONAL INFORMATION
All UI gates run against this branch state:
The v6-context requirement is self-enforcing in tests: any component migrated without the bridge in its test wrapper fails loudly (
useNavigate() may be used only in the context of a <Router>), which is how the InstanceList wrapper gap was caught.