Skip to content

Flip react-router-dom-v5-compat to v6 - #490

Closed
blaipr wants to merge 1 commit into
ctrliq:mainfrom
blaipr:feature/react-router-v6-proper
Closed

Flip react-router-dom-v5-compat to v6#490
blaipr wants to merge 1 commit into
ctrliq:mainfrom
blaipr:feature/react-router-v6-proper

Conversation

@blaipr

@blaipr blaipr commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

⚠️⚠️⚠️ Depends on #489 — please merge #489 first.


SUMMARY

Flip react-router-dom from v5 + the react-router-dom-v5-compat bridge to react-router v6 proper, completing Step 2 of the React modernization. Every screen <Switch> route tree is already v6 and the whole test suite is RTL, so the compat bridge is no longer needed.

Production changes

  • App.js: remove <CompatRouter> — the v6 <HashRouter> wraps <App> directly.
  • Retarget ~340 react-router-dom-v5-compat imports → react-router-dom.
  • Replace the v5-only APIs the compat layer had been shimming (which v6 drops):
    • ScreenHeader breadcrumbs: rewrite the recursive v5 <Route>/useRouteMatch walk as a plain useLocation() + cumulative-path lookup (breadcrumbConfig is keyed by resolved paths, so no route matching is needed).
    • useRouteMatchuseMatch (Setting AzureAD/LDAP/GitHub detail+edit, Subscription, Config, UserAndTeamAccessAdd) or location.pathname (Project/WorkflowApproval lists); <Redirect><Navigate>; matchPath → v6 signature.
    • <Link innerRef>ref (v6 Link is forwardRef).
  • react-router-dom ^5.3.3^6.30.4; remove react-router-dom-v5-compat.
  • history 4.10.1^5.3.0 (devDependency — v6's <Router> needs the v5 listen signature, and v6 no longer pulls history itself; only the test harness uses it directly).

Test harness

  • testUtils/rtlContexts.js: replace the v5-Router + nested-v6 CompatV6Layer with a single v6 history-driven <Router> (an inlined HistoryRouter).
  • Convert the router-using test suites to v6 (<Switch>/standalone <Route><Routes> + element, manual <Router history>renderWithContexts, useHistoryuseNavigate, mount useMatch/useParams components at concrete v6 URLs). No production assertions weakened; no new skips.

ISSUE TYPE

  • New or Enhanced Feature

COMPONENT NAME

  • UI

ASCENDER VERSION

awx: 25.4.1.dev110+g8e7ecff748

ADDITIONAL INFORMATION

Stacked on #489 (the JobOutput @tanstack/react-virtual swap) to avoid a package.json/JobOutput.js conflict; once #489 merges this rebases cleanly to drop that commit.

Verification

  • npm --prefix awx/ui run test: 552 suites / 2910 tests green (3 pre-existing documented skips); npm --prefix awx/ui run lint clean; npm --prefix awx/ui run build (production) compiles clean; test_licenses.py green (removed the orphaned react-router-dom-v5-compat license).
  • Browser-verified on a dev instance (headless Chrome, ~55 routes/flows), all with 0 /api/v2/.../undefined/... calls:
    • all routes render (no Not Found / blank / error overlay); breadcrumbs render + navigate; tab navigation incl. nested sub-tabs (inventory Hosts/Groups/Sources); useMatch detail pages (GitHub/Azure/LDAP detail + edit) render without redirect loops.
    • navigate-on-submit: create org → redirect to its detail, edit → redirect, delete from list → removed; 404 catch-all; logout → /login, protected route while logged out → /login, re-login → /home; browser back/forward.

Note: v6 emits benign v7_startTransition / v7_relativeSplatPath future-flag console warnings — harmless; opting into them belongs with the react-router 7 step.

@blaipr blaipr changed the title Flip react-router-dom to v6 proper and remove the v5-compat bridge Flip react-router-dom-v5-compat to v6 Jun 18, 2026
All screen <Switch> route trees are v6 and every test suite is RTL, so the
v5-compat bridge is no longer needed. Move react-router-dom from 5.3.3 + the
react-router-dom-v5-compat shim to v6.30.4 proper, and drop the bridge.

Production changes:
- App.js: remove <CompatRouter>; the v6 <HashRouter> now wraps <App> directly.
- Retarget all ~340 'react-router-dom-v5-compat' imports to 'react-router-dom'.
- Replace v5-only APIs that v6 drops (the compat layer had been shimming them):
  - ScreenHeader breadcrumbs: rewrite the recursive v5 <Route>/useRouteMatch
    walk as a plain useLocation() + cumulative-path lookup (breadcrumbConfig is
    keyed by resolved paths, so no route matching is needed).
  - useRouteMatch -> useMatch (Setting AzureAD/LDAP/GitHub detail+edit,
    Subscription, Config, UserAndTeamAccessAdd) or location.pathname (Project /
    WorkflowApproval lists); Redirect -> Navigate; matchPath -> v6 signature.
  - Link innerRef -> ref (v6 Link is forwardRef).
- history 4.10.1 -> 5.3.0 (devDependency; react-router v6 needs the v5 history
  listen signature for the test harness, and v6 no longer pulls history itself).

Test harness:
- rtlContexts.js: replace the v5-Router + nested-v6 CompatV6Layer with a single
  v6 history-driven <Router> (inlined HistoryRouter).
- Convert router-using tests to v6: standalone <Route>/<Switch> -> <Routes>
  + element, manual <Router history> -> renderWithContexts, useHistory ->
  useNavigate, and mount useMatch/useParams components at concrete v6 URLs.

Verified: full Jest suite green (552 suites / 2910 tests), lint clean,
test_licenses.py green. Browser-verified on a dev instance: 19 routes render,
breadcrumbs, tab + nested sub-tab navigation, Add-button links, useMatch detail
pages (GitHub/Azure/LDAP), and browser back/forward all work with no
/undefined/ API calls.
@blaipr
blaipr force-pushed the feature/react-router-v6-proper branch from 8e7ecff to 80edab9 Compare June 19, 2026 00:43
@cigamit
cigamit requested a review from Copilot June 19, 2026 02:29
@cigamit cigamit self-assigned this Jun 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@blaipr

blaipr commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Superseding this in favor of a 3-part split so the changes are reviewable in pieces (this single PR was 373 files, over Copilot's 300-file review limit, so it never got an automated review).

The v6 flip is inherently atomic because react-router-dom-v5-compat peer-requires react-router-dom v4/5 and cannot coexist with v6. To split it anyway, the new series introduces a one-line src/routerCompat re-export module, routes every consumer through it in two mechanical passes, then flips that single module to v6 last:

Each part is under 300 files, individually Copilot-reviewable, and leaves the app working and the suite green. Same end state as this PR (lint clean, 552 suites green, production build clean, test_licenses green). Closing this one.

@blaipr blaipr closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants