From 0febe4ffd24a3e360b1bb1f812ce8ffc4a2b97ec Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 31 Jul 2025 21:31:29 +0100 Subject: [PATCH 01/10] fix(react): Add support for React Router sub-routes from `handle` --- .../react-router-6/tests/lazy.test.ts | 37 ++++++++ .../react-router-7-cross-usage/src/index.tsx | 14 ++++ .../src/pages/InnerLazyRoutes.tsx | 37 ++++++++ .../tests/transactions.test.ts | 23 +++++ .../react-router-7-lazy-routes/.gitignore | 29 +++++++ .../react-router-7-lazy-routes/.npmrc | 2 + .../react-router-7-lazy-routes/package.json | 53 ++++++++++++ .../playwright.config.mjs | 7 ++ .../public/index.html | 24 ++++++ .../src/globals.d.ts | 5 ++ .../react-router-7-lazy-routes/src/index.tsx | 84 +++++++++++++++++++ .../src/pages/Index.tsx | 14 ++++ .../src/pages/InnerLazyRoutes.tsx | 37 ++++++++ .../src/react-app-env.d.ts | 1 + .../start-event-proxy.mjs | 6 ++ .../tests/transactions.test.ts | 22 +++++ .../react-router-7-lazy-routes/tsconfig.json | 20 +++++ .../react/src/reactrouterv6-compat-utils.tsx | 66 +++++++++++++++ packages/react/src/types.ts | 2 + 19 files changed, 483 insertions(+) create mode 100644 dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts b/dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts new file mode 100644 index 000000000000..8221c4c162b0 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts @@ -0,0 +1,37 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('should capture pageload transaction for lazy route', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-6', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + transactionEvent.transaction === '/lazy/inner-lazy/inner' + ); + }); + + // Navigate to a lazy-loaded route - try just /lazy first + await page.goto('/lazy'); + + // Debug: Check what's actually rendered on the page + const pageContent = await page.content(); + console.log('Page content for /lazy:', pageContent); + + // Check if any content is rendered - use a more general selector + await page.waitForLoadState('networkidle'); + const bodyContent = await page.locator('body').innerHTML(); + console.log('Body content:', bodyContent); + + // Try to navigate to the full nested path + await page.goto('/lazy/inner-lazy/inner'); + await page.waitForLoadState('networkidle'); + + const nestedPageContent = await page.content(); + console.log('Nested page content:', nestedPageContent); + + const event = await transactionPromise; + + expect(event.transaction).toBe('/lazy/inner-lazy'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('pageload'); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx index bfcc527ded1b..4085e1c3568c 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx @@ -77,6 +77,19 @@ const ViewsRoutes = () => }, ]); +const LazyRoutes = () => + sentryUseRoutes([ + { + path: 'inner-lazy', + handle: { + lazyChildren: async () => + import('./pages/InnerLazyRoutes').then( + (module) => module.someMoreNestedRoutes, + ), + }, + } + ]); + const ProjectsRoutes = () => ( }> @@ -85,6 +98,7 @@ const ProjectsRoutes = () => ( } /> + } /> ); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx new file mode 100644 index 000000000000..006af4e67b5b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx @@ -0,0 +1,37 @@ +import React from 'react'; + +export const someMoreNestedRoutes = [ + { + path: 'level-1', + children: [ + { + index: true, + element: <>Level 1, + }, + { + path: ':id', + children: [ + { + index: true, + element: <>Level 1 ID, + }, + { + path: ':anotherId', + children: [ + { + index: true, + element: <>Level 1 ID Another ID, + }, + { + path: ':someAnotherId', + element:
+ Level 1 ID Another ID Some Another ID +
, + }, + ], + }, + ], + }, + ], + }, +]; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts index 1b521964f770..1c48425f2e6f 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts @@ -136,3 +136,26 @@ test('sends a navigation transaction with a parameterized URL - alternative rout }, }); }); + +test('sends a pageload transaction for lazy route', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-7-cross-usage', async transactionEvent => { + console.debug('Lazy route transaction event', transactionEvent); + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + transactionEvent.transaction === '/lazy/inner-lazy/inner-inner-lazy' + ); + }); + + // Try to navigate to the full nested path + await page.goto('/lazy/level-1/123/456/789'); + + console.debug('Navigated to lazy route'); + console.debug('ROOT', await page.innerHTML('#root')); + + const event = await transactionPromise; + + expect(event.transaction).toBe('/lazy/inner-lazy/inner-inner-lazy'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('pageload'); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore new file mode 100644 index 000000000000..84634c973eeb --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore @@ -0,0 +1,29 @@ +# See https://help.github.com/articles/ignoring-files/ for more about ignoring files. + +# dependencies +/node_modules +/.pnp +.pnp.js + +# testing +/coverage + +# production +/build + +# misc +.DS_Store +.env.local +.env.development.local +.env.test.local +.env.production.local + +npm-debug.log* +yarn-debug.log* +yarn-error.log* + +/test-results/ +/playwright-report/ +/playwright/.cache/ + +!*.d.ts diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json new file mode 100644 index 000000000000..dc61a85199e9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json @@ -0,0 +1,53 @@ +{ + "name": "react-router-7-cross-usage", + "version": "0.1.0", + "private": true, + "dependencies": { + "@sentry/react": "latest || *", + "@types/react": "18.0.0", + "@types/react-dom": "18.0.0", + "express": "4.20.0", + "react": "18.2.0", + "react-dom": "18.2.0", + "react-router-dom": "7.6.0", + "react-scripts": "5.0.1", + "typescript": "~5.0.0" + }, + "scripts": { + "build": "react-scripts build", + "start": "serve -s build", + "test": "playwright test", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "test:build": "pnpm install && npx playwright install && pnpm build", + "test:build-ts3.8": "pnpm install && pnpm add typescript@3.8 && npx playwright install && pnpm build", + "test:build-canary": "pnpm install && pnpm add react@canary react-dom@canary && npx playwright install && pnpm build", + "test:assert": "pnpm test" + }, + "eslintConfig": { + "extends": [ + "react-app", + "react-app/jest" + ] + }, + "browserslist": { + "production": [ + ">0.2%", + "not dead", + "not op_mini all" + ], + "development": [ + "last 1 chrome version", + "last 1 firefox version", + "last 1 safari version" + ] + }, + "devDependencies": { + "@playwright/test": "~1.53.2", + "@sentry-internal/test-utils": "link:../../../test-utils", + "serve": "14.0.1", + "npm-run-all2": "^6.2.0" + }, + "volta": { + "extends": "../../package.json" + } +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs new file mode 100644 index 000000000000..31f2b913b58b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html new file mode 100644 index 000000000000..39da76522bea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html @@ -0,0 +1,24 @@ + + + + + + + + React App + + + +
+ + + diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts new file mode 100644 index 000000000000..ffa61ca49acc --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts @@ -0,0 +1,5 @@ +interface Window { + recordedTransactions?: string[]; + capturedExceptionId?: string; + sentryReplayId?: string; +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx new file mode 100644 index 000000000000..c827707e51ff --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx @@ -0,0 +1,84 @@ +import * as Sentry from '@sentry/react'; +import React from 'react'; +import ReactDOM from 'react-dom/client'; +import { + Navigate, + PatchRoutesOnNavigationFunction, + RouterProvider, + createBrowserRouter, + createRoutesFromChildren, + matchRoutes, + useLocation, + useNavigationType, +} from 'react-router-dom'; +import Index from './pages/Index'; + +const replay = Sentry.replayIntegration(); + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.REACT_APP_E2E_TEST_DSN, + integrations: [ + Sentry.reactRouterV7BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + trackFetchStreamPerformance: true, + }), + replay, + ], + // We recommend adjusting this value in production, or using tracesSampler + // for finer control + tracesSampleRate: 1.0, + release: 'e2e-test', + + // Always capture replays, so we can test this properly + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + + tunnel: 'http://localhost:3031', +}); + +const sentryCreateBrowserRouter = Sentry.wrapCreateBrowserRouterV7(createBrowserRouter); + +const router = sentryCreateBrowserRouter( + [ + { + element: , + children: [ + { + path: '/', + element: , + }, + { + path: '/lazy', + handle: { + lazyChildren: () => import('./pages/InnerLazyRoutes').then(module => module.someMoreNestedRoutes), + }, + }, + { + path: '/static', + element: <>Hello World, + }, + { + path: '*', + element: , + }, + ], + }, + ], + { + async patchRoutesOnNavigation({ matches, patch }: Parameters[0]) { + const leafRoute = matches[matches.length - 1]?.route; + if (leafRoute?.id && leafRoute?.handle?.lazyChildren) { + const children = await leafRoute.handle.lazyChildren(); + patch(leafRoute.id, children); + } + }, + }, +); + +const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement); +root.render(); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx new file mode 100644 index 000000000000..e24b1b7cbff7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx @@ -0,0 +1,14 @@ +import * as React from 'react'; +import { Link } from 'react-router-dom'; + +const Index = () => { + return ( + <> + + navigate + + + ); +}; + +export default Index; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx new file mode 100644 index 000000000000..e8385e54dab5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx @@ -0,0 +1,37 @@ +import React from 'react'; + +export const someMoreNestedRoutes = [ + { + path: 'inner', + children: [ + { + index: true, + element: <>Level 1, + }, + { + path: ':id', + children: [ + { + index: true, + element: <>Level 1 ID, + }, + { + path: ':anotherId', + children: [ + { + index: true, + element: <>Level 1 ID Another ID, + }, + { + path: ':someAnotherId', + element:
+ Rendered +
, + }, + ], + }, + ], + }, + ], + }, +]; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts new file mode 100644 index 000000000000..6431bc5fc6b2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts @@ -0,0 +1 @@ +/// diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs new file mode 100644 index 000000000000..5e5ac71f169b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'react-router-7-lazy-routes', +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts new file mode 100644 index 000000000000..cee8f0128eb2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts @@ -0,0 +1,22 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('sends a navigation transaction for lazy route', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + console.debug('Lazy route transaction event', transactionEvent); + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + await page.goto('/'); + await page.locator('id=navigation').click(); + + const event = await transactionPromise; + + expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('navigation'); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json new file mode 100644 index 000000000000..4cc95dc2689a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json @@ -0,0 +1,20 @@ +{ + "compilerOptions": { + "target": "es2018", + "lib": ["dom", "dom.iterable", "esnext"], + "allowJs": true, + "skipLibCheck": true, + "esModuleInterop": true, + "allowSyntheticDefaultImports": true, + "strict": true, + "forceConsistentCasingInFileNames": true, + "noFallthroughCasesInSwitch": true, + "module": "esnext", + "moduleResolution": "node", + "resolveJsonModule": true, + "isolatedModules": true, + "noEmit": true, + "jsx": "react" + }, + "include": ["src", "tests"] +} diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index c711d9f3d613..a5adda357f9a 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -63,6 +63,62 @@ type V6CompatibleVersion = '6' | '7'; // Keeping as a global variable for cross-usage in multiple functions const allRoutes = new Set(); +/** + * Recursively checks a route for async handlers and sets up Proxies to add discovered child routes to allRoutes when called. + */ +export function checkRouteForAsyncHandler(route: RouteObject): void { + // If the route has a handle, set up proxies for any functions on it + if (route.handle && typeof route.handle === 'object') { + for (const key of Object.keys(route.handle)) { + const maybeFn = route.handle[key]; + if (typeof maybeFn === 'function') { + // Create a proxy that intercepts function calls + route.handle[key] = new Proxy(maybeFn, { + apply(target, thisArg, argArray) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const result = target.apply(thisArg, argArray); + // If the result is a promise, handle it when resolved + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (result && typeof result.then === 'function') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + result + .then((resolvedRoutes: unknown) => { + if (Array.isArray(resolvedRoutes)) { + // Add resolved routes as children to maintain proper route tree structure + route.children = resolvedRoutes; + // Add discovered children to allRoutes + resolvedRoutes.forEach(child => { + allRoutes.add(child); + // Recursively check for async handlers in child routes + checkRouteForAsyncHandler(child); + }); + } + }) + .catch((e: unknown) => { + DEBUG_BUILD && debug.warn(`Error resolving async handler '${key}' for route`, route, e); + }); + } else if (Array.isArray(result)) { + // Handle synchronous array results - add as children to maintain route tree structure + route.children = result; + result.forEach(child => { + allRoutes.add(child); + checkRouteForAsyncHandler(child); + }); + } + return result; + }, + }); + } + } + } + // If the route has children, check them recursively + if (Array.isArray(route.children)) { + for (const child of route.children) { + checkRouteForAsyncHandler(child); + } + } +} + /** * Creates a wrapCreateBrowserRouter function that can be used with all React Router v6 compatible versions. */ @@ -85,6 +141,16 @@ export function createV6CompatibleWrapCreateBrowserRouter< return function (routes: RouteObject[], opts?: Record & { basename?: string }): TRouter { addRoutesToAllRoutes(routes); + // Check for async handlers that might contain sub-route declarations + const checkAsyncHandlers = (): void => { + for (const route of routes) { + checkRouteForAsyncHandler(route); + } + }; + + // Start checking async handlers + checkAsyncHandlers(); + const router = createRouterFunction(routes, opts); const basename = opts?.basename; diff --git a/packages/react/src/types.ts b/packages/react/src/types.ts index 19aacffc5ac3..c25ee5df1ae3 100644 --- a/packages/react/src/types.ts +++ b/packages/react/src/types.ts @@ -13,6 +13,7 @@ export type Location = { export interface NonIndexRouteObject { caseSensitive?: boolean; children?: RouteObject[]; + handle?: Record; element?: React.ReactNode | null; errorElement?: React.ReactNode | null; index?: any; @@ -22,6 +23,7 @@ export interface NonIndexRouteObject { export interface IndexRouteObject { caseSensitive?: boolean; children?: undefined; + handle?: Record; element?: React.ReactNode | null; errorElement?: React.ReactNode | null; index: any; From 0c5f0bca64c9a833b1d3b794bac81220d4a9d276 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 1 Aug 2025 19:58:43 +0100 Subject: [PATCH 02/10] Make the implementation more readable. --- .../react-router-7-lazy-routes/package.json | 2 +- .../react/src/reactrouterv6-compat-utils.tsx | 139 ++++++++++++------ 2 files changed, 96 insertions(+), 45 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json index dc61a85199e9..1c38ac1468ec 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json @@ -1,5 +1,5 @@ { - "name": "react-router-7-cross-usage", + "name": "react-router-7-lazy-routes", "version": "0.1.0", "private": true, "dependencies": { diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index a5adda357f9a..6c90fa3af4fa 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -64,54 +64,105 @@ type V6CompatibleVersion = '6' | '7'; const allRoutes = new Set(); /** - * Recursively checks a route for async handlers and sets up Proxies to add discovered child routes to allRoutes when called. + * Handles the result of an async handler function call. */ -export function checkRouteForAsyncHandler(route: RouteObject): void { - // If the route has a handle, set up proxies for any functions on it - if (route.handle && typeof route.handle === 'object') { - for (const key of Object.keys(route.handle)) { - const maybeFn = route.handle[key]; - if (typeof maybeFn === 'function') { - // Create a proxy that intercepts function calls - route.handle[key] = new Proxy(maybeFn, { - apply(target, thisArg, argArray) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const result = target.apply(thisArg, argArray); - // If the result is a promise, handle it when resolved - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (result && typeof result.then === 'function') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - result - .then((resolvedRoutes: unknown) => { - if (Array.isArray(resolvedRoutes)) { - // Add resolved routes as children to maintain proper route tree structure - route.children = resolvedRoutes; - // Add discovered children to allRoutes - resolvedRoutes.forEach(child => { - allRoutes.add(child); - // Recursively check for async handlers in child routes - checkRouteForAsyncHandler(child); - }); - } - }) - .catch((e: unknown) => { - DEBUG_BUILD && debug.warn(`Error resolving async handler '${key}' for route`, route, e); - }); - } else if (Array.isArray(result)) { - // Handle synchronous array results - add as children to maintain route tree structure - route.children = result; - result.forEach(child => { - allRoutes.add(child); - checkRouteForAsyncHandler(child); - }); - } - return result; - }, - }); +function handleAsyncHandlerResult(result: unknown, route: RouteObject, handlerKey: string): void { + if ( + result && + typeof result === 'object' && + 'then' in result && + typeof (result as Promise).then === 'function' + ) { + handlePromiseResult(result as Promise, route, handlerKey); + } else if (Array.isArray(result)) { + handleSynchronousArrayResult(result, route); + } +} + +/** + * Handles promise results from async handlers. + */ +function handlePromiseResult(promise: Promise, route: RouteObject, handlerKey: string): void { + promise + .then((resolvedRoutes: unknown) => { + if (Array.isArray(resolvedRoutes)) { + addResolvedRoutesToParent(resolvedRoutes, route); + processResolvedRoutes(resolvedRoutes); } + }) + .catch((e: unknown) => { + DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e); + }); +} + +/** + * Handles synchronous array results from handlers. + */ +function handleSynchronousArrayResult(result: RouteObject[], route: RouteObject): void { + addResolvedRoutesToParent(result, route); + processResolvedRoutes(result); +} + +/** + * Adds resolved routes as children to the parent route. + */ +function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void { + parentRoute.children = Array.isArray(parentRoute.children) + ? [...parentRoute.children, ...resolvedRoutes] + : resolvedRoutes; +} + +/** + * Processes resolved routes by adding them to allRoutes and checking for nested async handlers. + */ +function processResolvedRoutes(resolvedRoutes: RouteObject[]): void { + resolvedRoutes.forEach(child => { + allRoutes.add(child); + checkRouteForAsyncHandler(child); + }); +} + +/** + * Creates a proxy wrapper for an async handler function. + */ +function createAsyncHandlerProxy( + originalFunction: (...args: unknown[]) => unknown, + route: RouteObject, + handlerKey: string, +): (...args: unknown[]) => unknown { + return new Proxy(originalFunction, { + apply(target: (...args: unknown[]) => unknown, thisArg, argArray) { + const result = target.apply(thisArg, argArray); + handleAsyncHandlerResult(result, route, handlerKey); + return result; + }, + }); +} + +/** + * Sets up proxies for all function properties in a route's handle object. + */ +function setupHandleProxies(route: RouteObject): void { + if (!route.handle || typeof route.handle !== 'object') { + return; + } + + for (const key of Object.keys(route.handle)) { + const maybeFn = route.handle[key]; + if (typeof maybeFn === 'function') { + route.handle[key] = createAsyncHandlerProxy(maybeFn, route, key); } } - // If the route has children, check them recursively +} + +/** + * Recursively checks a route for async handlers and sets up Proxies to add discovered child routes to allRoutes when called. + */ +export function checkRouteForAsyncHandler(route: RouteObject): void { + // Set up proxies for any functions in the route's handle + setupHandleProxies(route); + + // Recursively check child routes if (Array.isArray(route.children)) { for (const child of route.children) { checkRouteForAsyncHandler(child); From 244a953a11d5de10110c6e667a27fe555728965a Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 1 Aug 2025 20:00:52 +0100 Subject: [PATCH 03/10] Remove updates on other test apps. --- .../react-router-6/tests/lazy.test.ts | 37 ------------------- .../react-router-7-cross-usage/src/index.tsx | 14 ------- .../src/pages/InnerLazyRoutes.tsx | 37 ------------------- .../tests/transactions.test.ts | 23 ------------ 4 files changed, 111 deletions(-) delete mode 100644 dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts delete mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts b/dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts deleted file mode 100644 index 8221c4c162b0..000000000000 --- a/dev-packages/e2e-tests/test-applications/react-router-6/tests/lazy.test.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { expect, test } from '@playwright/test'; -import { waitForTransaction } from '@sentry-internal/test-utils'; - -test('should capture pageload transaction for lazy route', async ({ page }) => { - const transactionPromise = waitForTransaction('react-router-6', async transactionEvent => { - return ( - !!transactionEvent?.transaction && - transactionEvent.contexts?.trace?.op === 'pageload' && - transactionEvent.transaction === '/lazy/inner-lazy/inner' - ); - }); - - // Navigate to a lazy-loaded route - try just /lazy first - await page.goto('/lazy'); - - // Debug: Check what's actually rendered on the page - const pageContent = await page.content(); - console.log('Page content for /lazy:', pageContent); - - // Check if any content is rendered - use a more general selector - await page.waitForLoadState('networkidle'); - const bodyContent = await page.locator('body').innerHTML(); - console.log('Body content:', bodyContent); - - // Try to navigate to the full nested path - await page.goto('/lazy/inner-lazy/inner'); - await page.waitForLoadState('networkidle'); - - const nestedPageContent = await page.content(); - console.log('Nested page content:', nestedPageContent); - - const event = await transactionPromise; - - expect(event.transaction).toBe('/lazy/inner-lazy'); - expect(event.type).toBe('transaction'); - expect(event.contexts?.trace?.op).toBe('pageload'); -}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx index 4085e1c3568c..bfcc527ded1b 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/index.tsx @@ -77,19 +77,6 @@ const ViewsRoutes = () => }, ]); -const LazyRoutes = () => - sentryUseRoutes([ - { - path: 'inner-lazy', - handle: { - lazyChildren: async () => - import('./pages/InnerLazyRoutes').then( - (module) => module.someMoreNestedRoutes, - ), - }, - } - ]); - const ProjectsRoutes = () => ( }> @@ -98,7 +85,6 @@ const ProjectsRoutes = () => ( } /> - } /> ); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx deleted file mode 100644 index 006af4e67b5b..000000000000 --- a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/src/pages/InnerLazyRoutes.tsx +++ /dev/null @@ -1,37 +0,0 @@ -import React from 'react'; - -export const someMoreNestedRoutes = [ - { - path: 'level-1', - children: [ - { - index: true, - element: <>Level 1, - }, - { - path: ':id', - children: [ - { - index: true, - element: <>Level 1 ID, - }, - { - path: ':anotherId', - children: [ - { - index: true, - element: <>Level 1 ID Another ID, - }, - { - path: ':someAnotherId', - element:
- Level 1 ID Another ID Some Another ID -
, - }, - ], - }, - ], - }, - ], - }, -]; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts index 1c48425f2e6f..1b521964f770 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-cross-usage/tests/transactions.test.ts @@ -136,26 +136,3 @@ test('sends a navigation transaction with a parameterized URL - alternative rout }, }); }); - -test('sends a pageload transaction for lazy route', async ({ page }) => { - const transactionPromise = waitForTransaction('react-router-7-cross-usage', async transactionEvent => { - console.debug('Lazy route transaction event', transactionEvent); - return ( - !!transactionEvent?.transaction && - transactionEvent.contexts?.trace?.op === 'pageload' && - transactionEvent.transaction === '/lazy/inner-lazy/inner-inner-lazy' - ); - }); - - // Try to navigate to the full nested path - await page.goto('/lazy/level-1/123/456/789'); - - console.debug('Navigated to lazy route'); - console.debug('ROOT', await page.innerHTML('#root')); - - const event = await transactionPromise; - - expect(event.transaction).toBe('/lazy/inner-lazy/inner-inner-lazy'); - expect(event.type).toBe('transaction'); - expect(event.contexts?.trace?.op).toBe('pageload'); -}); From d8216c1158872e30d413468cf136112094a55a35 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 4 Aug 2025 10:55:41 +0100 Subject: [PATCH 04/10] Simplify --- .../react/src/reactrouterv6-compat-utils.tsx | 81 +++++-------------- 1 file changed, 21 insertions(+), 60 deletions(-) diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index 6c90fa3af4fa..517be14ea391 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -73,45 +73,20 @@ function handleAsyncHandlerResult(result: unknown, route: RouteObject, handlerKe 'then' in result && typeof (result as Promise).then === 'function' ) { - handlePromiseResult(result as Promise, route, handlerKey); + (result as Promise) + .then((resolvedRoutes: unknown) => { + if (Array.isArray(resolvedRoutes)) { + processResolvedRoutes(resolvedRoutes); + } + }) + .catch((e: unknown) => { + DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e); + }); } else if (Array.isArray(result)) { - handleSynchronousArrayResult(result, route); + processResolvedRoutes(result); } } -/** - * Handles promise results from async handlers. - */ -function handlePromiseResult(promise: Promise, route: RouteObject, handlerKey: string): void { - promise - .then((resolvedRoutes: unknown) => { - if (Array.isArray(resolvedRoutes)) { - addResolvedRoutesToParent(resolvedRoutes, route); - processResolvedRoutes(resolvedRoutes); - } - }) - .catch((e: unknown) => { - DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e); - }); -} - -/** - * Handles synchronous array results from handlers. - */ -function handleSynchronousArrayResult(result: RouteObject[], route: RouteObject): void { - addResolvedRoutesToParent(result, route); - processResolvedRoutes(result); -} - -/** - * Adds resolved routes as children to the parent route. - */ -function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void { - parentRoute.children = Array.isArray(parentRoute.children) - ? [...parentRoute.children, ...resolvedRoutes] - : resolvedRoutes; -} - /** * Processes resolved routes by adding them to allRoutes and checking for nested async handlers. */ @@ -139,28 +114,19 @@ function createAsyncHandlerProxy( }); } -/** - * Sets up proxies for all function properties in a route's handle object. - */ -function setupHandleProxies(route: RouteObject): void { - if (!route.handle || typeof route.handle !== 'object') { - return; - } - - for (const key of Object.keys(route.handle)) { - const maybeFn = route.handle[key]; - if (typeof maybeFn === 'function') { - route.handle[key] = createAsyncHandlerProxy(maybeFn, route, key); - } - } -} - /** * Recursively checks a route for async handlers and sets up Proxies to add discovered child routes to allRoutes when called. */ export function checkRouteForAsyncHandler(route: RouteObject): void { // Set up proxies for any functions in the route's handle - setupHandleProxies(route); + if (route.handle && typeof route.handle === 'object') { + for (const key of Object.keys(route.handle)) { + const maybeFn = route.handle[key]; + if (typeof maybeFn === 'function') { + route.handle[key] = createAsyncHandlerProxy(maybeFn, route, key); + } + } + } // Recursively check child routes if (Array.isArray(route.children)) { @@ -193,14 +159,9 @@ export function createV6CompatibleWrapCreateBrowserRouter< addRoutesToAllRoutes(routes); // Check for async handlers that might contain sub-route declarations - const checkAsyncHandlers = (): void => { - for (const route of routes) { - checkRouteForAsyncHandler(route); - } - }; - - // Start checking async handlers - checkAsyncHandlers(); + for (const route of routes) { + checkRouteForAsyncHandler(route); + } const router = createRouterFunction(routes, opts); const basename = opts?.basename; From 06e535e6e216c12b6a0f6187e7fd81171e9ad1f2 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 5 Aug 2025 06:43:19 +0100 Subject: [PATCH 05/10] Add support for parameterized `pageload` transactions --- .../react-router-7-lazy-routes/src/index.tsx | 35 +- .../tests/transactions.test.ts | 42 ++- .../react/src/reactrouterv6-compat-utils.tsx | 47 ++- .../test/reactrouterv6-compat-utils.test.tsx | 306 +++++++++++++++++- 4 files changed, 400 insertions(+), 30 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx index c827707e51ff..02324fb77db9 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx @@ -46,27 +46,22 @@ const sentryCreateBrowserRouter = Sentry.wrapCreateBrowserRouterV7(createBrowser const router = sentryCreateBrowserRouter( [ { + path: '/', element: , - children: [ - { - path: '/', - element: , - }, - { - path: '/lazy', - handle: { - lazyChildren: () => import('./pages/InnerLazyRoutes').then(module => module.someMoreNestedRoutes), - }, - }, - { - path: '/static', - element: <>Hello World, - }, - { - path: '*', - element: , - }, - ], + }, + { + path: '/lazy', + handle: { + lazyChildren: () => import('./pages/InnerLazyRoutes').then(module => module.someMoreNestedRoutes), + }, + }, + { + path: '/static', + element: <>Hello World, + }, + { + path: '*', + element: , }, ], { diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts index cee8f0128eb2..b5cc44795a0b 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts @@ -1,9 +1,34 @@ import { expect, test } from '@playwright/test'; import { waitForTransaction } from '@sentry-internal/test-utils'; -test('sends a navigation transaction for lazy route', async ({ page }) => { + +test('Creates a pageload transaction with parameterized route', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + console.debug('transactionEvent', transactionEvent); + + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + await page.goto('/lazy/inner/1/2/3'); + const event = await transactionPromise; + + + const lazyRouteContent = page.locator('id=innermost-lazy-route'); + + await expect(lazyRouteContent).toBeVisible(); + + // Validate the transaction event + expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('pageload'); +}); + +test('Creates a navigation transaction inside a lazy route', async ({ page }) => { const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { - console.debug('Lazy route transaction event', transactionEvent); return ( !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation' && @@ -12,10 +37,21 @@ test('sends a navigation transaction for lazy route', async ({ page }) => { }); await page.goto('/'); - await page.locator('id=navigation').click(); + // Check if the navigation link exists + const navigationLink = page.locator('id=navigation'); + await expect(navigationLink).toBeVisible(); + + // Click the navigation link to navigate to the lazy route + await navigationLink.click(); const event = await transactionPromise; + // Check if the lazy route content is rendered + const lazyRouteContent = page.locator('id=innermost-lazy-route'); + + await expect(lazyRouteContent).toBeVisible(); + + // Validate the transaction event expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); expect(event.type).toBe('transaction'); expect(event.contexts?.trace?.op).toBe('navigation'); diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index 517be14ea391..fe5311b43cec 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -10,6 +10,7 @@ import { } from '@sentry/browser'; import type { Client, Integration, Span, TransactionSource } from '@sentry/core'; import { + addNonEnumerableProperty, debug, getActiveSpan, getClient, @@ -63,6 +64,15 @@ type V6CompatibleVersion = '6' | '7'; // Keeping as a global variable for cross-usage in multiple functions const allRoutes = new Set(); +/** + * Adds resolved routes as children to the parent route. + */ +function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void { + parentRoute.children = Array.isArray(parentRoute.children) + ? [...parentRoute.children, ...resolvedRoutes] + : resolvedRoutes; +} + /** * Handles the result of an async handler function call. */ @@ -76,25 +86,47 @@ function handleAsyncHandlerResult(result: unknown, route: RouteObject, handlerKe (result as Promise) .then((resolvedRoutes: unknown) => { if (Array.isArray(resolvedRoutes)) { - processResolvedRoutes(resolvedRoutes); + processResolvedRoutes(resolvedRoutes, route); } }) .catch((e: unknown) => { DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e); }); } else if (Array.isArray(result)) { - processResolvedRoutes(result); + processResolvedRoutes(result, route); } } /** * Processes resolved routes by adding them to allRoutes and checking for nested async handlers. */ -function processResolvedRoutes(resolvedRoutes: RouteObject[]): void { +function processResolvedRoutes(resolvedRoutes: RouteObject[], parentRoute?: RouteObject): void { resolvedRoutes.forEach(child => { allRoutes.add(child); checkRouteForAsyncHandler(child); }); + + if (parentRoute) { + // If a parent route is provided, add the resolved routes as children to the parent route + addResolvedRoutesToParent(resolvedRoutes, parentRoute); + } + + // After processing lazy routes, check if we need to update an active pageload transaction + const activeRootSpan = getActiveRootSpan(); + if (activeRootSpan && spanToJSON(activeRootSpan).op === 'pageload') { + const location = WINDOW.location; + if (location) { + // Re-run the pageload transaction update with the newly loaded routes + updatePageloadTransaction( + activeRootSpan, + { pathname: location.pathname }, + Array.from(allRoutes), + undefined, + undefined, + Array.from(allRoutes), + ); + } + } } /** @@ -105,13 +137,17 @@ function createAsyncHandlerProxy( route: RouteObject, handlerKey: string, ): (...args: unknown[]) => unknown { - return new Proxy(originalFunction, { + const proxy = new Proxy(originalFunction, { apply(target: (...args: unknown[]) => unknown, thisArg, argArray) { const result = target.apply(thisArg, argArray); handleAsyncHandlerResult(result, route, handlerKey); return result; }, }); + + addNonEnumerableProperty(proxy, '__sentry_proxied__', true); + + return proxy; } /** @@ -122,7 +158,7 @@ export function checkRouteForAsyncHandler(route: RouteObject): void { if (route.handle && typeof route.handle === 'object') { for (const key of Object.keys(route.handle)) { const maybeFn = route.handle[key]; - if (typeof maybeFn === 'function') { + if (typeof maybeFn === 'function' && !(maybeFn as { __sentry_proxied__?: boolean }).__sentry_proxied__) { route.handle[key] = createAsyncHandlerProxy(maybeFn, route, key); } } @@ -620,6 +656,7 @@ function getNormalizedName( } let pathBuilder = ''; + if (branches) { for (const branch of branches) { const route = branch.route; diff --git a/packages/react/test/reactrouterv6-compat-utils.test.tsx b/packages/react/test/reactrouterv6-compat-utils.test.tsx index 193b4aaab223..1f10d89c8558 100644 --- a/packages/react/test/reactrouterv6-compat-utils.test.tsx +++ b/packages/react/test/reactrouterv6-compat-utils.test.tsx @@ -1,5 +1,6 @@ -import { describe, expect } from 'vitest'; -import { getNumberOfUrlSegments } from '../src/reactrouterv6-compat-utils'; +import { describe, expect, it, test } from 'vitest'; +import { checkRouteForAsyncHandler, getNumberOfUrlSegments } from '../src/reactrouterv6-compat-utils'; +import type { RouteObject } from '../src/types'; describe('getNumberOfUrlSegments', () => { test.each([ @@ -11,3 +12,304 @@ describe('getNumberOfUrlSegments', () => { expect(getNumberOfUrlSegments(input)).toEqual(output); }); }); + +describe('checkRouteForAsyncHandler', () => { + it('should not create nested proxies when called multiple times on the same route', () => { + const mockHandler = () => Promise.resolve([]); + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: mockHandler, + }, + }; + + checkRouteForAsyncHandler(route); + checkRouteForAsyncHandler(route); + + const proxiedHandler = route.handle?.lazyChildren; + expect(typeof proxiedHandler).toBe('function'); + expect(proxiedHandler).not.toBe(mockHandler); + + expect((proxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + const proxyHandler = (proxiedHandler as any)?.__sentry_proxied__; + expect(proxyHandler).toBe(true); + }); + + it('should handle routes without handle property', () => { + const route: RouteObject = { + path: '/test', + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with non-function handle properties', () => { + const route: RouteObject = { + path: '/test', + handle: { + someData: 'not a function', + }, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with null/undefined handle properties', () => { + const route: RouteObject = { + path: '/test', + handle: null as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with mixed function and non-function handle properties', () => { + const mockHandler = () => Promise.resolve([]); + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: mockHandler, + someData: 'not a function', + anotherData: 123, + }, + }; + + checkRouteForAsyncHandler(route); + + const proxiedHandler = route.handle?.lazyChildren; + expect(typeof proxiedHandler).toBe('function'); + expect(proxiedHandler).not.toBe(mockHandler); + expect((proxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + // Non-function properties should remain unchanged + expect(route.handle?.someData).toBe('not a function'); + expect(route.handle?.anotherData).toBe(123); + }); + + it('should handle nested routes with async handlers', () => { + const parentHandler = () => Promise.resolve([]); + const childHandler = () => Promise.resolve([]); + + const route: RouteObject = { + path: '/parent', + handle: { + lazyChildren: parentHandler, + }, + children: [ + { + path: '/child', + handle: { + lazyChildren: childHandler, + }, + }, + ], + }; + + checkRouteForAsyncHandler(route); + + // Check parent handler is proxied + const proxiedParentHandler = route.handle?.lazyChildren; + expect(typeof proxiedParentHandler).toBe('function'); + expect(proxiedParentHandler).not.toBe(parentHandler); + expect((proxiedParentHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + // Check child handler is proxied + const proxiedChildHandler = route.children?.[0]?.handle?.lazyChildren; + expect(typeof proxiedChildHandler).toBe('function'); + expect(proxiedChildHandler).not.toBe(childHandler); + expect((proxiedChildHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + }); + + it('should handle deeply nested routes', () => { + const level1Handler = () => Promise.resolve([]); + const level2Handler = () => Promise.resolve([]); + const level3Handler = () => Promise.resolve([]); + + const route: RouteObject = { + path: '/level1', + handle: { + lazyChildren: level1Handler, + }, + children: [ + { + path: '/level2', + handle: { + lazyChildren: level2Handler, + }, + children: [ + { + path: '/level3', + handle: { + lazyChildren: level3Handler, + }, + }, + ], + }, + ], + }; + + checkRouteForAsyncHandler(route); + + // Check all handlers are proxied + expect((route.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + expect((route.children?.[0]?.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe( + true, + ); + expect( + (route.children?.[0]?.children?.[0]?.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__, + ).toBe(true); + }); + + it('should handle routes with multiple async handlers', () => { + const handler1 = () => Promise.resolve([]); + const handler2 = () => Promise.resolve([]); + const handler3 = () => Promise.resolve([]); + + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: handler1, + asyncLoader: handler2, + dataLoader: handler3, + }, + }; + + checkRouteForAsyncHandler(route); + + // Check all handlers are proxied + expect((route.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + expect((route.handle?.asyncLoader as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + expect((route.handle?.dataLoader as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + }); + + it('should not re-proxy already proxied functions', () => { + const mockHandler = () => Promise.resolve([]); + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: mockHandler, + }, + }; + + // First call should proxy the function + checkRouteForAsyncHandler(route); + const firstProxiedHandler = route.handle?.lazyChildren; + expect(firstProxiedHandler).not.toBe(mockHandler); + expect((firstProxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + // Second call should not create a new proxy + checkRouteForAsyncHandler(route); + const secondProxiedHandler = route.handle?.lazyChildren; + expect(secondProxiedHandler).toBe(firstProxiedHandler); // Should be the same proxy + expect((secondProxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + }); + + it('should handle routes with empty children array', () => { + const route: RouteObject = { + path: '/test', + children: [], + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with undefined children', () => { + const route: RouteObject = { + path: '/test', + children: undefined, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with null children', () => { + const route: RouteObject = { + path: '/test', + children: null as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with non-array children', () => { + const route: RouteObject = { + path: '/test', + children: 'not an array' as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is not an object', () => { + const route: RouteObject = { + path: '/test', + handle: 'not an object' as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is null', () => { + const route: RouteObject = { + path: '/test', + handle: null as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is undefined', () => { + const route: RouteObject = { + path: '/test', + handle: undefined as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a function', () => { + const route: RouteObject = { + path: '/test', + handle: (() => {}) as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a string', () => { + const route: RouteObject = { + path: '/test', + handle: 'string handle' as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a number', () => { + const route: RouteObject = { + path: '/test', + handle: 42 as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a boolean', () => { + const route: RouteObject = { + path: '/test', + handle: true as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is an array', () => { + const route: RouteObject = { + path: '/test', + handle: [] as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); +}); From e968adf69a54ea33821c0978a010d866ac75b41d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 5 Aug 2025 06:57:57 +0100 Subject: [PATCH 06/10] Prevent duplicate children in `addResolvedRoutesToParent` --- .../tests/transactions.test.ts | 2 -- .../react/src/reactrouterv6-compat-utils.tsx | 19 ++++++++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts index b5cc44795a0b..74a1fcff1faa 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts @@ -4,8 +4,6 @@ import { waitForTransaction } from '@sentry-internal/test-utils'; test('Creates a pageload transaction with parameterized route', async ({ page }) => { const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { - console.debug('transactionEvent', transactionEvent); - return ( !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload' && diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index fe5311b43cec..5b46467e3840 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -66,11 +66,24 @@ const allRoutes = new Set(); /** * Adds resolved routes as children to the parent route. + * Prevents duplicate routes by checking if they already exist. */ function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void { - parentRoute.children = Array.isArray(parentRoute.children) - ? [...parentRoute.children, ...resolvedRoutes] - : resolvedRoutes; + const existingChildren = parentRoute.children || []; + + const newRoutes = resolvedRoutes.filter( + newRoute => + !existingChildren.some( + existing => + existing === newRoute || + (newRoute.path && existing.path === newRoute.path) || + (newRoute.id && existing.id === newRoute.id), + ), + ); + + if (newRoutes.length > 0) { + parentRoute.children = [...existingChildren, ...newRoutes]; + } } /** From 2e0a52121e76d418e0c2ece13993467d5ffe40f5 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 6 Aug 2025 10:20:57 +0100 Subject: [PATCH 07/10] Only run async handler checks when it's enabled on the integration --- .../react-router-7-lazy-routes/src/index.tsx | 1 + .../react/src/reactrouterv6-compat-utils.tsx | 24 +++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx index 02324fb77db9..c45da9bd0933 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx @@ -26,6 +26,7 @@ Sentry.init({ createRoutesFromChildren, matchRoutes, trackFetchStreamPerformance: true, + enableAsyncRouteHandlers: true, }), replay, ], diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index 5b46467e3840..ab0ddcfb8988 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -47,6 +47,7 @@ let _useNavigationType: UseNavigationType; let _createRoutesFromChildren: CreateRoutesFromChildren; let _matchRoutes: MatchRoutes; let _stripBasename: boolean = false; +let _enableAsyncRouteHandlers: boolean = false; const CLIENTS_WITH_INSTRUMENT_NAVIGATION = new WeakSet(); @@ -57,6 +58,7 @@ export interface ReactRouterOptions { createRoutesFromChildren: CreateRoutesFromChildren; matchRoutes: MatchRoutes; stripBasename?: boolean; + enableAsyncRouteHandlers?: boolean; } type V6CompatibleVersion = '6' | '7'; @@ -116,7 +118,10 @@ function handleAsyncHandlerResult(result: unknown, route: RouteObject, handlerKe function processResolvedRoutes(resolvedRoutes: RouteObject[], parentRoute?: RouteObject): void { resolvedRoutes.forEach(child => { allRoutes.add(child); - checkRouteForAsyncHandler(child); + // Only check for async handlers if the feature is enabled + if (_enableAsyncRouteHandlers) { + checkRouteForAsyncHandler(child); + } }); if (parentRoute) { @@ -207,9 +212,11 @@ export function createV6CompatibleWrapCreateBrowserRouter< return function (routes: RouteObject[], opts?: Record & { basename?: string }): TRouter { addRoutesToAllRoutes(routes); - // Check for async handlers that might contain sub-route declarations - for (const route of routes) { - checkRouteForAsyncHandler(route); + // Check for async handlers that might contain sub-route declarations (only if enabled) + if (_enableAsyncRouteHandlers) { + for (const route of routes) { + checkRouteForAsyncHandler(route); + } } const router = createRouterFunction(routes, opts); @@ -291,6 +298,13 @@ export function createV6CompatibleWrapCreateMemoryRouter< ): TRouter { addRoutesToAllRoutes(routes); + // Check for async handlers that might contain sub-route declarations (only if enabled) + if (_enableAsyncRouteHandlers) { + for (const route of routes) { + checkRouteForAsyncHandler(route); + } + } + const router = createRouterFunction(routes, opts); const basename = opts?.basename; @@ -357,6 +371,7 @@ export function createReactRouterV6CompatibleTracingIntegration( createRoutesFromChildren, matchRoutes, stripBasename, + enableAsyncRouteHandlers = false, instrumentPageLoad = true, instrumentNavigation = true, } = options; @@ -372,6 +387,7 @@ export function createReactRouterV6CompatibleTracingIntegration( _matchRoutes = matchRoutes; _createRoutesFromChildren = createRoutesFromChildren; _stripBasename = stripBasename || false; + _enableAsyncRouteHandlers = enableAsyncRouteHandlers; }, afterAllSetup(client) { integration.afterAllSetup(client); From 739e50ae02f68fb9ead143aac06931c1fd8481a7 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 7 Aug 2025 20:28:39 +0100 Subject: [PATCH 08/10] Add JSDocs for `stripBasename` and `enableAsyncRouteHandlers` --- packages/react/src/reactrouterv6-compat-utils.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index ab0ddcfb8988..f364facca1db 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -57,7 +57,19 @@ export interface ReactRouterOptions { useNavigationType: UseNavigationType; createRoutesFromChildren: CreateRoutesFromChildren; matchRoutes: MatchRoutes; + /** + * Whether to strip the basename from the pathname when creating transactions. + * + * This is useful for applications that use a basename in their routing setup. + * @default false + */ stripBasename?: boolean; + /** + * Enables support for async route handlers. + * + * This allows Sentry to track and instrument routes dynamically resolved from async handlers. + * @default false + */ enableAsyncRouteHandlers?: boolean; } From 8815203535c56c6f5643c2f05a61e5dbc9212008 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 7 Aug 2025 20:31:27 +0100 Subject: [PATCH 09/10] Remove replay from e2e test app --- .../react-router-7-lazy-routes/src/index.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx index c45da9bd0933..ae790a922abc 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx @@ -13,7 +13,6 @@ import { } from 'react-router-dom'; import Index from './pages/Index'; -const replay = Sentry.replayIntegration(); Sentry.init({ environment: 'qa', // dynamic sampling bias to keep transactions @@ -28,17 +27,12 @@ Sentry.init({ trackFetchStreamPerformance: true, enableAsyncRouteHandlers: true, }), - replay, ], // We recommend adjusting this value in production, or using tracesSampler // for finer control tracesSampleRate: 1.0, release: 'e2e-test', - // Always capture replays, so we can test this properly - replaysSessionSampleRate: 1.0, - replaysOnErrorSampleRate: 0.0, - tunnel: 'http://localhost:3031', }); From 1bb16303c145fd6e12a037e39e8ab9767f020a3f Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 7 Aug 2025 20:41:03 +0100 Subject: [PATCH 10/10] Remove unnecessary escape --- .../test-applications/react-router-7-lazy-routes/src/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx index ae790a922abc..4570c23d06f5 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx @@ -56,7 +56,7 @@ const router = sentryCreateBrowserRouter( }, { path: '*', - element: , + element: , }, ], {