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..ce0a0699352a 100644 --- a/packages/react/src/reactrouter-compat-utils/utils.ts +++ b/packages/react/src/reactrouter-compat-utils/utils.ts @@ -231,6 +231,15 @@ function reconstructNameFromDescendantParent( return remainingName ? prefixWithSlash(`${parentTemplate}${prefixWithSlash(remainingName)}`) : undefined; } +/** + * Returns the routes that aren't a child of another route in the list. + * `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(routes.flatMap(route => route.children ?? [])); + return routes.filter(route => !nestedRoutes.has(route)); +} + /** * Checks if the current location is inside a descendant route (route with splat parameter). */ @@ -239,17 +248,11 @@ export function locationIsInsideDescendantRoute( routes: RouteObject[], matchRoutes: MatchRoutes, ): boolean { - const matchedRoutes = matchRoutes(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)); } /** @@ -348,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/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..7e1435521dc7 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,6 +248,17 @@ 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] }; + const descendant: RouteObject = { path: '/users/*', element: 'div' }; + + expect(getRootRoutes([root, settings, leaf, descendant])).toEqual([root, descendant]); + }); + }); + describe('locationIsInsideDescendantRoute', () => { it('should return true when location is inside descendant route', () => { const routes: RouteObject[] = [ @@ -541,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' }, @@ -569,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 diff --git a/packages/react/test/reactrouter-cross-usage.test.tsx b/packages/react/test/reactrouter-cross-usage.test.tsx index 580cd11841df..2c8a15789370 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,57 @@ describe('React Router cross usage of wrappers', () => { }); }); }); + + describe('nested route matching', () => { + it('should name 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
}], + }, + ], + }, + ]); + + render(); + + await act(async () => { + router.navigate('/account'); + }); + + await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1)); + + expect(router.state.matches.map(match => match.route.path)).toEqual(['/', ':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', + }, + }); + }); + }); });