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 f6694a954915..581014169a78 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 @@ -3,6 +3,7 @@ import React from 'react'; import ReactDOM from 'react-dom/client'; import { BrowserRouter, + Outlet, Route, Routes, createRoutesFromChildren, @@ -48,17 +49,28 @@ const DetailsRoutes = () => ( ); +const DetailsRoutesAlternative = () => ( + + Details} /> + +); + const ViewsRoutes = () => ( Views} /> } /> + } /> ); const ProjectsRoutes = () => ( - }> - No Match Page} /> + }> + Project Page Root} /> + }> + } /> + + ); @@ -67,7 +79,7 @@ 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 aa99b61f89ea..d2362c149f84 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 @@ -8,6 +8,9 @@ const Index = () => { navigate + + navigate old + ); }; 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 23bc0aaabe95..2f13b7cc1eac 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 @@ -10,6 +10,7 @@ test('sends a pageload transaction with a parameterized URL', async ({ page }) = const rootSpan = await transactionPromise; + expect((await page.innerHTML('#root')).includes('Details')).toBe(true); expect(rootSpan).toMatchObject({ contexts: { trace: { @@ -24,6 +25,30 @@ test('sends a pageload transaction with a parameterized URL', async ({ page }) = }); }); +test('sends a pageload transaction with a parameterized URL - alternative route', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/projects/234/old-views/234/567`); + + const rootSpan = await transactionPromise; + + expect((await page.innerHTML('#root')).includes('Details')).toBe(true); + expect(rootSpan).toMatchObject({ + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.react.reactrouter_v6', + }, + }, + transaction: '/projects/:projectId/old-views/:viewId/:detailId', + 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'; @@ -52,6 +77,8 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) const linkElement = page.locator('id=navigation'); const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]); + + expect((await page.innerHTML('#root')).includes('Details')).toBe(true); expect(navigationTxn).toMatchObject({ contexts: { trace: { @@ -65,3 +92,47 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) }, }); }); + +test('sends a navigation transaction with a parameterized URL - alternative route', 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', + }, + }, + transaction: '/', + transaction_info: { + source: 'route', + }, + }); + + const linkElement = page.locator('id=old-navigation'); + + const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]); + + expect((await page.innerHTML('#root')).includes('Details')).toBe(true); + expect(navigationTxn).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react.reactrouter_v6', + }, + }, + transaction: '/projects/:projectId/old-views/:viewId/:detailId', + transaction_info: { + source: 'route', + }, + }); +}); diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index 1211435254a1..6aabe0d0db59 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -180,7 +180,7 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio return origUseRoutes; } - const allRoutes: RouteObject[] = []; + const allRoutes: Set = new Set(); const SentryRoutes: React.FC<{ children?: React.ReactNode; @@ -207,10 +207,21 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio if (isMountRenderPass.current) { routes.forEach(route => { - allRoutes.push(...getChildRoutesRecursively(route)); + const extractedChildRoutes = getChildRoutesRecursively(route); + + extractedChildRoutes.forEach(r => { + allRoutes.add(r); + }); }); - updatePageloadTransaction(getActiveRootSpan(), normalizedLocation, routes, undefined, undefined, allRoutes); + updatePageloadTransaction( + getActiveRootSpan(), + normalizedLocation, + routes, + undefined, + undefined, + Array.from(allRoutes), + ); isMountRenderPass.current = false; } else { handleNavigation({ @@ -218,7 +229,7 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio routes, navigationType, version, - allRoutes, + allRoutes: Array.from(allRoutes), }); } }, [navigationType, stableLocationParam]); @@ -343,14 +354,18 @@ function locationIsInsideDescendantRoute(location: Location, routes: RouteObject return false; } -function getChildRoutesRecursively(route: RouteObject, allRoutes: RouteObject[] = []): RouteObject[] { - if (route.children && !route.index) { - route.children.forEach(child => { - allRoutes.push(...getChildRoutesRecursively(child, allRoutes)); - }); - } +function getChildRoutesRecursively(route: RouteObject, allRoutes: Set = new Set()): Set { + if (!allRoutes.has(route)) { + allRoutes.add(route); - allRoutes.push(route); + if (route.children && !route.index) { + route.children.forEach(child => { + const childRoutes = getChildRoutesRecursively(child, allRoutes); + + childRoutes.forEach(r => allRoutes.add(r)); + }); + } + } return allRoutes; } @@ -513,7 +528,7 @@ export function createV6CompatibleWithSentryReactRouterRouting

= new Set(); const SentryRoutes: React.FC

= (props: P) => { const isMountRenderPass = React.useRef(true); @@ -527,10 +542,14 @@ export function createV6CompatibleWithSentryReactRouterRouting

{ - allRoutes.push(...getChildRoutesRecursively(route)); + const extractedChildRoutes = getChildRoutesRecursively(route); + + extractedChildRoutes.forEach(r => { + allRoutes.add(r); + }); }); - updatePageloadTransaction(getActiveRootSpan(), location, routes, undefined, undefined, allRoutes); + updatePageloadTransaction(getActiveRootSpan(), location, routes, undefined, undefined, Array.from(allRoutes)); isMountRenderPass.current = false; } else { handleNavigation({ @@ -538,7 +557,7 @@ export function createV6CompatibleWithSentryReactRouterRouting

{ + const actual = jest.requireActual('@sentry/browser'); + return { + ...actual, + startBrowserTracingNavigationSpan: (...args: unknown[]) => { + mockStartBrowserTracingNavigationSpan(...args); + return actual.startBrowserTracingNavigationSpan(...args); + }, + startBrowserTracingPageLoadSpan: (...args: unknown[]) => { + mockStartBrowserTracingPageLoadSpan(...args); + return actual.startBrowserTracingPageLoadSpan(...args); + }, + }; +}); + +jest.mock('@sentry/core', () => { + const actual = jest.requireActual('@sentry/core'); + return { + ...actual, + getRootSpan: () => { + return mockRootSpan; + }, + }; +}); + +describe('React Router Descendant Routes', () => { + function createMockBrowserClient(): BrowserClient { + return new BrowserClient({ + integrations: [], + tracesSampleRate: 1, + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], + }); + } + + beforeEach(() => { + jest.clearAllMocks(); + getCurrentScope().setClient(undefined); + }); + + describe('withSentryReactRouterV6Routing', () => { + it('works with descendant wildcard routes - pageload', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + const DetailsRoutes = () => ( + + Details} /> + + ); + + const ViewsRoutes = () => ( + + Views} /> + } /> + + ); + + const ProjectsRoutes = () => ( + + }> + No Match Page} /> + + ); + + const { container } = render( + + + }> + + , + ); + + expect(container.innerHTML).toContain('Details'); + + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/projects/:projectId/views/:viewId/:detailId'); + expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + }); + + it('works with descendant wildcard routes - navigation', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + const DetailsRoutes = () => ( + + Details} /> + + ); + + const ViewsRoutes = () => ( + + Views} /> + } /> + + ); + + const ProjectsRoutes = () => ( + + }> + No Match Page} /> + + ); + + const { container } = render( + + + } /> + }> + + , + ); + + expect(container.innerHTML).toContain('Details'); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/projects/:projectId/views/:viewId/:detailId', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', + }, + }); + }); + + it('works with descendant wildcard routes with outlets', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + const DetailsRoutes = () => ( + + Details} /> + + ); + + const ViewsRoutes = () => ( + + Views} /> + } /> + + ); + + const ProjectsRoutes = () => ( + + }> + Project Page Root} /> + }> + } /> + + + + ); + + const { container } = render( + + + } /> + }> + + , + ); + + expect(container.innerHTML).toContain('Details'); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/projects/:projectId/views/:viewId/:detailId', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', + }, + }); + }); + }); + + describe('wrapUseRoutesV6', () => { + it('works with descendant wildcard routes - pageload', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + + const wrappedUseRoutes = wrapUseRoutesV6(useRoutes); + + const DetailsRoutes = () => + wrappedUseRoutes([ + { + path: ':detailId', + element:

Details
, + }, + ]); + + const ViewsRoutes = () => + wrappedUseRoutes([ + { + index: true, + element:
Views
, + }, + { + path: 'views/:viewId/*', + element: , + }, + ]); + + const ProjectsRoutes = () => + wrappedUseRoutes([ + { + path: 'projects/:projectId/*', + element: , + }, + { + path: '*', + element:
No Match Page
, + }, + ]); + + const Routes = () => + wrappedUseRoutes([ + { + path: '/*', + element: , + }, + ]); + + const { container } = render( + + + , + ); + + expect(container.innerHTML).toContain('Details'); + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/projects/:projectId/views/:viewId/:detailId'); + expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + }); + + it('works with descendant wildcard routes - navigation', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + client.addIntegration( + reactRouterV6BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + }), + ); + + const wrappedUseRoutes = wrapUseRoutesV6(useRoutes); + + const DetailsRoutes = () => + wrappedUseRoutes([ + { + path: ':detailId', + element:
Details
, + }, + ]); + + const ViewsRoutes = () => + wrappedUseRoutes([ + { + index: true, + element:
Views
, + }, + { + path: 'views/:viewId/*', + element: , + }, + ]); + + const ProjectsRoutes = () => + wrappedUseRoutes([ + { + path: 'projects/:projectId/*', + element: , + }, + { + path: '*', + element:
No Match Page
, + }, + ]); + + const Routes = () => + wrappedUseRoutes([ + { + index: true, + element: , + }, + { + path: '/*', + element: , + }, + ]); + + const { container } = render( + + + , + ); + + expect(container.innerHTML).toContain('Details'); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/projects/:projectId/views/:viewId/:detailId', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', + }, + }); + }); + }); +}); diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index 33e330c2e232..f61436667210 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -491,7 +491,7 @@ describe('reactRouterV6BrowserTracingIntegration', () => { }); }); - it('works with descendant wildcard routes - pageload', () => { + it('works under a slash route with a trailing slash', () => { const client = createMockBrowserClient(); setCurrentClient(client); @@ -506,40 +506,31 @@ describe('reactRouterV6BrowserTracingIntegration', () => { ); const SentryRoutes = withSentryReactRouterV6Routing(Routes); - const DetailsRoutes = () => ( - - Details} /> - - ); - - const ViewsRoutes = () => ( - - Views} /> - } /> - - ); - - const ProjectsRoutes = () => ( - - }> - No Match Page} /> - - ); - render( - + - }> + } /> + root}> + issues group}> + index} /> + + , ); - expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); - expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/projects/:projectId/views/:viewId/:detailId'); - expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/issues/:groupId/', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', + }, + }); }); - it('works with descendant wildcard routes - navigation', () => { + it('works nested under a slash root without a trailing slash', () => { const client = createMockBrowserClient(); setCurrentClient(client); @@ -554,38 +545,22 @@ describe('reactRouterV6BrowserTracingIntegration', () => { ); const SentryRoutes = withSentryReactRouterV6Routing(Routes); - const DetailsRoutes = () => ( - - Details} /> - - ); - - const ViewsRoutes = () => ( - - Views} /> - } /> - - ); - - const ProjectsRoutes = () => ( - - }> - No Match Page} /> - - ); - render( - } /> - }> + } /> + root}> + issues group}> + index} /> + + , ); expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { - name: '/projects/:projectId/views/:viewId/:detailId', + name: '/issues/:groupId/', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', @@ -1214,150 +1189,6 @@ describe('reactRouterV6BrowserTracingIntegration', () => { }); }); - it('works with descendant wildcard routes - pageload', () => { - const client = createMockBrowserClient(); - setCurrentClient(client); - - client.addIntegration( - reactRouterV6BrowserTracingIntegration({ - useEffect: React.useEffect, - useLocation, - useNavigationType, - createRoutesFromChildren, - matchRoutes, - }), - ); - - const wrappedUseRoutes = wrapUseRoutesV6(useRoutes); - - const DetailsRoutes = () => - wrappedUseRoutes([ - { - path: ':detailId', - element:
Details
, - }, - ]); - - const ViewsRoutes = () => - wrappedUseRoutes([ - { - index: true, - element:
Views
, - }, - { - path: 'views/:viewId/*', - element: , - }, - ]); - - const ProjectsRoutes = () => - wrappedUseRoutes([ - { - path: 'projects/:projectId/*', - element: , - }, - { - path: '*', - element:
No Match Page
, - }, - ]); - - const Routes = () => - wrappedUseRoutes([ - { - path: '/*', - element: , - }, - ]); - - render( - - - , - ); - - expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); - expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/projects/:projectId/views/:viewId/:detailId'); - expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); - }); - - it('works with descendant wildcard routes - navigation', () => { - const client = createMockBrowserClient(); - setCurrentClient(client); - - client.addIntegration( - reactRouterV6BrowserTracingIntegration({ - useEffect: React.useEffect, - useLocation, - useNavigationType, - createRoutesFromChildren, - matchRoutes, - }), - ); - - const wrappedUseRoutes = wrapUseRoutesV6(useRoutes); - - const DetailsRoutes = () => - wrappedUseRoutes([ - { - path: ':detailId', - element:
Details
, - }, - ]); - - const ViewsRoutes = () => - wrappedUseRoutes([ - { - index: true, - element:
Views
, - }, - { - path: 'views/:viewId/*', - element: , - }, - ]); - - const ProjectsRoutes = () => - wrappedUseRoutes([ - { - path: 'projects/:projectId/*', - element: , - }, - { - path: '*', - element:
No Match Page
, - }, - ]); - - const Routes = () => - wrappedUseRoutes([ - { - index: true, - element: , - }, - { - path: '/*', - element: , - }, - ]); - - render( - - - , - ); - - expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); - expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { - name: '/projects/:projectId/views/:viewId/:detailId', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6', - }, - }); - }); - it('does not add double slashes to URLS', () => { const client = createMockBrowserClient(); setCurrentClient(client);