From 5028435b014b1d8ec24b402cbb99574f2e774847 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 19 Jan 2026 12:17:28 +0000 Subject: [PATCH 1/5] fix(react): Defer React Router span finalization until lazy routes load --- .../react-router-7-lazy-routes/src/index.tsx | 14 ++ .../src/pages/WildcardLazyRoutes.tsx | 17 ++ .../tests/transactions.test.ts | 46 +++++ .../instrumentation.tsx | 194 ++++++++++++++---- .../instrumentation.test.tsx | 88 ++++++++ 5 files changed, 319 insertions(+), 40 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/WildcardLazyRoutes.tsx 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 1bcad5eaf4ce..a35a1b8ae077 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 @@ -134,6 +134,20 @@ const router = sentryCreateBrowserRouter( lazyChildren: () => import('./pages/SlowFetchLazyRoutes').then(module => module.slowFetchRoutes), }, }, + { + // Route with wildcard placeholder that gets replaced by lazy-loaded parameterized routes + // This tests that wildcard transaction names get upgraded to parameterized routes + path: '/wildcard-lazy', + children: [ + { + path: '*', // Catch-all wildcard - will be matched initially before lazy routes load + element: <>Loading..., + }, + ], + handle: { + lazyChildren: () => import('./pages/WildcardLazyRoutes').then(module => module.wildcardRoutes), + }, + }, ], { async patchRoutesOnNavigation({ matches, patch }: Parameters[0]) { diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/WildcardLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/WildcardLazyRoutes.tsx new file mode 100644 index 000000000000..8be773e6613a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/WildcardLazyRoutes.tsx @@ -0,0 +1,17 @@ +import React from 'react'; +import { useParams } from 'react-router-dom'; + +// Simulate slow lazy route loading (500ms delay via top-level await) +await new Promise(resolve => setTimeout(resolve, 500)); + +function WildcardItem() { + const { id } = useParams(); + return
Wildcard Item: {id}
; +} + +export const wildcardRoutes = [ + { + path: ':id', + 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 f7a3ec4a5519..9e19d84b7391 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 @@ -1228,3 +1228,49 @@ test('Query/hash navigation does not corrupt transaction name', async ({ page }) const corruptedToRoot = navigationTransactions.filter(t => t.name === '/'); expect(corruptedToRoot.length).toBe(0); }); + +// Regression: Pageload to slow lazy route should get parameterized name even if span ends early +test('Slow lazy route pageload with early span end still gets parameterized route name (regression)', async ({ + page, +}) => { + const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + (transactionEvent.transaction?.startsWith('/slow-fetch') ?? false) + ); + }); + + // idleTimeout=300 ends span before 500ms lazy route loads, timeout=1000 waits for lazy routes + await page.goto('/slow-fetch/123?idleTimeout=300&timeout=1000'); + + const event = await transactionPromise; + + expect(event.transaction).toBe('/slow-fetch/:id'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('pageload'); + expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route'); + + const idleSpanFinishReason = event.contexts?.trace?.data?.['sentry.idle_span_finish_reason']; + expect(['idleTimeout', 'externalFinish']).toContain(idleSpanFinishReason); +}); + +// Regression: Wildcard route names should be upgraded to parameterized routes when lazy routes load +test('Wildcard route pageload gets upgraded to parameterized route name (regression)', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + (transactionEvent.transaction?.startsWith('/wildcard-lazy') ?? false) + ); + }); + + await page.goto('/wildcard-lazy/456?idleTimeout=300&timeout=1000'); + + const event = await transactionPromise; + + expect(event.transaction).toBe('/wildcard-lazy/:id'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('pageload'); + expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route'); +}); diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index d646624618f9..121fe95abb73 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -71,6 +71,9 @@ export const allRoutes = new Set(); // Tracks lazy route loads to wait before finalizing span names const pendingLazyRouteLoads = new WeakMap>>(); +// Tracks deferred lazy route promises that can be resolved when patchRoutesOnNavigation is called +const deferredLazyRouteResolvers = new WeakMap void>(); + /** * Schedules a callback using requestAnimationFrame when available (browser), * or falls back to setTimeout for SSR environments (Node.js, createMemoryRouter tests). @@ -233,6 +236,32 @@ function trackLazyRouteLoad(span: Span, promise: Promise): void { }); } +/** + * Creates a deferred promise for a span that will be resolved when patchRoutesOnNavigation is called. + * This ensures that patchedEnd waits for patchRoutesOnNavigation to be called before ending the span. + */ +function createDeferredLazyRoutePromise(span: Span): void { + let resolvePromise: () => void; + const deferredPromise = new Promise(resolve => { + resolvePromise = resolve; + }); + + deferredLazyRouteResolvers.set(span, resolvePromise!); + trackLazyRouteLoad(span, deferredPromise); +} + +/** + * Resolves the deferred lazy route promise for a span. + * Called when patchRoutesOnNavigation is invoked. + */ +function resolveDeferredLazyRoutePromise(span: Span): void { + const resolver = deferredLazyRouteResolvers.get(span); + if (resolver) { + resolver(); + deferredLazyRouteResolvers.delete(span); + } +} + /** * Processes resolved routes by adding them to allRoutes and checking for nested async handlers. * When capturedSpan is provided, updates that specific span instead of the current active span. @@ -454,10 +483,30 @@ export function createV6CompatibleWrapCreateBrowserRouter< } } - const wrappedOpts = wrapPatchRoutesOnNavigation(opts); + // Capture the active span BEFORE creating the router. + // This is important because the span might end (due to idle timeout) before + // patchRoutesOnNavigation is called by React Router. + const activeRootSpan = getActiveRootSpan(); + + // If patchRoutesOnNavigation is provided and we have an active span, + // mark the span as having potential lazy routes and create a deferred promise. + const hasPatchRoutesOnNavigation = + opts && 'patchRoutesOnNavigation' in opts && typeof opts.patchRoutesOnNavigation === 'function'; + if (hasPatchRoutesOnNavigation && activeRootSpan) { + // Mark the span as potentially having lazy routes + addNonEnumerableProperty( + activeRootSpan as unknown as Record, + '__sentry_may_have_lazy_routes__', + true, + ); + createDeferredLazyRoutePromise(activeRootSpan); + } + + // Pass the captured span to wrapPatchRoutesOnNavigation so it uses the same span + // even if the span has ended by the time patchRoutesOnNavigation is called. + const wrappedOpts = wrapPatchRoutesOnNavigation(opts, false, activeRootSpan); const router = createRouterFunction(routes, wrappedOpts); const basename = opts?.basename; - const activeRootSpan = getActiveRootSpan(); if (router.state.historyAction === 'POP' && activeRootSpan) { updatePageloadTransaction({ @@ -510,7 +559,23 @@ export function createV6CompatibleWrapCreateMemoryRouter< } } - const wrappedOpts = wrapPatchRoutesOnNavigation(opts, true); + // Capture the active span BEFORE creating the router (same as browser router) + const memoryActiveRootSpanEarly = getActiveRootSpan(); + + // If patchRoutesOnNavigation is provided and we have an active span, + // mark the span as having potential lazy routes and create a deferred promise. + const hasPatchRoutesOnNavigation = + opts && 'patchRoutesOnNavigation' in opts && typeof opts.patchRoutesOnNavigation === 'function'; + if (hasPatchRoutesOnNavigation && memoryActiveRootSpanEarly) { + addNonEnumerableProperty( + memoryActiveRootSpanEarly as unknown as Record, + '__sentry_may_have_lazy_routes__', + true, + ); + createDeferredLazyRoutePromise(memoryActiveRootSpanEarly); + } + + const wrappedOpts = wrapPatchRoutesOnNavigation(opts, true, memoryActiveRootSpanEarly); const router = createRouterFunction(routes, wrappedOpts); const basename = opts?.basename; @@ -709,6 +774,7 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio function wrapPatchRoutesOnNavigation( opts: Record | undefined, isMemoryRouter = false, + capturedSpan?: Span, ): Record { if (!opts || !('patchRoutesOnNavigation' in opts) || typeof opts.patchRoutesOnNavigation !== 'function') { return opts || {}; @@ -721,29 +787,53 @@ function wrapPatchRoutesOnNavigation( // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access const targetPath = (args as any)?.path; - const activeRootSpan = getActiveRootSpan(); + // Use current active span if available, otherwise fall back to captured span (from router creation time). + // This ensures navigation spans use their own span (not the stale pageload span), while still + // supporting pageload spans that may have ended before patchRoutesOnNavigation is called. + const activeRootSpan = getActiveRootSpan() ?? capturedSpan; if (!isMemoryRouter) { // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access const originalPatch = (args as any)?.patch; + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const matches = (args as any)?.matches as Array<{ route: RouteObject }> | undefined; if (originalPatch) { // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access (args as any).patch = (routeId: string, children: RouteObject[]) => { addRoutesToAllRoutes(children); + + // Find the parent route from matches and attach children to it in allRoutes. + // React Router's patch attaches children to its internal route copies, but we need + // to update the route objects in our allRoutes Set for proper route matching. + if (matches && matches.length > 0) { + const leafMatch = matches[matches.length - 1]; + if (leafMatch?.route) { + // Find the matching route in allRoutes by reference or path + for (const route of allRoutes) { + if (route === leafMatch.route || (route.path && route.path === leafMatch.route.path)) { + // Attach children to this parent route + addResolvedRoutesToParent(children, route); + break; + } + } + } + } + const currentActiveRootSpan = getActiveRootSpan(); // Only update if we have a valid targetPath (patchRoutesOnNavigation can be called without path) - if ( - targetPath && - currentActiveRootSpan && - (spanToJSON(currentActiveRootSpan) as { op?: string }).op === 'navigation' - ) { - updateNavigationSpan( - currentActiveRootSpan, - { pathname: targetPath, search: '', hash: '', state: null, key: 'default' }, - Array.from(allRoutes), - true, - _matchRoutes, - ); + if (targetPath && currentActiveRootSpan) { + const spanOp = (spanToJSON(currentActiveRootSpan) as { op?: string }).op; + const location = { pathname: targetPath, search: '', hash: '', state: null, key: 'default' }; + if (spanOp === 'navigation') { + updateNavigationSpan(currentActiveRootSpan, location, Array.from(allRoutes), true, _matchRoutes); + } else if (spanOp === 'pageload') { + updatePageloadTransaction({ + activeRootSpan: currentActiveRootSpan, + location, + routes: Array.from(allRoutes), + allRoutes: Array.from(allRoutes), + }); + } } return originalPatch(routeId, children); }; @@ -758,20 +848,30 @@ function wrapPatchRoutesOnNavigation( result = await originalPatchRoutes(args); } finally { clearNavigationContext(contextToken); + // Resolve the deferred promise now that patchRoutesOnNavigation has completed. + // This ensures patchedEnd has waited long enough for the lazy routes to load. + if (activeRootSpan) { + resolveDeferredLazyRoutePromise(activeRootSpan); + } } const currentActiveRootSpan = getActiveRootSpan(); - if (currentActiveRootSpan && (spanToJSON(currentActiveRootSpan) as { op?: string }).op === 'navigation') { + if (currentActiveRootSpan) { + const spanOp = (spanToJSON(currentActiveRootSpan) as { op?: string }).op; const pathname = isMemoryRouter ? targetPath : targetPath || WINDOW.location?.pathname; if (pathname) { - updateNavigationSpan( - currentActiveRootSpan, - { pathname, search: '', hash: '', state: null, key: 'default' }, - Array.from(allRoutes), - false, - _matchRoutes, - ); + const location = { pathname, search: '', hash: '', state: null, key: 'default' }; + if (spanOp === 'navigation') { + updateNavigationSpan(currentActiveRootSpan, location, Array.from(allRoutes), false, _matchRoutes); + } else if (spanOp === 'pageload') { + updatePageloadTransaction({ + activeRootSpan: currentActiveRootSpan, + location, + routes: Array.from(allRoutes), + allRoutes: Array.from(allRoutes), + }); + } } } @@ -893,7 +993,7 @@ export function handleNavigation(opts: { pathname: location.pathname, locationKey, }); - patchSpanEnd(navigationSpan, location, routes, basename, allRoutes, 'navigation'); + patchSpanEnd(navigationSpan, location, routes, basename, 'navigation'); } else { // If no span was created, remove the placeholder activeNavigationSpans.delete(client); @@ -965,8 +1065,13 @@ function updatePageloadTransaction({ activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); // Patch span.end() to ensure we update the name one last time before the span is sent - patchSpanEnd(activeRootSpan, location, routes, basename, allRoutes, 'pageload'); + patchSpanEnd(activeRootSpan, location, routes, basename, 'pageload'); } + } else if (activeRootSpan) { + // Even if branches is null (can happen when lazy routes haven't loaded yet), + // we still need to patch span.end() so that when lazy routes load and the span ends, + // we can update the transaction name correctly. + patchSpanEnd(activeRootSpan, location, routes, basename, 'pageload'); } } @@ -1061,7 +1166,6 @@ function patchSpanEnd( location: Location, routes: RouteObject[], basename: string | undefined, - _allRoutes: RouteObject[] | undefined, spanType: 'pageload' | 'navigation', ): void { const patchedPropertyName = `__sentry_${spanType}_end_patched__` as const; @@ -1071,8 +1175,7 @@ function patchSpanEnd( return; } - // Use the passed route context, or fall back to global Set - const allRoutesSet = _allRoutes ? new Set(_allRoutes) : allRoutes; + // Uses global allRoutes to access lazy-loaded routes added after this function was called. const originalEnd = span.end.bind(span); let endCalled = false; @@ -1103,29 +1206,40 @@ function patchSpanEnd( }; const pendingPromises = pendingLazyRouteLoads.get(span); + const mayHaveLazyRoutes = (span as unknown as Record).__sentry_may_have_lazy_routes__; + // Wait for lazy routes if: - // 1. There are pending promises AND + // 1. (There are pending promises OR the span was marked as potentially having lazy routes) AND // 2. Current name exists AND // 3. Either the name has a wildcard OR the source is not 'route' (URL-based names) + const hasPendingOrMayHaveLazyRoutes = (pendingPromises && pendingPromises.size > 0) || mayHaveLazyRoutes; const shouldWaitForLazyRoutes = - pendingPromises && - pendingPromises.size > 0 && + hasPendingOrMayHaveLazyRoutes && currentName && (transactionNameHasWildcard(currentName) || currentSource !== 'route'); if (shouldWaitForLazyRoutes) { if (_lazyRouteTimeout === 0) { - tryUpdateSpanNameBeforeEnd(span, spanJson, currentName, location, routes, basename, spanType, allRoutesSet); + tryUpdateSpanNameBeforeEnd(span, spanJson, currentName, location, routes, basename, spanType, allRoutes); cleanupNavigationSpan(); originalEnd(endTimestamp); return; } - const allSettled = Promise.allSettled(pendingPromises).then(() => {}); - const waitPromise = - _lazyRouteTimeout === Infinity - ? allSettled - : Promise.race([allSettled, new Promise(r => setTimeout(r, _lazyRouteTimeout))]); + // If we have pending promises, wait for them. Otherwise, just wait for the timeout. + // This handles the case where we know lazy routes might load but patchRoutesOnNavigation + // hasn't been called yet. + const timeoutPromise = new Promise(r => setTimeout(r, _lazyRouteTimeout)); + let waitPromise: Promise; + + if (pendingPromises && pendingPromises.size > 0) { + const allSettled = Promise.allSettled(pendingPromises).then(() => {}); + waitPromise = _lazyRouteTimeout === Infinity ? allSettled : Promise.race([allSettled, timeoutPromise]); + } else { + // No pending promises yet, but we know lazy routes might load + // Wait for the timeout to give React Router time to call patchRoutesOnNavigation + waitPromise = timeoutPromise; + } waitPromise .then(() => { @@ -1138,7 +1252,7 @@ function patchSpanEnd( routes, basename, spanType, - allRoutesSet, + allRoutes, ); cleanupNavigationSpan(); originalEnd(endTimestamp); @@ -1150,7 +1264,7 @@ function patchSpanEnd( return; } - tryUpdateSpanNameBeforeEnd(span, spanJson, currentName, location, routes, basename, spanType, allRoutesSet); + tryUpdateSpanNameBeforeEnd(span, spanJson, currentName, location, routes, basename, spanType, allRoutes); cleanupNavigationSpan(); originalEnd(endTimestamp); }; diff --git a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx index 3d2b4f198cf5..7cc99641b5c2 100644 --- a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx +++ b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx @@ -1309,4 +1309,92 @@ describe('tryUpdateSpanNameBeforeEnd - source upgrade logic', () => { } }); }); + + describe('allRoutes global set (lazy routes behavior)', () => { + it('should allow adding routes to allRoutes after initial setup', () => { + // Clear the set first + allRoutes.clear(); + + const initialRoutes: RouteObject[] = [{ path: '/', element:
Home
}]; + const lazyRoutes: RouteObject[] = [{ path: '/lazy/:id', element:
Lazy
}]; + + // Add initial routes + addRoutesToAllRoutes(initialRoutes); + expect(allRoutes.size).toBe(1); + expect(allRoutes.has(initialRoutes[0]!)).toBe(true); + + // Simulate lazy route loading via patchRoutesOnNavigation + addRoutesToAllRoutes(lazyRoutes); + expect(allRoutes.size).toBe(2); + expect(allRoutes.has(lazyRoutes[0]!)).toBe(true); + }); + + it('should not duplicate routes when adding same route multiple times', () => { + allRoutes.clear(); + + const routes: RouteObject[] = [{ path: '/users', element:
Users
}]; + + addRoutesToAllRoutes(routes); + addRoutesToAllRoutes(routes); // Add same route again + + // Set should have unique entries only + expect(allRoutes.size).toBe(1); + }); + + it('should recursively add nested children routes', () => { + allRoutes.clear(); + + const parentRoute: RouteObject = { + path: '/parent', + element:
Parent
, + children: [ + { + path: ':id', + element:
Child
, + children: [{ path: 'nested', element:
Nested
}], + }, + ], + }; + + addRoutesToAllRoutes([parentRoute]); + + // Should add parent and all nested children + expect(allRoutes.size).toBe(3); + expect(allRoutes.has(parentRoute)).toBe(true); + expect(allRoutes.has(parentRoute.children![0]!)).toBe(true); + expect(allRoutes.has(parentRoute.children![0]!.children![0]!)).toBe(true); + }); + + // Regression test: Verify that routes added AFTER a span starts are still accessible + // This is the key fix for the lazy routes pageload bug where patchSpanEnd + // was using a stale snapshot instead of the global allRoutes set. + it('should maintain reference to global set (not snapshot) for late route additions', () => { + allRoutes.clear(); + + // Initial routes at "pageload start" time + const initialRoutes: RouteObject[] = [ + { path: '/', element:
Home
}, + { path: '/slow-fetch', element:
Slow Fetch Parent
}, + ]; + addRoutesToAllRoutes(initialRoutes); + + // Capture a reference to allRoutes (simulating what patchSpanEnd does AFTER the fix) + const routesReference = allRoutes; + + // Later, lazy routes are loaded via patchRoutesOnNavigation + const lazyLoadedRoutes: RouteObject[] = [{ path: ':id', element:
Lazy Child
}]; + addRoutesToAllRoutes(lazyLoadedRoutes); + + // The reference should see the newly added routes (fix behavior) + // Before the fix, a snapshot (new Set(allRoutes)) was taken, which wouldn't see new routes + expect(routesReference.size).toBe(3); + expect(routesReference.has(lazyLoadedRoutes[0]!)).toBe(true); + + // Convert to array and verify all routes are present + const allRoutesArray = Array.from(routesReference); + expect(allRoutesArray).toContain(initialRoutes[0]); + expect(allRoutesArray).toContain(initialRoutes[1]); + expect(allRoutesArray).toContain(lazyLoadedRoutes[0]); + }); + }); }); From 2a4b90704f8e784bf19f3a9e75c4974e3a9b4679 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 20 Jan 2026 12:36:40 +0000 Subject: [PATCH 2/5] Clean up deferred promise pattern and clear lazy routes flag after resolution --- .../src/reactrouter-compat-utils/instrumentation.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 121fe95abb73..3819232110bd 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -241,12 +241,10 @@ function trackLazyRouteLoad(span: Span, promise: Promise): void { * This ensures that patchedEnd waits for patchRoutesOnNavigation to be called before ending the span. */ function createDeferredLazyRoutePromise(span: Span): void { - let resolvePromise: () => void; const deferredPromise = new Promise(resolve => { - resolvePromise = resolve; + deferredLazyRouteResolvers.set(span, resolve); }); - deferredLazyRouteResolvers.set(span, resolvePromise!); trackLazyRouteLoad(span, deferredPromise); } @@ -259,6 +257,10 @@ function resolveDeferredLazyRoutePromise(span: Span): void { if (resolver) { resolver(); deferredLazyRouteResolvers.delete(span); + // Clear the flag so patchSpanEnd doesn't wait unnecessarily for routes that have already loaded + if ((span as unknown as Record).__sentry_may_have_lazy_routes__) { + (span as unknown as Record).__sentry_may_have_lazy_routes__ = false; + } } } From a9ed0f4f5d3a27b9b1330b36ac5329984f407bcc Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 20 Jan 2026 13:14:41 +0000 Subject: [PATCH 3/5] Improve route matching robustness and reduce code duplication in patchRoutesOnNavigation --- .../instrumentation.tsx | 75 ++++++++++--------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 3819232110bd..f2ae6461f4e7 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -773,6 +773,32 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio }; } +/** + * Helper to update the current span (navigation or pageload) with lazy-loaded route information. + * Reduces code duplication in patchRoutesOnNavigation wrapper. + */ +function updateSpanWithLazyRoutes(pathname: string, forceUpdate: boolean): void { + const currentActiveRootSpan = getActiveRootSpan(); + if (!currentActiveRootSpan) { + return; + } + + const spanOp = (spanToJSON(currentActiveRootSpan) as { op?: string }).op; + const location = { pathname, search: '', hash: '', state: null, key: 'default' }; + const routesArray = Array.from(allRoutes); + + if (spanOp === 'navigation') { + updateNavigationSpan(currentActiveRootSpan, location, routesArray, forceUpdate, _matchRoutes); + } else if (spanOp === 'pageload') { + updatePageloadTransaction({ + activeRootSpan: currentActiveRootSpan, + location, + routes: routesArray, + allRoutes: routesArray, + }); + } +} + function wrapPatchRoutesOnNavigation( opts: Record | undefined, isMemoryRouter = false, @@ -809,10 +835,16 @@ function wrapPatchRoutesOnNavigation( // to update the route objects in our allRoutes Set for proper route matching. if (matches && matches.length > 0) { const leafMatch = matches[matches.length - 1]; - if (leafMatch?.route) { - // Find the matching route in allRoutes by reference or path + const leafRoute = leafMatch?.route; + if (leafRoute) { + // Find the matching route in allRoutes by id, reference, or path for (const route of allRoutes) { - if (route === leafMatch.route || (route.path && route.path === leafMatch.route.path)) { + const idMatches = route.id !== undefined && route.id === routeId; + const referenceMatches = route === leafRoute; + const pathMatches = + route.path !== undefined && leafRoute.path !== undefined && route.path === leafRoute.path; + + if (idMatches || referenceMatches || pathMatches) { // Attach children to this parent route addResolvedRoutesToParent(children, route); break; @@ -821,21 +853,9 @@ function wrapPatchRoutesOnNavigation( } } - const currentActiveRootSpan = getActiveRootSpan(); // Only update if we have a valid targetPath (patchRoutesOnNavigation can be called without path) - if (targetPath && currentActiveRootSpan) { - const spanOp = (spanToJSON(currentActiveRootSpan) as { op?: string }).op; - const location = { pathname: targetPath, search: '', hash: '', state: null, key: 'default' }; - if (spanOp === 'navigation') { - updateNavigationSpan(currentActiveRootSpan, location, Array.from(allRoutes), true, _matchRoutes); - } else if (spanOp === 'pageload') { - updatePageloadTransaction({ - activeRootSpan: currentActiveRootSpan, - location, - routes: Array.from(allRoutes), - allRoutes: Array.from(allRoutes), - }); - } + if (targetPath) { + updateSpanWithLazyRoutes(targetPath, true); } return originalPatch(routeId, children); }; @@ -857,24 +877,9 @@ function wrapPatchRoutesOnNavigation( } } - const currentActiveRootSpan = getActiveRootSpan(); - if (currentActiveRootSpan) { - const spanOp = (spanToJSON(currentActiveRootSpan) as { op?: string }).op; - const pathname = isMemoryRouter ? targetPath : targetPath || WINDOW.location?.pathname; - - if (pathname) { - const location = { pathname, search: '', hash: '', state: null, key: 'default' }; - if (spanOp === 'navigation') { - updateNavigationSpan(currentActiveRootSpan, location, Array.from(allRoutes), false, _matchRoutes); - } else if (spanOp === 'pageload') { - updatePageloadTransaction({ - activeRootSpan: currentActiveRootSpan, - location, - routes: Array.from(allRoutes), - allRoutes: Array.from(allRoutes), - }); - } - } + const pathname = isMemoryRouter ? targetPath : targetPath || WINDOW.location?.pathname; + if (pathname) { + updateSpanWithLazyRoutes(pathname, false); } return result; From e55c099476b9c3bac0bec7702baa532a7aa14054 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 21 Jan 2026 19:32:16 +0000 Subject: [PATCH 4/5] Use .find() for route matching in patchRoutesOnNavigation --- .../src/reactrouter-compat-utils/instrumentation.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index f2ae6461f4e7..1cfa0951ddc4 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -838,17 +838,17 @@ function wrapPatchRoutesOnNavigation( const leafRoute = leafMatch?.route; if (leafRoute) { // Find the matching route in allRoutes by id, reference, or path - for (const route of allRoutes) { + const matchingRoute = Array.from(allRoutes).find(route => { const idMatches = route.id !== undefined && route.id === routeId; const referenceMatches = route === leafRoute; const pathMatches = route.path !== undefined && leafRoute.path !== undefined && route.path === leafRoute.path; - if (idMatches || referenceMatches || pathMatches) { - // Attach children to this parent route - addResolvedRoutesToParent(children, route); - break; - } + return idMatches || referenceMatches || pathMatches; + }); + + if (matchingRoute) { + addResolvedRoutesToParent(children, matchingRoute); } } } From 96f80cf87425989b286326fb6d2f0fe17ac38887 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 21 Jan 2026 19:32:49 +0000 Subject: [PATCH 5/5] Add regression test for navigation spans to slow lazy routes --- .../src/pages/Index.tsx | 4 +++ .../tests/transactions.test.ts | 34 +++++++++++++++++++ 2 files changed, 38 insertions(+) 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 index cf80af402b96..c22153441862 100644 --- 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 @@ -35,6 +35,10 @@ const Index = () => { Navigate to Slow Fetch Route (500ms delay with fetch) +
+ + Navigate to Wildcard Lazy Route (500ms delay, no fetch) + ); }; 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 9e19d84b7391..9ebfa7ceb8c3 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 @@ -1274,3 +1274,37 @@ test('Wildcard route pageload gets upgraded to parameterized route name (regress expect(event.contexts?.trace?.op).toBe('pageload'); expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route'); }); + +// Regression: Navigation to slow lazy route should get parameterized name even if span ends early. +// Network activity from dynamic imports extends the idle timeout until lazy routes load. +test('Slow lazy route navigation with early span end still gets parameterized route name (regression)', async ({ + page, +}) => { + // Configure short idle timeout (300ms) but longer lazy route timeout (1000ms) + await page.goto('/?idleTimeout=300&timeout=1000'); + + // Wait for pageload to complete + await page.waitForTimeout(500); + + const navigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + (transactionEvent.transaction?.startsWith('/wildcard-lazy') ?? false) + ); + }); + + // Navigate to wildcard-lazy route (500ms delay in module via top-level await) + // The dynamic import creates network activity that extends the span lifetime + const wildcardLazyLink = page.locator('id=navigation-to-wildcard-lazy'); + await expect(wildcardLazyLink).toBeVisible(); + await wildcardLazyLink.click(); + + const event = await navigationPromise; + + // The navigation transaction should have the parameterized route name + expect(event.transaction).toBe('/wildcard-lazy/:id'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('navigation'); + expect(event.contexts?.trace?.data?.['sentry.source']).toBe('route'); +});