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 581014169a78..4ae0c208e998 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 @@ -74,11 +74,41 @@ const ProjectsRoutes = () => ( ); +// Descendant whose matched route (`:id` via its index) sits above further non-wildcard nested +// child routes (`:sub`). The nested subtree must not steal the transaction name from the `child/*` +// parent - the pageload/navigation name should stay `/child/:id`, not `/:id/:sub` (see issue #22194). +const ChildRoutes = () => ( + + + Child} /> + + Sub} /> + + + +); + +// Deep wildcard chain: three levels of `/*` nesting. +// workspace/* → :teamId/* → :memberId +const DeepMemberRoutes = () => ( + + Deep Member} /> + +); + +const DeepTeamRoutes = () => ( + + } /> + +); + const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement); 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 f0dc55c761a6..e0b372a51965 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 @@ -10,6 +10,12 @@ const Index = () => { navigate old + + navigate child + + + navigate deep member + ); }; 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 7c5a1bd7ac67..3c4598be922a 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 @@ -61,6 +61,38 @@ test('sends a pageload transaction with a parameterized URL - alternative route' }); }); +test('keeps the parent path prefix for a descendant route with non-wildcard nested children - pageload', async ({ + page, +}) => { + const transactionPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/child/abc123`); + + const rootSpan = await transactionPromise; + + expect((await page.innerHTML('#root')).includes('Child')).toBe(true); + expect(rootSpan).toMatchObject({ + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.react.reactrouter_v6', + data: { + 'sentry.source': 'route', + 'url.template': '/child/:id', + 'url.path': '/child/abc123', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/child\/abc123$/), + }, + }, + }, + transaction: '/child/:id', + transaction_info: { + source: 'route', + }, + }); +}); + test('sends a navigation transaction with a parameterized URL', async ({ page }) => { const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; @@ -172,3 +204,128 @@ test('sends a navigation transaction with a parameterized URL - alternative rout }, }); }); + +test('keeps the parent path prefix for a descendant route with non-wildcard nested children - navigation', async ({ + page, +}) => { + const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + const navigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; + }); + + await page.goto(`/`); + const pageloadTxn = await pageloadTxnPromise; + + expect(pageloadTxn).toMatchObject({ + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.react.reactrouter_v6', + data: { + 'sentry.source': 'route', + 'url.template': '/', + 'url.path': '/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), + }, + }, + }, + transaction: '/', + transaction_info: { + source: 'route', + }, + }); + + const linkElement = page.locator('id=child-navigation'); + + const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]); + + expect((await page.innerHTML('#root')).includes('Child')).toBe(true); + expect(navigationTxn).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react.reactrouter_v6', + data: { + 'sentry.source': 'route', + 'url.template': '/child/:id', + 'url.path': '/child/abc123', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/child\/abc123$/), + }, + }, + }, + transaction: '/child/:id', + transaction_info: { + source: 'route', + }, + }); +}); + +test('resolves deep wildcard chain with three levels of nesting - pageload', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/workspace/team/u123`); + + const rootSpan = await transactionPromise; + + expect((await page.innerHTML('#root')).includes('Deep Member')).toBe(true); + expect(rootSpan).toMatchObject({ + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.react.reactrouter_v6', + data: { + 'sentry.source': 'route', + 'url.template': '/workspace/:teamId/:memberId', + 'url.path': '/workspace/team/u123', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/workspace\/team\/u123$/), + }, + }, + }, + transaction: '/workspace/:teamId/:memberId', + 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'; + }); + + const navigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; + }); + + await page.goto(`/`); + await pageloadTxnPromise; + + const linkElement = page.locator('id=deep-member-navigation'); + + const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]); + + expect((await page.innerHTML('#root')).includes('Deep Member')).toBe(true); + expect(navigationTxn).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react.reactrouter_v6', + data: { + 'sentry.source': 'route', + 'url.template': '/workspace/:teamId/:memberId', + 'url.path': '/workspace/team/u123', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/workspace\/team\/u123$/), + }, + }, + }, + transaction: '/workspace/:teamId/:memberId', + transaction_info: { + source: 'route', + }, + }); +}); diff --git a/packages/react/src/reactrouter-compat-utils/utils.ts b/packages/react/src/reactrouter-compat-utils/utils.ts index a43288823070..edb7b8fdb5d3 100644 --- a/packages/react/src/reactrouter-compat-utils/utils.ts +++ b/packages/react/src/reactrouter-compat-utils/utils.ts @@ -180,6 +180,52 @@ export function rebuildRoutePathFromAllRoutes(allRoutes: RouteObject[], location return ''; } +/** + * Recovers the parent prefix for descendant `` names. + * + * `allRoutes` flattens every mounted `` into one set, so an orphaned descendant subtree can + * outscore the `.../*` route that anchors the location and drop its prefix (e.g. `/:id/:sub` instead of `/child/:id`). + * Matching only the descendant-parent routes recovers the true anchor. + */ +function reconstructNameFromDescendantParent( + location: Location, + allRoutes: RouteObject[], + currentName: string | undefined, +): string | undefined { + const descendantParents = allRoutes.filter(routeIsDescendant); + if (!descendantParents.length) { + return undefined; + } + + const matchedParents = _matchRoutes(descendantParents, location) as RouteMatch[] | null; + const parentMatch = matchedParents?.[matchedParents.length - 1]; + if (!parentMatch || !pickSplat(parentMatch)) { + return undefined; + } + + const parentTemplate = trimSlash(trimWildcard(parentMatch.route.path || '')); + + // Only a static leading segment (e.g. `child/*`) has a fixed position in the absolute URL; dynamic + // leads (`:projectId/*`) are relative and stay with the existing wildcard-rebuild path. + if (!parentTemplate || parentTemplate.startsWith(':')) { + return undefined; + } + + const expectedPrefix = prefixWithSlash(parentTemplate); + if (currentName === expectedPrefix || currentName?.startsWith(`${expectedPrefix}/`)) { + return undefined; + } + + const remainingPathname = + stripBasenameFromPathname(location.pathname, prefixWithSlash(parentMatch.pathnameBase)) || '/'; + const remainingName = rebuildRoutePathFromAllRoutes( + allRoutes.filter(route => route !== parentMatch.route), + { pathname: remainingPathname }, + ); + + return remainingName ? prefixWithSlash(`${parentTemplate}${prefixWithSlash(remainingName)}`) : undefined; +} + /** * Checks if the current location is inside a descendant route (route with splat parameter). */ @@ -303,6 +349,13 @@ export function resolveRouteNameAndSource( [name, source] = getNormalizedName(routes, location, branches, basename); } + // Guard against orphaned descendant subtrees stealing the transaction name: if the location is + // anchored by a descendant-parent route (`.../*`) whose prefix was dropped, reconstruct with it. + const anchoredName = reconstructNameFromDescendantParent(location, allRoutes, name); + if (anchoredName) { + return [anchoredName, 'route']; + } + return [name || location.pathname, source]; } diff --git a/packages/react/test/reactrouter-descendant-routes.test.tsx b/packages/react/test/reactrouter-descendant-routes.test.tsx index e172b8b65f0b..b7e068df0c97 100644 --- a/packages/react/test/reactrouter-descendant-routes.test.tsx +++ b/packages/react/test/reactrouter-descendant-routes.test.tsx @@ -88,6 +88,231 @@ describe('React Router Descendant Routes', () => { }); describe('withSentryReactRouterV6Routing', () => { + it('keeps the parent path prefix for descendant routes with non-wildcard nested children - pageload', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + // A descendant whose matched route (`:id` via its index) sits above further nested + // non-wildcard child routes (`:sub`). The nested subtree must not steal the transaction name from + // the `child/*` parent (see issue #22194). + const ChildRouter = () => ( + + + Child} /> + + Sub} /> + + + + ); + + const { container } = render( + + + } /> + + , + ); + + expect(container.innerHTML).toContain('Child'); + + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/child/:id'); + expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(URL_TEMPLATE, '/child/:id'); + }); + + it('keeps the parent path prefix for descendant routes with non-wildcard nested children - navigation', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + const ChildRouter = () => ( + + + Child} /> + + Sub} /> + + + + ); + + const { container } = render( + + + } /> + } /> + + , + ); + + expect(container.innerHTML).toContain('Child'); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/child/:id', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [URL_TEMPLATE]: '/child/:id', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', + }, + }); + }); + + it('prefers a concrete sibling route over a descendant wildcard parent', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + const ChildRouter = () => ( + + + Child} /> + + Sub} /> + + + + ); + + const { container } = render( + + + Settings} /> + } /> + + , + ); + + expect(container.innerHTML).toContain('Settings'); + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/child/settings'); + }); + + it('keeps the parent path prefix for a dynamic-lead descendant parent with non-wildcard nested children - pageload', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + // The descendant parent has a dynamic leading segment (`:orgId/*`). The nested `:sub` subtree must + // not steal the transaction name - it should stay `/:orgId/:id`, not `/:id/:sub`. + const OrgRouter = () => ( + + + Org} /> + + Sub} /> + + + + ); + + const { container } = render( + + + } /> + + , + ); + + expect(container.innerHTML).toContain('Org'); + + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/:orgId/:id'); + expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(URL_TEMPLATE, '/:orgId/:id'); + }); + + it('keeps the parent path prefix for a dynamic-lead descendant parent with non-wildcard nested children - navigation', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + const OrgRouter = () => ( + + + Org} /> + + Sub} /> + + + + ); + + const { container } = render( + + + } /> + } /> + + , + ); + + expect(container.innerHTML).toContain('Org'); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/:orgId/:id', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [URL_TEMPLATE]: '/:orgId/:id', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', + }, + }); + }); + it('works with descendant wildcard routes - pageload', () => { const client = createMockBrowserClient(); setCurrentClient(client);