From c98c2940e90fa64e17ead3785832d7f54cf3b5c6 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Tue, 28 Jul 2026 17:07:31 -0400 Subject: [PATCH 1/3] test(react): Add e2e repro for allRoutes accumulation across independent routers Adds two independent descendant trees (foo/*, bar/*) and a cross-router client-side navigation. Because allRoutes is a shared module-level set, navigating into the second router picks up the first router's param name, producing the hybrid /bar/:fooId instead of /bar/:barId. This test currently fails, documenting issue #22782. --- .../src/index.tsx | 18 ++++++ .../src/pages/Index.tsx | 6 ++ .../tests/transactions.test.ts | 60 +++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/index.tsx index 09751f0f920d..31f7765f5374 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/index.tsx @@ -103,6 +103,22 @@ const DeepTeamRoutes = () => ( ); +// Two independent descendant trees that each contribute a single-segment param leaf +// (`:fooId` / `:barId`). Because `allRoutes` is a shared module-level set, once both have mounted a +// navigation into one can pick up the param name from the other, yielding a hybrid name like +// `/bar/:fooId` instead of `/bar/:barId` (see issue #22782). +const FooRoutes = () => ( + + Foo} /> + +); + +const BarRoutes = () => ( + + Bar} /> + +); + const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement); root.render( @@ -110,6 +126,8 @@ root.render( } /> } /> } /> + } /> + } /> } /> , diff --git a/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/pages/Index.tsx b/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/pages/Index.tsx index e0b372a51965..ff6b75a0a536 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/pages/Index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/src/pages/Index.tsx @@ -16,6 +16,12 @@ const Index = () => { navigate deep member + + navigate foo + + + navigate bar + ); }; diff --git a/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/tests/transactions.test.ts index 3c4598be922a..9e25f8d65d9d 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/tests/transactions.test.ts @@ -293,6 +293,66 @@ test('resolves deep wildcard chain with three levels of nesting - pageload', asy }); }); +test('does not mix param names across independent descendant routers', async ({ page }) => { + const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + const fooNavigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return ( + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.contexts?.trace?.data?.['url.path'] === '/foo/123' + ); + }); + + const barNavigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return ( + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.contexts?.trace?.data?.['url.path'] === '/bar/456' + ); + }); + + await page.goto(`/`); + await pageloadTxnPromise; + + // Mount the first descendant router (`foo/*` -> `:fooId`), which populates the shared `allRoutes` set. + const [, fooNavigationTxn] = await Promise.all([page.locator('id=foo-navigation').click(), fooNavigationTxnPromise]); + + expect((await page.innerHTML('#root')).includes('Foo')).toBe(true); + expect(fooNavigationTxn).toMatchObject({ + transaction: '/foo/:fooId', + transaction_info: { source: 'route' }, + }); + + // Return to the index so we can navigate into the second, unrelated descendant router client-side. + // A fresh page load would reset the module-level `allRoutes` and hide the bug. + await page.goBack(); + await page.locator('id=bar-navigation').waitFor(); + + // Now mount the second descendant router (`bar/*` -> `:barId`). With the accumulation bug, the name + // comes out as the hybrid `/bar/:fooId`. + const [, barNavigationTxn] = await Promise.all([page.locator('id=bar-navigation').click(), barNavigationTxnPromise]); + + expect((await page.innerHTML('#root')).includes('Bar')).toBe(true); + expect(barNavigationTxn).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react.reactrouter_v6', + data: { + 'sentry.source': 'route', + 'url.template': '/bar/:barId', + 'url.path': '/bar/456', + }, + }, + }, + transaction: '/bar/:barId', + transaction_info: { + source: 'route', + }, + }); +}); + test('resolves deep wildcard chain with three levels of nesting - navigation', async ({ page }) => { const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; From 1e330547d5a736a513cb7f0e7dd42cd1470c0a4e Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Tue, 28 Jul 2026 18:09:22 -0400 Subject: [PATCH 2/3] fix(react): Remove routes from shared set on `` unmount The module-level `allRoutes` set accumulated every route ever mounted and never removed any, so once two independent routers had each been mounted, `matchRoutes` ran over the union of both and could name a transaction with one router's static segment and another's param (e.g. `/bar/:fooId`). Removing a ``'s routes when it unmounts keeps the set to what is currently mounted, so stale routes from an unrelated router can't be matched against a later navigation. Fixes #22782 --- .../instrumentation.tsx | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 61e11e46c7ac..49fbf0b45502 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -755,6 +755,7 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio locationArg?: Partial | string; }> = (props: { children?: React.ReactNode; routes: RouteObject[]; locationArg?: Partial | string }) => { const isMountRenderPass = React.useRef(true); + const addedRoutes = React.useRef([]); const { routes, locationArg } = props; const Routes = origUseRoutes(routes, locationArg); @@ -771,7 +772,7 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio typeof stableLocationParam === 'string' ? { pathname: stableLocationParam } : stableLocationParam; if (isMountRenderPass.current) { - addRoutesToAllRoutes(routes); + addedRoutes.current = addRoutesToAllRoutes(routes); updatePageloadTransaction({ activeRootSpan: getActiveRootSpan(), @@ -794,6 +795,10 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio } }, [navigationType, stableLocationParam]); + // Drop this ``'s routes from the shared set when it unmounts, so they don't leak into later + // unrelated navigations (#22782). + useIsomorphicLayoutEffect(() => () => removeRoutesFromAllRoutes(addedRoutes.current), []); + return Routes; }; @@ -1049,14 +1054,29 @@ export function handleNavigation(opts: { } /* Only exported for testing purposes */ -export function addRoutesToAllRoutes(routes: RouteObject[]): void { +export function addRoutesToAllRoutes(routes: RouteObject[]): RouteObject[] { + const added: RouteObject[] = []; routes.forEach(route => { const extractedChildRoutes = getChildRoutesRecursively(route); extractedChildRoutes.forEach(r => { allRoutes.add(r); + added.push(r); }); }); + + return added; +} + +/** + * Removes routes previously added via `addRoutesToAllRoutes` from the shared set. Called when a + * `` unmounts so its routes don't linger and get matched against later, unrelated navigations + * (which produced hybrid names like `/bar/:fooId` across independent routers - see #22782). + */ +function removeRoutesFromAllRoutes(routes: RouteObject[]): void { + routes.forEach(route => { + allRoutes.delete(route); + }); } function getChildRoutesRecursively(route: RouteObject, allRoutes: Set = new Set()): Set { @@ -1351,6 +1371,7 @@ export function createV6CompatibleWithSentryReactRouterRouting

= (props: P) => { const isMountRenderPass = React.useRef(true); + const addedRoutes = React.useRef([]); const location = _useLocation(); const navigationType = _useNavigationType(); @@ -1360,7 +1381,7 @@ export function createV6CompatibleWithSentryReactRouterRouting

`'s routes from the shared set when it unmounts, so they don't leak into later + // unrelated navigations (#22782). + useIsomorphicLayoutEffect(() => () => removeRoutesFromAllRoutes(addedRoutes.current), []); + // @ts-expect-error Setting more specific React Component typing for `R` generic above // will break advanced type inference done by react router params return ; From 3039c3187028804d14aec222210e6ab83e90cb8d Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Wed, 29 Jul 2026 10:11:45 -0400 Subject: [PATCH 3/3] fix(react): Tie route registration to effect lifecycle for StrictMode The previous cleanup added routes only during the mount render pass but removed them on every unmount. Under StrictMode (mount, unmount, remount) the remount skipped the add, leaving the shared set missing routes and breaking descendant name reconstruction in cross-usage scenarios. Add and remove now share one effect lifecycle, so a StrictMode remount re-adds the routes it removed on the intermediate unmount. --- .../instrumentation.tsx | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 49fbf0b45502..62bf4521c2b4 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -755,7 +755,6 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio locationArg?: Partial | string; }> = (props: { children?: React.ReactNode; routes: RouteObject[]; locationArg?: Partial | string }) => { const isMountRenderPass = React.useRef(true); - const addedRoutes = React.useRef([]); const { routes, locationArg } = props; const Routes = origUseRoutes(routes, locationArg); @@ -767,13 +766,20 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio const stableLocationParam = typeof locationArg === 'string' || locationArg?.pathname ? (locationArg as { pathname: string }) : location; + // Register this ``'s routes in the shared set for as long as it is mounted, removing them on + // unmount so they don't leak into later unrelated navigations (#22782). Tying add and remove to the + // same effect lifecycle keeps it correct under StrictMode's mount/unmount/remount. + useIsomorphicLayoutEffect(() => { + const added = addRoutesToAllRoutes(routes); + + return () => removeRoutesFromAllRoutes(added); + }); + useIsomorphicLayoutEffect(() => { const normalizedLocation = typeof stableLocationParam === 'string' ? { pathname: stableLocationParam } : stableLocationParam; if (isMountRenderPass.current) { - addedRoutes.current = addRoutesToAllRoutes(routes); - updatePageloadTransaction({ activeRootSpan: getActiveRootSpan(), location: normalizedLocation, @@ -795,10 +801,6 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio } }, [navigationType, stableLocationParam]); - // Drop this ``'s routes from the shared set when it unmounts, so they don't leak into later - // unrelated navigations (#22782). - useIsomorphicLayoutEffect(() => () => removeRoutesFromAllRoutes(addedRoutes.current), []); - return Routes; }; @@ -1371,18 +1373,24 @@ export function createV6CompatibleWithSentryReactRouterRouting

= (props: P) => { const isMountRenderPass = React.useRef(true); - const addedRoutes = React.useRef([]); const location = _useLocation(); const navigationType = _useNavigationType(); + const routes = _createRoutesFromChildren(props.children) as RouteObject[]; + + // Register this ``'s routes in the shared set for as long as it is mounted, removing them on + // unmount so they don't leak into later unrelated navigations (#22782). Tying add and remove to the + // same effect lifecycle keeps it correct under StrictMode's mount/unmount/remount. + useIsomorphicLayoutEffect(() => { + const added = addRoutesToAllRoutes(routes); + + return () => removeRoutesFromAllRoutes(added); + }); + useIsomorphicLayoutEffect( () => { - const routes = _createRoutesFromChildren(props.children) as RouteObject[]; - if (isMountRenderPass.current) { - addedRoutes.current = addRoutesToAllRoutes(routes); - updatePageloadTransaction({ activeRootSpan: getActiveRootSpan(), location, @@ -1401,10 +1409,6 @@ export function createV6CompatibleWithSentryReactRouterRouting

`'s routes from the shared set when it unmounts, so they don't leak into later - // unrelated navigations (#22782). - useIsomorphicLayoutEffect(() => () => removeRoutesFromAllRoutes(addedRoutes.current), []); - // @ts-expect-error Setting more specific React Component typing for `R` generic above // will break advanced type inference done by react router params return ;