fix(deps): update react-router and react-router-dom to ^6.30.6

[brian-smith-tcril]

Summary

Take react-router and react-router-dom to ^6.30.6 together, plus the two changes that bump requires. Supersedes #935 (renovate), which bumps only react-router-dom and is red on CI. Separately, #1952 (dependabot) proposes react-router → 8.3.0; that's a v7/v8 migration and a different conversation.

What changed

• package.json — both pins move as a pair. Bumping one alone nests a second react-router underneath react-router-dom, so the app gets two distinct router context objects, and the three modules importing from bare react-router (Unit/index.test.jsx:1, CoursewareSearch.jsx:2, CoursewareResultsFilter.jsx:5) read a different context than the rest of the tree. That accounts for 7 of the 9 failures on fix(deps): update dependency react-router-dom to v6.30.6 - autoclosed #935 — useLocation() may be used only in the context of a <Router> (45 occurrences in that CI log) and a courserun_key: undefined tracking event.
• decode-page-route/index.jsx (3 lines) — useMatch(route) → matchPath(route, location.pathname). react-router 6.20 changed useMatch to decode the pathname before matching, so computedMatch.pathname became the decoded path and DecodePageRoute's redirect guard silently collapsed. matchPath never decoded, so feeding it the raw pathname restores that field to what it held on 6.15 — everything from if (computedMatch) down is untouched. matchPath is also not a hook, so the match stops running inside a forEach callback.
• decode-page-route/index.test.jsx (1 line) — renderPage mounts on path="*". The same decoding applies to <Routes>, so the fixture's %2B-spelled route literal can no longer match and the component never mounted. That outer route was v5→v6 migration leftover: before b788b96 the match was injected as a computedMatch prop and there was no outer route at all. No fixture value and no assertion changed.

Behavior

No user-facing change, pinned by a characterization test added first, on 6.15.0, so the bump had to preserve behavior rather than redefine it.

134 cases: 13 route templates × 10 course-key spellings — modern, modern using the ~ . : punctuation opaque-keys' ALLOWED_ID_CHARS permits, deprecated, and deprecated carrying a literal %; each unencoded / encoded once / encoded six times, minus the cells where a deprecated key has no unencoded spelling — plus 4 rows for block-key spellings. 134/134 on 6.15.0 and 134/134 on 6.30.6, same landing URL character for character.

Without the DecodePageRoute change, six of those URL shapes stop normalizing — e.g. /course/course-v1%3AedX%2BDemoX%2BDemo_Course/home would stay encoded instead of redirecting to its decoded form.

Worth knowing downstream: on 6.20+, a route pattern whose literal is spelled with a percent-escape other than %2F can never match, because <Routes> matches against the decoded pathname. No route in this repo spells one, but a plugin or downstream MFE might.

Two pre-existing bugs surfaced and deliberately left alone: decodeUrl corrupts deprecated course keys containing a literal % (reproducible standalone, now pinned on both versions), and useParams decoding is untested because RedirectPage.test.jsx:13 mocks it out.

Testing

npm run lint ✓, npm run types ✓. Full suite on 6.30.6: 1092 passed — the single failure is the known Course.test.jsx "displays learner tools" parallel-run flake, green in isolation. src/decode-page-route/: 138/138 (134 matrix + the 4 original tests, all unchanged).

Decisions

Full decision log

Decisions — react-router 6.15.0 → 6.30.6

Working log for taking PR #935 (renovate, react-router-dom 6.15.0 → 6.30.6) to green.

Context

Two bots are fighting over these two packages:

• fix(deps): update dependency react-router-dom to v6.30.6 - autoclosed #935 (renovate, open since 2022, rebased forward) bumps only react-router-dom to 6.30.6.
• chore(deps): bump react-router from 6.30.6 to 7.18.2 #1952 (dependabot) bumps only react-router to 8.3.0.

package.json pins both packages separately and exactly, so either PR on its own splits the
versions. #935's CI is red: 9 failures across 3 suites.

Decision 1 — bump both pins together, as caret ranges

react-router: ^6.30.6 and react-router-dom: ^6.30.6. 6.30.6 is the newest 6.x for both.
npm install then produces a flat lock — one react-router, one react-router-dom, one
@remix-run/router@1.23.4, no nesting.

Why

#935 bumps react-router-dom while react-router stays at 6.15.0, so npm nests a second copy at
node_modules/react-router-dom/node_modules/react-router@6.30.6. Two copies means two distinct
React context objects, and this repo imports from both entrypoints — three files use bare
react-router (Unit/index.test.jsx:1, CoursewareSearch.jsx:2,
CoursewareResultsFilter.jsx:5) against 86 using react-router-dom.

That accounted for 7 of the 9 failures in #935:

• Unit/index.test.jsx wraps in MemoryRouter from react-router (6.15.0) while the tree's hooks
  resolve to the nested 6.30.6 copy → useLocation() may be used only in the context of a <Router>
  (45 occurrences in the CI log) → error boundary renders data-testid="error-page", so all 6
  assertions miss their testids.
• CoursewareSearch.test.jsx provides Route/Routes from react-router-dom while the component
  reads useParams from react-router → courserun_key: undefined in the tracking event.

Bumping both pins in lockstep removes the nesting and all 7 failures.

Decision 2 — DecodePageRoute matches on the raw pathname

src/decode-page-route/index.jsx, three lines: useMatch(route) → matchPath(route, location.pathname), with location from useLocation(). Everything from if (computedMatch)
down is untouched.

Why

@remix-run/router 1.13 (react-router 6.20) changed matching to decode the pathname first:

// react-router 6.15.0 — dist/react-router.development.js:127
return React.useMemo(() => matchPath(pattern, pathname), [pathname, pattern]);
// react-router 6.30.6 — same file, same line
return React.useMemo(() => matchPath(pattern, UNSAFE_decodePath(pathname)), [pathname, pattern]);

useMatch is exactly useLocation + matchPath, with a decodePath wedged between them since
6.20. matchPath itself never decoded the pathname, and still doesn't.

The component's guard, newUrl !== pathname (index.jsx:42), asks "is the URL in the address bar
different from the fully-decoded URL?" On 6.15 computedMatch.pathname was the raw pathname —
useMatch passes end: true, so the match spans the whole path and the field is just the pathname
it was given. On 6.30 that same field is the decoded pathname, so the comparison collapses to
newUrl === computedMatch.pathname and the redirect silently stops firing.

Feeding matchPath the raw pathname restores that field, so the guard keeps reading
computedMatch.pathname exactly as it did on 6.15 — matchPath compiles string patterns with
end: true, so the match spans the whole path and computedMatch.pathname is the pathname it was
given. The fix sits entirely upstream of the comparison; the comparison itself is not edited.

params do differ between versions (6.15 matchPath decoded them once; 6.30 returns them raw bar
%2F), but decodeUrl's recurse-to-fixed-point washes that out — generatePath's output is
byte-identical across both versions.

Side effect, accepted

useMatch memoised each match; a plain matchPath recompiles the pattern regex every render,
×13 root patterns. Cheap, and it removes a react-hooks/rules-of-hooks irregularity — the old code
called a hook inside ROUTES.forEach, safe only because ROUTES is module-level.

Rejected alternative — re-baseline the tests instead

Leave the component alone and accept the new behavior: matchRoutes decodes too
(router.js:527), so <Routes> matches encoded URLs and useParams() yields decoded values
without any redirect. Rejected because it changes two tests instead of one, and one of those
changes inverts an assertion from "this redirect happens" to "this redirect doesn't happen" —
declaring a user-visible behavior change correct rather than proving it harmless. Measured: six of
nine representative URLs stop normalizing (see below).

Evidence — behavior pinned before the bump

src/decode-page-route/decodeRoutes.test.tsx, committed on 6.15.0 before the bump, records
where a learner lands for each URL shape, as a matrix rather than a hand-picked list:

• 13 route templates — access-denied, home, live, dates, course-end, progress, progress for a
  target user, bare course, courseware sequence, courseware unit, preview sequence, preview unit,
  discussion topic.
• 10 course-key spellings — 4 key values × their valid encodings: modern, modern with the
  ~ . : punctuation ALLOWED_ID_CHARS permits, deprecated, and deprecated carrying a literal
  %; each unencoded / encoded once / encoded six times, except the deprecated pair which has no
  unencoded spelling because its / separators cannot sit in one path segment.

13 × 10 = 130 cases, plus 4 explicit rows for the things that vary something other than the
course key (block-key spellings, a unit key containing an encoded slash, and a mangled url with an
encoded space). 134 total, 134 passing on 6.15.0.

Design notes:

• Each key declares landsAs — where it actually ends up — rather than the test deriving the
  expectation by a rule. A rule would either be circular (mirroring the implementation) or would
  paper over the deprecated-% corruption below.
• Block keys are held at canonical values in the matrix. A :unitId is a UsageKey with its own
  grammar, so drawing it from the course key list would assert that a course key is a valid unit
  id. Their spelling variations live in the 4 explicit rows.
• Pairing a deprecated course key with modern block-v1: keys (in the courseware/preview
  templates) is not a realistic combination — real deprecated courses have i4x:// usage keys —
  but the routes pass strings through, and the encoding behavior is what is under test.

134/134 on 6.30.6 as well, with Decision 2 applied — same landing URL, character for
character, including every deprecated-key and literal-% row. Those are the rows with a plausible
reason to differ, since decodePath decodes each segment and then re-escapes slashes, which is
aimed squarely at %2F-bearing keys.

Without Decision 2, six of the URL shapes stop normalizing, e.g.
/course/course-v1%3AedX%2BDemoX%2BDemo_Course/home stays encoded instead of redirecting to
/course/course-v1:edX+DemoX+Demo_Course/home.

Harness caveat

MemoryRouter stores a pushed path verbatim; a browser history may re-encode it. The fixtures'
landing strings contain only characters browsers leave alone in a path (: + / ~ .), with one
exception: the mangled-url row's landing holds a literal space, where a browser would show %20.
That row's expected value is memory-history-specific.

What a course key can actually be

From openedx/opaque-keys, opaque_keys/edx/locator.py:

ALLOWED_ID_CHARS            = r'[\w\-~.:]'
DEPRECATED_ALLOWED_ID_CHARS = r'[\w\-~.:%]'

CourseLocator.__init__ validates org/course/run against ALLOWED_ID_RE, serialized as
"+".join([org, course, run]) behind course-v1:. The platform's URL layer is far more permissive
(openedx/core/constants.py:10):

COURSE_KEY_PATTERN = r'(?P<course_key_string>[^/+]+(/|\+)[^/+]+(/|\+)[^/?]+)'

Consequences for fixtures:

• Modern keys cannot contain %, space, /, or + inside a part. They can contain extra
  colons, tildes and periods — not currently exercised.

• Deprecated keys (org/course/run) may contain a literal %, and their / separators must
  travel as %2F within one path segment. So the encoded-slash fixture models a real key shape,
  and decodePath's decode-then-re-escape-slashes is aimed squarely at it.

• The encoded-space fixture is not a valid course key; it models a mangled URL, which is what
  PathFixesProvider (src/index.jsx:51, generic/path-fixes/PathFixesProvider.jsx:19) exists to
  repair.

• Confirmed bug — decodeUrl corrupts deprecated keys containing a literal %. It recurses
  until the string stops changing, so it cannot distinguish "encoded twice" from "the key contains
  a percent sign", and takes one pass too many:

  key as authored      : edX/DemoX/Demo%2BCourse
  url segment          : edX%2FDemoX%2FDemo%252BCourse
  one decode (router)  : edX/DemoX/Demo%2BCourse      <- correct key recovered
  decodeUrl fixed point: edX/DemoX/Demo+Course        <- %2B destroyed
  

  Pure string logic in index.jsx:14-20 — reproducible standalone, with no router or test harness
  involved. Pre-existing on 6.15.0 and unchanged by the bump, so it is pinned by the matrix rather
  than fixed here. Deserves its own issue. Note index.test.jsx's existing describe('decodeUrl')
  block only covers keys that decode cleanly.

Decision 3 — renderPage mounts the component on a catch-all route

src/decode-page-route/index.test.jsx, one line inside renderPage:

-        <Route path={props?.pattern?.path} element={<DecodePageRoute> {[]} </DecodePageRoute>} />
+        <Route path="*" element={<DecodePageRoute> {[]} </DecodePageRoute>} />

No test data, no assertion, and no fixture value changes. All four tests in the file pass.

Why it was failing

should only decode the url params and not the entire url fails on 6.30.6, and no component change
can fix it. renderPage mounted the component under props.pattern.path, which for that test is
/course/:courseId/some%2Bthing/:unitId. Under 6.20+ <Routes> matches against the decoded
pathname, so a pattern literal spelled with a percent-escape can never match, and
DecodePageRoute never mounts — zero <Navigate> renders, zero mockNavigate calls. The broken
link is the harness's outer <Route>, not the component and not the fixture.

Why the outer route was there in the first place

It is v5→v6 migration leftover, not a deliberate assertion about routing. In the introducing commit
52235ebc there was no outer <Route> at all — the component took its match as a prop, per
react-router v5's computedMatch convention:

<Router history={history}>
  <DecodePageRoute computedMatch={props} />
</Router>

The v6 upgrade (b788b969) rewrote the component to compute its own match with useMatch, so the
injected prop had nowhere to go. To get the component rendered under a router at a chosen URL, the
upgrade wrapped it in <Routes> and reused the surviving props fixture for both the URL and the
route path. The pattern was a convenient mounting point, nothing more.

path="*" therefore restores the original intent — put the component at this URL and let it do its
own matching — and drops an incidental coupling the v6 upgrade introduced.

Rejected alternative — respell the fixture's literal as %2F

decodePath decodes each segment then re-escapes slashes, so some%2Fthing round-trips and is the
one percent-escape a route can still declare. Changing MOCK_ROUTE_2 and the test's inline pattern
to use it would keep all three assertions passing.

Rejected because it changes what the test exercises. The point of the test is that generatePath
substitutes decoded params into pattern literals it copies verbatim; respelling the literal narrows
that to a demonstration of react-router's slash re-escaping, and amounts to adjusting test data
until it passes rather than fixing the thing that broke.

What this file no longer covers

With a catch-all mount, the outer <Routes> no longer participates, so this file no longer shows
that a real route pattern can mount the component. That is what decodeRoutes.test.tsx covers,
against the real DECODE_ROUTES — and it is the right split, since this file mocks DECODE_ROUTES
entirely and is a unit test of the component's own matching, not of app routing.

The property under test still holds after the bump

"Only decode the url params and not the entire url" remains true of the component: generatePath
copies pattern literals through untouched and substitutes decoded params. What the bump removed is
the ability for a route whose literal is spelled with a percent-escape to match at all.

Route patterns are compiled to a regex from the literal string as written (regex specials escaped,
no URL decoding), so pattern literals must now be spelled decoded. Encoded URLs still route;
only the pattern's spelling moved. %2F is the sole exception that round-trips, because
decodePath decodes and then re-escapes slashes (router.js:858-865).

What the test protects, per the original commit 52235eb (comments dropped by the RTL rewrite
in #1757), all three still asserted verbatim:

// unitId get decoded
// path remain encoded
// courseId get decoded

No route in DECODE_ROUTES spells a literal with a percent-escape, so nothing in this app
regresses — the mocked pattern at index.test.jsx:24 is the only one in the repo. Downstream
consumers declaring such a route are affected; see the gaps below.

Gaps this work surfaced, not addressed here

1. useParams decoding is untested. For the redirect routes the bump-sensitive question is what
   useParams() hands RedirectPage, since that string is baked into a
   global.location.assign() (RedirectPage.tsx:35). RedirectPage.test.jsx:13 mocks useParams
   to a hardcoded plain course id, so the decoding is stubbed out. 37 non-test modules call
   useParams.
2. The route tree isn't exported. index.jsx:41-142 builds it inline inside the APP_READY
   subscription, wired to createRoot, so tests reconstruct it — CoursewareContainer.test.jsx:108
   and ProductTours.test.jsx:278 do the same. The reconstruction is exact for the 13 absolute
   patterns and got the 2 relative ones wrong.
3. REDIRECT_HOME / REDIRECT_SURVEY are uncovered. CoursewareRedirectLandingPage nests them
   under ROUTES.REDIRECT and RedirectPage leaves via global.location.assign, so no landing
   pathname is observable in a MemoryRouter.
4. Downstream impact of the bump: any consumer declaring a route pattern with a percent-escape
   other than %2F breaks. Belongs in the PR description.

Verification

• Characterization matrix: 134/134 on 6.15.0 (pre-bump, committed first) and 134/134 on
  6.30.6 with Decision 2 applied.
• src/decode-page-route/ on 6.30.6 after Decision 3: 138/138 — the 134 matrix cases plus all
  four original tests in index.test.jsx.
• Full suite on 6.30.6 after Decision 3: 1092 passed, 1 failed. The failure is
  Course.test.jsx "displays learner tools when screen is wide enough (browser)", the known
  pre-existing flake — it passes in isolation (119 ms here, 306 ms earlier) and is unrelated to
  this work. No decode-page-route failures remain.
• Full suite before Decision 3, for comparison: 1092 passed, 1 failed, that failure being
  index.test.jsx test 4.
• npm run types ✓ and npm run lint ✓ on 6.30.6 against the full working tree — the matrix, the
  component change (Decision 2) and the harness change (Decision 3).
• Node: .nvmrc pins 24; all runs on v24.13.0.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.58%. Comparing base (e1b45e8) to head (0e958f7).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2047   +/-   ##
=======================================
  Coverage   93.58%   93.58%           
=======================================
  Files         367      367           
  Lines        6003     6004    +1     
  Branches     1419     1382   -37     
=======================================
+ Hits         5618     5619    +1     
- Misses        368      369    +1     
+ Partials       17       16    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[arbrandes]

👍🏼