From fc8c653d126255c8901b705dfab0c71e752317fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 13 Oct 2026 11:35:36 -0400 Subject: [PATCH 1/4] fix(react): Match React Router navigations against root routes only --- .../src/reactrouter-compat-utils/index.ts | 1 + .../instrumentation.tsx | 13 +++-- .../src/reactrouter-compat-utils/utils.ts | 19 ++++++- .../instrumentation.test.tsx | 1 + .../reactrouter-compat-utils/utils.test.ts | 35 ++++++++++++ .../test/reactrouter-cross-usage.test.tsx | 57 +++++++++++++++++++ 6 files changed, 121 insertions(+), 5 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/index.ts b/packages/react/src/reactrouter-compat-utils/index.ts index 76b01c413615..426c6747eff7 100644 --- a/packages/react/src/reactrouter-compat-utils/index.ts +++ b/packages/react/src/reactrouter-compat-utils/index.ts @@ -18,6 +18,7 @@ export { export { resolveRouteNameAndSource, getNormalizedName, + getRootRoutes, locationIsInsideDescendantRoute, prefixWithSlash, rebuildRoutePathFromAllRoutes, diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 629b839d11ad..f18a90a3fb07 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -44,6 +44,7 @@ import { checkRouteForAsyncHandler } from './lazy-routes'; import { clearNavigationContext, getActiveRootSpan, + getRootRoutes, resolveRouteNameAndSource, setNavigationContext, transactionNameHasWildcard, @@ -389,7 +390,7 @@ export function updateNavigationSpan( const shouldUpdate = !hasBeenNamed || forceUpdate || currentNameHasWildcard; if (shouldUpdate && !end_timestamp) { - const currentBranches = config.matchRoutes(allRoutes, location); + const currentBranches = config.matchRoutes(getRootRoutes(allRoutes), location); const [name, source] = resolveRouteNameAndSource( location, allRoutes, @@ -993,7 +994,7 @@ export function handleNavigation(opts: { const { location, routes, navigationType, version, config, matches, allRoutes } = opts; const branches = Array.isArray(matches) ? matches - : config.matchRoutes(allRoutes || routes, location, config.basename); + : config.matchRoutes(getRootRoutes(allRoutes || routes), location, config.basename); const client = getClient(); if (!client || !config.instrumentNavigation) { @@ -1167,7 +1168,7 @@ function updatePageloadTransaction({ }): void { const branches = Array.isArray(matches) ? matches - : (config.matchRoutes(allRoutes || routes, location, config.basename) as unknown as RouteMatch[]); + : (config.matchRoutes(getRootRoutes(allRoutes || routes), location, config.basename) as unknown as RouteMatch[]); if (branches) { const [name, source] = resolveRouteNameAndSource( @@ -1263,7 +1264,11 @@ function tryUpdateSpanNameBeforeEnd( const currentAllRoutes = Array.from(allRoutes); const routesToUse = currentAllRoutes.length > 0 ? currentAllRoutes : routes; - const branches = config.matchRoutes(routesToUse, location, config.basename) as unknown as RouteMatch[]; + const branches = config.matchRoutes( + getRootRoutes(routesToUse), + location, + config.basename, + ) as unknown as RouteMatch[]; if (!branches) { return; diff --git a/packages/react/src/reactrouter-compat-utils/utils.ts b/packages/react/src/reactrouter-compat-utils/utils.ts index 9f33127c8ce1..aadd8c57eaeb 100644 --- a/packages/react/src/reactrouter-compat-utils/utils.ts +++ b/packages/react/src/reactrouter-compat-utils/utils.ts @@ -231,6 +231,23 @@ function reconstructNameFromDescendantParent( return remainingName ? prefixWithSlash(`${parentTemplate}${prefixWithSlash(remainingName)}`) : undefined; } +/** + * Returns the routes that aren't a child of another route in the list. + * `allRoutes` holds every nested route alongside its ancestors, and `matchRoutes` flattens each + * entry's children again, so matching the full list multiplies the branches it ranks and tests. + */ +export function getRootRoutes(routes: RouteObject[]): RouteObject[] { + const nestedRoutes = new Set(); + for (const route of routes) { + if (route.children && !route.index) { + for (const child of route.children) { + nestedRoutes.add(child); + } + } + } + return nestedRoutes.size ? routes.filter(route => !nestedRoutes.has(route)) : routes; +} + /** * Checks if the current location is inside a descendant route (route with splat parameter). */ @@ -239,7 +256,7 @@ export function locationIsInsideDescendantRoute( routes: RouteObject[], matchRoutes: MatchRoutes, ): boolean { - const matchedRoutes = matchRoutes(routes, location) as RouteMatch[]; + const matchedRoutes = matchRoutes(getRootRoutes(routes), location) as RouteMatch[]; if (matchedRoutes) { for (const match of matchedRoutes) { diff --git a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx index fd6f00d13690..07dd0e5b66ac 100644 --- a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx +++ b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx @@ -80,6 +80,7 @@ vi.mock('../../src/reactrouter-compat-utils/utils', () => ({ return name.includes('/*') || name === '*' || name.endsWith('*'); }), getActiveRootSpan: vi.fn(() => undefined), + getRootRoutes: vi.fn((routes: unknown[]) => routes), })); vi.mock('../../src/reactrouter-compat-utils/lazy-routes', () => ({ diff --git a/packages/react/test/reactrouter-compat-utils/utils.test.ts b/packages/react/test/reactrouter-compat-utils/utils.test.ts index e2a1e7f9a256..82a6cfedf124 100644 --- a/packages/react/test/reactrouter-compat-utils/utils.test.ts +++ b/packages/react/test/reactrouter-compat-utils/utils.test.ts @@ -4,6 +4,7 @@ import { getNavigationContext, getNormalizedName, getNumberOfUrlSegments, + getRootRoutes, locationIsInsideDescendantRoute, pathEndsWithWildcard, pathIsWildcardAndHasChildren, @@ -247,7 +248,41 @@ describe('reactrouter-compat-utils/utils', () => { }); }); + describe('getRootRoutes', () => { + it('should drop routes whose parent is also in the list', () => { + const leaf: RouteObject = { path: 'account' }; + const settings: RouteObject = { path: 'settings', children: [leaf] }; + const root: RouteObject = { path: '/', children: [settings] }; + + expect(getRootRoutes([root, settings, leaf])).toEqual([root]); + }); + + it('should keep routes whose parent is not in the list', () => { + const leaf: RouteObject = { path: 'account' }; + const descendant: RouteObject = { path: '/users/*', element: 'div' }; + + expect(getRootRoutes([leaf, descendant])).toEqual([leaf, descendant]); + }); + + it('should not treat children of index routes as nested', () => { + const child: RouteObject = { path: 'child' }; + const indexRoute: RouteObject = { index: true, children: [child] }; + + expect(getRootRoutes([indexRoute, child])).toEqual([indexRoute, child]); + }); + }); + describe('locationIsInsideDescendantRoute', () => { + it('should only match against root routes', () => { + const child: RouteObject = { path: 'profile' }; + const parent: RouteObject = { path: '/users', children: [child] }; + mockMatchRoutes.mockReturnValue(null); + + locationIsInsideDescendantRoute({ pathname: '/users/profile' }, [parent, child], mockMatchRoutes as MatchRoutes); + + expect(mockMatchRoutes).toHaveBeenCalledWith([parent], { pathname: '/users/profile' }); + }); + it('should return true when location is inside descendant route', () => { const routes: RouteObject[] = [ { diff --git a/packages/react/test/reactrouter-cross-usage.test.tsx b/packages/react/test/reactrouter-cross-usage.test.tsx index 580cd11841df..0d8b825be675 100644 --- a/packages/react/test/reactrouter-cross-usage.test.tsx +++ b/packages/react/test/reactrouter-cross-usage.test.tsx @@ -12,6 +12,7 @@ import { matchRoutes, MemoryRouter, Navigate, + Outlet, Route, RouterProvider, Routes, @@ -911,4 +912,60 @@ describe('React Router cross usage of wrappers', () => { }); }); }); + + describe('nested route matching', () => { + it('names navigations after the route the router renders, not a nested route matched on its own', async () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + + const createSentryMemoryRouter = wrapCreateMemoryRouterV6(createMemoryRouter); + + const router = createSentryMemoryRouter( + [ + { + path: '/', + element: , + children: [ + { path: ':orgId', element:
Org
}, + { + path: 'settings', + element: , + children: [{ path: 'account', element:
Account settings
}], + }, + ], + }, + ], + { initialEntries: ['/settings/account'] }, + ); + + render(); + + await act(async () => { + router.navigate('/account'); + }); + + await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1)); + + expect(router.state.matches.at(-1)?.route.path).toBe(':orgId'); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/:orgId', + attributes: { + [SENTRY_SEGMENT_NAME_SOURCE]: 'route', + [URL_TEMPLATE]: '/:orgId', + [SENTRY_OP]: 'navigation', + [SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', + }, + }); + }); + }); }); From c59748c3ce2f90ea8a6b587812ef1c8404c459d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 13 Oct 2026 11:46:03 -0400 Subject: [PATCH 2/4] ref(react): Simplify root route filtering and trim tests --- .../src/reactrouter-compat-utils/utils.ts | 14 ++------ .../reactrouter-compat-utils/utils.test.ts | 15 +-------- .../test/reactrouter-cross-usage.test.tsx | 33 +++++++++---------- 3 files changed, 19 insertions(+), 43 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/utils.ts b/packages/react/src/reactrouter-compat-utils/utils.ts index aadd8c57eaeb..e9d67e5ca16c 100644 --- a/packages/react/src/reactrouter-compat-utils/utils.ts +++ b/packages/react/src/reactrouter-compat-utils/utils.ts @@ -233,19 +233,11 @@ function reconstructNameFromDescendantParent( /** * Returns the routes that aren't a child of another route in the list. - * `allRoutes` holds every nested route alongside its ancestors, and `matchRoutes` flattens each - * entry's children again, so matching the full list multiplies the branches it ranks and tests. + * `matchRoutes` already walks each route's children, so also passing those children multiplies its work. */ export function getRootRoutes(routes: RouteObject[]): RouteObject[] { - const nestedRoutes = new Set(); - for (const route of routes) { - if (route.children && !route.index) { - for (const child of route.children) { - nestedRoutes.add(child); - } - } - } - return nestedRoutes.size ? routes.filter(route => !nestedRoutes.has(route)) : routes; + const nestedRoutes = new Set(routes.flatMap(route => route.children ?? [])); + return routes.filter(route => !nestedRoutes.has(route)); } /** diff --git a/packages/react/test/reactrouter-compat-utils/utils.test.ts b/packages/react/test/reactrouter-compat-utils/utils.test.ts index 82a6cfedf124..4cf7e3a3557f 100644 --- a/packages/react/test/reactrouter-compat-utils/utils.test.ts +++ b/packages/react/test/reactrouter-compat-utils/utils.test.ts @@ -253,22 +253,9 @@ describe('reactrouter-compat-utils/utils', () => { const leaf: RouteObject = { path: 'account' }; const settings: RouteObject = { path: 'settings', children: [leaf] }; const root: RouteObject = { path: '/', children: [settings] }; - - expect(getRootRoutes([root, settings, leaf])).toEqual([root]); - }); - - it('should keep routes whose parent is not in the list', () => { - const leaf: RouteObject = { path: 'account' }; const descendant: RouteObject = { path: '/users/*', element: 'div' }; - expect(getRootRoutes([leaf, descendant])).toEqual([leaf, descendant]); - }); - - it('should not treat children of index routes as nested', () => { - const child: RouteObject = { path: 'child' }; - const indexRoute: RouteObject = { index: true, children: [child] }; - - expect(getRootRoutes([indexRoute, child])).toEqual([indexRoute, child]); + expect(getRootRoutes([root, settings, leaf, descendant])).toEqual([root, descendant]); }); }); diff --git a/packages/react/test/reactrouter-cross-usage.test.tsx b/packages/react/test/reactrouter-cross-usage.test.tsx index 0d8b825be675..5e8fd28652cd 100644 --- a/packages/react/test/reactrouter-cross-usage.test.tsx +++ b/packages/react/test/reactrouter-cross-usage.test.tsx @@ -914,7 +914,7 @@ describe('React Router cross usage of wrappers', () => { }); describe('nested route matching', () => { - it('names navigations after the route the router renders, not a nested route matched on its own', async () => { + it('should name navigations after the route the router renders, not a nested route matched on its own', async () => { const client = createMockBrowserClient(); setCurrentClient(client); @@ -930,23 +930,20 @@ describe('React Router cross usage of wrappers', () => { const createSentryMemoryRouter = wrapCreateMemoryRouterV6(createMemoryRouter); - const router = createSentryMemoryRouter( - [ - { - path: '/', - element: , - children: [ - { path: ':orgId', element:
Org
}, - { - path: 'settings', - element: , - children: [{ path: 'account', element:
Account settings
}], - }, - ], - }, - ], - { initialEntries: ['/settings/account'] }, - ); + const router = createSentryMemoryRouter([ + { + path: '/', + element: , + children: [ + { path: ':orgId', element:
Org
}, + { + path: 'settings', + element: , + children: [{ path: 'account', element:
Account settings
}], + }, + ], + }, + ]); render(); From 455b51e50f3454b4af13d5fc9f3edf9bcc70c434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 13 Oct 2026 11:46:52 -0400 Subject: [PATCH 3/4] test(react): Drop redundant root route wiring test --- .../react/test/reactrouter-compat-utils/utils.test.ts | 10 ---------- packages/react/test/reactrouter-cross-usage.test.tsx | 2 +- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/packages/react/test/reactrouter-compat-utils/utils.test.ts b/packages/react/test/reactrouter-compat-utils/utils.test.ts index 4cf7e3a3557f..fea0a96b8032 100644 --- a/packages/react/test/reactrouter-compat-utils/utils.test.ts +++ b/packages/react/test/reactrouter-compat-utils/utils.test.ts @@ -260,16 +260,6 @@ describe('reactrouter-compat-utils/utils', () => { }); describe('locationIsInsideDescendantRoute', () => { - it('should only match against root routes', () => { - const child: RouteObject = { path: 'profile' }; - const parent: RouteObject = { path: '/users', children: [child] }; - mockMatchRoutes.mockReturnValue(null); - - locationIsInsideDescendantRoute({ pathname: '/users/profile' }, [parent, child], mockMatchRoutes as MatchRoutes); - - expect(mockMatchRoutes).toHaveBeenCalledWith([parent], { pathname: '/users/profile' }); - }); - it('should return true when location is inside descendant route', () => { const routes: RouteObject[] = [ { diff --git a/packages/react/test/reactrouter-cross-usage.test.tsx b/packages/react/test/reactrouter-cross-usage.test.tsx index 5e8fd28652cd..2c8a15789370 100644 --- a/packages/react/test/reactrouter-cross-usage.test.tsx +++ b/packages/react/test/reactrouter-cross-usage.test.tsx @@ -953,7 +953,7 @@ describe('React Router cross usage of wrappers', () => { await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1)); - expect(router.state.matches.at(-1)?.route.path).toBe(':orgId'); + expect(router.state.matches.map(match => match.route.path)).toEqual(['/', ':orgId']); expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { name: '/:orgId', attributes: { From 2a5a32b8f1efd4fccb53e957838fdcd2c2e8fc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 13 Oct 2026 15:07:01 -0400 Subject: [PATCH 4/4] simplify locationIsInsideDescendantRoute with a return matchesAreInsideDescendantRoute --- .../src/reactrouter-compat-utils/utils.ts | 19 ++++++++----------- .../reactrouter-compat-utils/utils.test.ts | 14 +------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/utils.ts b/packages/react/src/reactrouter-compat-utils/utils.ts index e9d67e5ca16c..ce0a0699352a 100644 --- a/packages/react/src/reactrouter-compat-utils/utils.ts +++ b/packages/react/src/reactrouter-compat-utils/utils.ts @@ -248,17 +248,11 @@ export function locationIsInsideDescendantRoute( routes: RouteObject[], matchRoutes: MatchRoutes, ): boolean { - const matchedRoutes = matchRoutes(getRootRoutes(routes), location) as RouteMatch[]; - - if (matchedRoutes) { - for (const match of matchedRoutes) { - if (routeIsDescendant(match.route) && pickSplat(match)) { - return true; - } - } - } + return matchesAreInsideDescendantRoute(matchRoutes(getRootRoutes(routes), location) as RouteMatch[] | null); +} - return false; +function matchesAreInsideDescendantRoute(matches: RouteMatch[] | null): boolean { + return !!matches?.some(match => routeIsDescendant(match.route) && pickSplat(match)); } /** @@ -357,7 +351,10 @@ export function resolveRouteNameAndSource( let name: string | undefined; let source: TransactionSource = 'url'; - const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes, matchRoutes); + // `branches` were matched with the basename, which the descendant route lookups don't strip. + const isInDescendantRoute = basename + ? locationIsInsideDescendantRoute(location, allRoutes, matchRoutes) + : matchesAreInsideDescendantRoute(branches); if (isInDescendantRoute) { name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes, location, matchRoutes)); diff --git a/packages/react/test/reactrouter-compat-utils/utils.test.ts b/packages/react/test/reactrouter-compat-utils/utils.test.ts index fea0a96b8032..7e1435521dc7 100644 --- a/packages/react/test/reactrouter-compat-utils/utils.test.ts +++ b/packages/react/test/reactrouter-compat-utils/utils.test.ts @@ -553,16 +553,6 @@ describe('reactrouter-compat-utils/utils', () => { { path: '/users', element: null }, ]; const branches: RouteMatch[] = [ - { - route: { path: '/users', element: null }, - params: {}, - pathname: '/users', - pathnameBase: '', - }, - ]; - - // Mock for descendant route check - const descendantMatches: RouteMatch[] = [ { route: { path: '/users/*', element: 'div' }, params: { '*': '123/profile' }, @@ -581,9 +571,7 @@ describe('reactrouter-compat-utils/utils', () => { }, ]; - mockMatchRoutes - .mockReturnValueOnce(descendantMatches) // First call for descendant check - .mockReturnValueOnce(rebuildMatches); // Second call for path rebuild + mockMatchRoutes.mockReturnValueOnce(rebuildMatches); const result = resolveRouteNameAndSource(location, routes, allRoutes, branches, makeConfig()); // Since locationIsInsideDescendantRoute returns true, it uses route source