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 2c960db9c16b..521048fd18f4 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 @@ -55,6 +55,12 @@ const router = sentryCreateBrowserRouter( lazyChildren: () => import('./pages/AnotherLazyRoutes').then(module => module.anotherNestedRoutes), }, }, + { + path: '/long-running', + handle: { + lazyChildren: () => import('./pages/LongRunningLazyRoutes').then(module => module.longRunningNestedRoutes), + }, + }, { path: '/static', element: <>Hello World, 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 aefa39d63811..3053aa57b887 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 @@ -15,6 +15,10 @@ const Index = () => { Navigate to Another Deep Lazy Route +
+ + Navigate to Long Running Lazy Route + ); }; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/LongRunningLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/LongRunningLazyRoutes.tsx new file mode 100644 index 000000000000..416fb1e162f8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/LongRunningLazyRoutes.tsx @@ -0,0 +1,49 @@ +import React, { useEffect, useState } from 'react'; +import { Link, useParams } from 'react-router-dom'; + +// Component that simulates a long-running component load +// This is used to test the POP guard during long-running pageloads +const SlowLoadingComponent = () => { + const { id } = useParams<{ id: string }>(); + const [data, setData] = useState(null); + const [isLoading, setIsLoading] = useState(true); + + useEffect(() => { + // Simulate a component that takes time to initialize + // This extends the pageload duration to create a window where POP events might occur + setTimeout(() => { + setData(`Data loaded for ID: ${id}`); + setIsLoading(false); + }, 1000); + }, [id]); + + if (isLoading) { + return
Loading...
; + } + + return ( +
+
{data}
+ + Go Home + +
+ ); +}; + +export const longRunningNestedRoutes = [ + { + path: 'slow', + children: [ + { + path: ':id', + element: , + loader: async () => { + // Simulate slow data fetching in the loader + await new Promise(resolve => setTimeout(resolve, 2000)); + return null; + }, + }, + ], + }, +]; 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 34e5105f8f9d..59d43c14ae95 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 @@ -294,3 +294,86 @@ test('Does not send any duplicate navigation transaction names browsing between '/lazy/inner/:id/:anotherId', ]); }); + +test('Does not create premature navigation transaction during long-running lazy route pageload', async ({ page }) => { + const navigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction.includes('long-running') + ); + }); + + const pageloadPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + transactionEvent.transaction === '/long-running/slow/:id' + ); + }); + + await page.goto('/long-running/slow/12345'); + + const pageloadEvent = await pageloadPromise; + + expect(pageloadEvent.transaction).toBe('/long-running/slow/:id'); + expect(pageloadEvent.contexts?.trace?.op).toBe('pageload'); + + const slowLoadingContent = page.locator('id=slow-loading-content'); + await expect(slowLoadingContent).toBeVisible({ timeout: 5000 }); + + const result = await Promise.race([ + navigationPromise.then(() => 'navigation'), + new Promise<'timeout'>(resolve => setTimeout(() => resolve('timeout'), 2000)), + ]); + + // Should timeout, meaning no unwanted navigation transaction was created + expect(result).toBe('timeout'); +}); + +test('Allows legitimate POP navigation (back/forward) after pageload completes', async ({ page }) => { + await page.goto('/'); + + const navigationToLongRunning = page.locator('id=navigation-to-long-running'); + await expect(navigationToLongRunning).toBeVisible(); + + const firstNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/long-running/slow/:id' + ); + }); + + await navigationToLongRunning.click(); + + const slowLoadingContent = page.locator('id=slow-loading-content'); + await expect(slowLoadingContent).toBeVisible({ timeout: 5000 }); + + const firstNavigationEvent = await firstNavigationPromise; + + expect(firstNavigationEvent.transaction).toBe('/long-running/slow/:id'); + expect(firstNavigationEvent.contexts?.trace?.op).toBe('navigation'); + + // Now navigate back using browser back button (POP event) + // This should create a navigation transaction since pageload is complete + const backNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/' + ); + }); + + await page.goBack(); + + // Verify we're back at home + const homeLink = page.locator('id=navigation'); + await expect(homeLink).toBeVisible(); + + const backNavigationEvent = await backNavigationPromise; + + // Validate that the back navigation (POP) was properly tracked + expect(backNavigationEvent.transaction).toBe('/'); + expect(backNavigationEvent.contexts?.trace?.op).toBe('navigation'); +}); diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 10db32231195..bf57fdbd74dc 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -241,6 +241,12 @@ export function createV6CompatibleWrapCreateBrowserRouter< const activeRootSpan = getActiveRootSpan(); + // Track whether we've completed the initial pageload to properly distinguish + // between POPs that occur during pageload vs. legitimate back/forward navigation. + let isInitialPageloadComplete = false; + let hasSeenPageloadSpan = !!activeRootSpan && spanToJSON(activeRootSpan).op === 'pageload'; + let hasSeenPopAfterPageload = false; + // The initial load ends when `createBrowserRouter` is called. // This is the earliest convenient time to update the transaction name. // Callbacks to `router.subscribe` are not called for the initial load. @@ -255,20 +261,31 @@ export function createV6CompatibleWrapCreateBrowserRouter< } router.subscribe((state: RouterState) => { - if (state.historyAction === 'PUSH' || state.historyAction === 'POP') { - // Wait for the next render if loading an unsettled route - if (state.navigation.state !== 'idle') { - requestAnimationFrame(() => { - handleNavigation({ - location: state.location, - routes, - navigationType: state.historyAction, - version, - basename, - allRoutes: Array.from(allRoutes), - }); - }); - } else { + // Track pageload completion to distinguish POPs during pageload from legitimate back/forward navigation + if (!isInitialPageloadComplete) { + const currentRootSpan = getActiveRootSpan(); + const isCurrentlyInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + + if (isCurrentlyInPageload) { + hasSeenPageloadSpan = true; + } else if (hasSeenPageloadSpan) { + // Pageload span was active but is now gone - pageload has completed + if (state.historyAction === 'POP' && !hasSeenPopAfterPageload) { + // Pageload ended: ignore the first POP after pageload + hasSeenPopAfterPageload = true; + } else { + // Pageload ended: either non-POP action or subsequent POP + isInitialPageloadComplete = true; + } + } + // If we haven't seen a pageload span yet, keep waiting (don't mark as complete) + } + + const shouldHandleNavigation = + state.historyAction === 'PUSH' || (state.historyAction === 'POP' && isInitialPageloadComplete); + + if (shouldHandleNavigation) { + const navigationHandler = (): void => { handleNavigation({ location: state.location, routes, @@ -277,6 +294,13 @@ export function createV6CompatibleWrapCreateBrowserRouter< basename, allRoutes: Array.from(allRoutes), }); + }; + + // Wait for the next render if loading an unsettled route + if (state.navigation.state !== 'idle') { + requestAnimationFrame(navigationHandler); + } else { + navigationHandler(); } } }); @@ -327,7 +351,6 @@ export function createV6CompatibleWrapCreateMemoryRouter< const router = createRouterFunction(routes, wrappedOpts); const basename = opts?.basename; - const activeRootSpan = getActiveRootSpan(); let initialEntry = undefined; const initialEntries = opts?.initialEntries; @@ -348,21 +371,68 @@ export function createV6CompatibleWrapCreateMemoryRouter< : initialEntry : router.state.location; - if (router.state.historyAction === 'POP' && activeRootSpan) { - updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) }); + const memoryActiveRootSpan = getActiveRootSpan(); + + if (router.state.historyAction === 'POP' && memoryActiveRootSpan) { + updatePageloadTransaction({ + activeRootSpan: memoryActiveRootSpan, + location, + routes, + basename, + allRoutes: Array.from(allRoutes), + }); } + // Track whether we've completed the initial pageload to properly distinguish + // between POPs that occur during pageload vs. legitimate back/forward navigation. + let isInitialPageloadComplete = false; + let hasSeenPageloadSpan = !!memoryActiveRootSpan && spanToJSON(memoryActiveRootSpan).op === 'pageload'; + let hasSeenPopAfterPageload = false; + router.subscribe((state: RouterState) => { + // Track pageload completion to distinguish POPs during pageload from legitimate back/forward navigation + if (!isInitialPageloadComplete) { + const currentRootSpan = getActiveRootSpan(); + const isCurrentlyInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + + if (isCurrentlyInPageload) { + hasSeenPageloadSpan = true; + } else if (hasSeenPageloadSpan) { + // Pageload span was active but is now gone - pageload has completed + if (state.historyAction === 'POP' && !hasSeenPopAfterPageload) { + // Pageload ended: ignore the first POP after pageload + hasSeenPopAfterPageload = true; + } else { + // Pageload ended: either non-POP action or subsequent POP + isInitialPageloadComplete = true; + } + } + // If we haven't seen a pageload span yet, keep waiting (don't mark as complete) + } + const location = state.location; - if (state.historyAction === 'PUSH' || state.historyAction === 'POP') { - handleNavigation({ - location, - routes, - navigationType: state.historyAction, - version, - basename, - allRoutes: Array.from(allRoutes), - }); + + const shouldHandleNavigation = + state.historyAction === 'PUSH' || (state.historyAction === 'POP' && isInitialPageloadComplete); + + if (shouldHandleNavigation) { + const navigationHandler = (): void => { + handleNavigation({ + location, + routes, + navigationType: state.historyAction, + version, + basename, + allRoutes: Array.from(allRoutes), + }); + }; + + // Wait for the next render if loading an unsettled route + if (state.navigation.state !== 'idle') { + requestAnimationFrame(navigationHandler); + } else { + navigationHandler(); + } } }); @@ -532,8 +602,16 @@ function wrapPatchRoutesOnNavigation( // Update navigation span after routes are patched const activeRootSpan = getActiveRootSpan(); if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') { - // For memory routers, we should not access window.location; use targetPath only - const pathname = isMemoryRouter ? targetPath : targetPath || WINDOW.location?.pathname; + // Determine pathname based on router type + let pathname: string | undefined; + if (isMemoryRouter) { + // For memory routers, only use targetPath + pathname = targetPath; + } else { + // For browser routers, use targetPath or fall back to window.location + pathname = targetPath || WINDOW.location?.pathname; + } + if (pathname) { updateNavigationSpan( activeRootSpan,