From 249062eee7418cf8284465862ad70169f1ce94d6 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 6 Oct 2025 12:09:40 +0100 Subject: [PATCH 1/8] fix(react): Add `POP` guard for long-running `pageload` spans --- .../instrumentation.tsx | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 10db32231195..f2a122d9c3bd 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -241,6 +241,10 @@ export function createV6CompatibleWrapCreateBrowserRouter< const activeRootSpan = getActiveRootSpan(); + // Track whether we've handled the initial POP to avoid creating navigation spans + // for POP events that occur during the initial pageload phase. + let hasHandledInitialPop = 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. @@ -252,23 +256,19 @@ export function createV6CompatibleWrapCreateBrowserRouter< basename, allRoutes: Array.from(allRoutes), }); + hasHandledInitialPop = true; } 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 { + const shouldHandleNavigation = + state.historyAction === 'PUSH' || + // Only handle POP events after the initial pageload POP has been processed. + // This prevents creating navigation spans for POP events during long-running pageloads, + // while still allowing legitimate back/forward button navigation to be tracked. + (state.historyAction === 'POP' && hasHandledInitialPop); + + if (shouldHandleNavigation) { + const navigationHandler = (): void => { handleNavigation({ location: state.location, routes, @@ -277,6 +277,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(); } } }); From af570953ba6c034ce2309bdeb06c1c6b2c2c7c16 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 6 Oct 2025 12:45:33 +0100 Subject: [PATCH 2/8] Improve --- .../instrumentation.tsx | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index f2a122d9c3bd..4624d30ed6dd 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -241,9 +241,9 @@ export function createV6CompatibleWrapCreateBrowserRouter< const activeRootSpan = getActiveRootSpan(); - // Track whether we've handled the initial POP to avoid creating navigation spans - // for POP events that occur during the initial pageload phase. - let hasHandledInitialPop = false; + // Track whether we've completed the initial pageload to properly distinguish + // between POPs that occur during pageload vs. legitimate back/forward navigation. + let isPageloadComplete = false; // The initial load ends when `createBrowserRouter` is called. // This is the earliest convenient time to update the transaction name. @@ -256,16 +256,23 @@ export function createV6CompatibleWrapCreateBrowserRouter< basename, allRoutes: Array.from(allRoutes), }); - hasHandledInitialPop = true; } router.subscribe((state: RouterState) => { + const currentRootSpan = getActiveRootSpan(); + const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + + // Mark pageload as complete once we're no longer in a pageload span + if (!isStillInPageload && !isPageloadComplete) { + isPageloadComplete = true; + } + const shouldHandleNavigation = state.historyAction === 'PUSH' || - // Only handle POP events after the initial pageload POP has been processed. + // Only handle POP events after pageload is complete. // This prevents creating navigation spans for POP events during long-running pageloads, // while still allowing legitimate back/forward button navigation to be tracked. - (state.historyAction === 'POP' && hasHandledInitialPop); + (state.historyAction === 'POP' && isPageloadComplete); if (shouldHandleNavigation) { const navigationHandler = (): void => { @@ -359,9 +366,28 @@ export function createV6CompatibleWrapCreateMemoryRouter< updatePageloadTransaction({ activeRootSpan, 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 isPageloadComplete = false; + router.subscribe((state: RouterState) => { + const currentRootSpan = getActiveRootSpan(); + const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + + // Mark pageload as complete once we're no longer in a pageload span + if (!isStillInPageload && !isPageloadComplete) { + isPageloadComplete = true; + } + const location = state.location; - if (state.historyAction === 'PUSH' || state.historyAction === 'POP') { + const shouldHandleNavigation = + state.historyAction === 'PUSH' || + // Only handle POP events after pageload is complete. + // This prevents creating navigation spans for POP events during long-running pageloads, + // while still allowing legitimate back/forward button navigation to be tracked. + (state.historyAction === 'POP' && isPageloadComplete); + + if (shouldHandleNavigation) { handleNavigation({ location, routes, From 52740b97e606b892ada344683dea6d5f929bcceb Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 6 Oct 2025 13:21:44 +0100 Subject: [PATCH 3/8] Make memory router consistent --- .../instrumentation.tsx | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 4624d30ed6dd..756a3a05b4d1 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -388,14 +388,23 @@ export function createV6CompatibleWrapCreateMemoryRouter< (state.historyAction === 'POP' && isPageloadComplete); if (shouldHandleNavigation) { - handleNavigation({ - location, - routes, - navigationType: state.historyAction, - version, - basename, - allRoutes: Array.from(allRoutes), - }); + 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(); + } } }); From 9ac974d5481f46d9c05bb39dbf3ed3b5d1d4970a Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 8 Oct 2025 20:48:39 +0100 Subject: [PATCH 4/8] Improve the loading state checks --- .../instrumentation.tsx | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 756a3a05b4d1..1619aa5fa083 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -243,7 +243,7 @@ export function createV6CompatibleWrapCreateBrowserRouter< // Track whether we've completed the initial pageload to properly distinguish // between POPs that occur during pageload vs. legitimate back/forward navigation. - let isPageloadComplete = false; + let isInitialPageloadComplete = false; // The initial load ends when `createBrowserRouter` is called. // This is the earliest convenient time to update the transaction name. @@ -259,20 +259,20 @@ export function createV6CompatibleWrapCreateBrowserRouter< } router.subscribe((state: RouterState) => { - const currentRootSpan = getActiveRootSpan(); - const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; - - // Mark pageload as complete once we're no longer in a pageload span - if (!isStillInPageload && !isPageloadComplete) { - isPageloadComplete = true; + // Mark pageload as complete on first PUSH navigation or when pageload span ends + if (state.historyAction === 'PUSH') { + isInitialPageloadComplete = true; + } else if (!isInitialPageloadComplete && state.navigation.state === 'idle') { + const currentRootSpan = getActiveRootSpan(); + const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + if (!isStillInPageload) { + isInitialPageloadComplete = true; + return; // Don't handle the POP that completes the initial pageload + } } const shouldHandleNavigation = - state.historyAction === 'PUSH' || - // Only handle POP events after pageload is complete. - // This prevents creating navigation spans for POP events during long-running pageloads, - // while still allowing legitimate back/forward button navigation to be tracked. - (state.historyAction === 'POP' && isPageloadComplete); + state.historyAction === 'PUSH' || (state.historyAction === 'POP' && isInitialPageloadComplete); if (shouldHandleNavigation) { const navigationHandler = (): void => { @@ -368,24 +368,24 @@ export function createV6CompatibleWrapCreateMemoryRouter< // Track whether we've completed the initial pageload to properly distinguish // between POPs that occur during pageload vs. legitimate back/forward navigation. - let isPageloadComplete = false; + let isInitialPageloadComplete = false; router.subscribe((state: RouterState) => { - const currentRootSpan = getActiveRootSpan(); - const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; - - // Mark pageload as complete once we're no longer in a pageload span - if (!isStillInPageload && !isPageloadComplete) { - isPageloadComplete = true; + // Mark pageload as complete on first PUSH navigation or when pageload span ends + if (state.historyAction === 'PUSH') { + isInitialPageloadComplete = true; + } else if (!isInitialPageloadComplete && state.navigation.state === 'idle') { + const currentRootSpan = getActiveRootSpan(); + const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + if (!isStillInPageload) { + isInitialPageloadComplete = true; + return; // Don't handle the POP that completes the initial pageload + } } const location = state.location; const shouldHandleNavigation = - state.historyAction === 'PUSH' || - // Only handle POP events after pageload is complete. - // This prevents creating navigation spans for POP events during long-running pageloads, - // while still allowing legitimate back/forward button navigation to be tracked. - (state.historyAction === 'POP' && isPageloadComplete); + state.historyAction === 'PUSH' || (state.historyAction === 'POP' && isInitialPageloadComplete); if (shouldHandleNavigation) { const navigationHandler = (): void => { From 1ba305f77cdf45b36f6243e0719646dd14470c79 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 8 Oct 2025 20:52:43 +0100 Subject: [PATCH 5/8] Add E2E tests --- .../react-router-7-lazy-routes/src/index.tsx | 6 ++ .../src/pages/Index.tsx | 4 + .../src/pages/LongRunningLazyRoutes.tsx | 49 +++++++++++ .../tests/transactions.test.ts | 83 +++++++++++++++++++ 4 files changed, 142 insertions(+) create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/LongRunningLazyRoutes.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 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'); +}); From 85764624d7379b3359f05d0557bb2d76066a5f30 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 8 Oct 2025 21:10:39 +0100 Subject: [PATCH 6/8] Stop relying on `idle` state --- .../instrumentation.tsx | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index 1619aa5fa083..f849f865cb00 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -259,15 +259,17 @@ export function createV6CompatibleWrapCreateBrowserRouter< } router.subscribe((state: RouterState) => { - // Mark pageload as complete on first PUSH navigation or when pageload span ends - if (state.historyAction === 'PUSH') { - isInitialPageloadComplete = true; - } else if (!isInitialPageloadComplete && state.navigation.state === 'idle') { + // Check if pageload span has ended to mark completion + if (!isInitialPageloadComplete) { const currentRootSpan = getActiveRootSpan(); const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + if (!isStillInPageload) { isInitialPageloadComplete = true; - return; // Don't handle the POP that completes the initial pageload + // Don't handle this specific callback if it's a POP that marks completion + if (state.historyAction === 'POP') { + return; + } } } @@ -371,15 +373,17 @@ export function createV6CompatibleWrapCreateMemoryRouter< let isInitialPageloadComplete = false; router.subscribe((state: RouterState) => { - // Mark pageload as complete on first PUSH navigation or when pageload span ends - if (state.historyAction === 'PUSH') { - isInitialPageloadComplete = true; - } else if (!isInitialPageloadComplete && state.navigation.state === 'idle') { + // Check if pageload span has ended to mark completion + if (!isInitialPageloadComplete) { const currentRootSpan = getActiveRootSpan(); const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + if (!isStillInPageload) { isInitialPageloadComplete = true; - return; // Don't handle the POP that completes the initial pageload + // Don't handle this specific callback if it's a POP that marks completion + if (state.historyAction === 'POP') { + return; + } } } From 851b1b0da43a80c43da32d6d03915143c78caab5 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 9 Oct 2025 11:48:40 +0100 Subject: [PATCH 7/8] Make it more defensive --- .../instrumentation.tsx | 68 +++++++++++++------ 1 file changed, 49 insertions(+), 19 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index f849f865cb00..a17ff2da9029 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -244,6 +244,8 @@ export function createV6CompatibleWrapCreateBrowserRouter< // 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. @@ -259,17 +261,22 @@ export function createV6CompatibleWrapCreateBrowserRouter< } router.subscribe((state: RouterState) => { - // Check if pageload span has ended to mark completion + // Track pageload completion to distinguish POPs during pageload from legitimate back/forward navigation if (!isInitialPageloadComplete) { const currentRootSpan = getActiveRootSpan(); - const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + const isCurrentlyInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; - if (!isStillInPageload) { + if (isCurrentlyInPageload) { + hasSeenPageloadSpan = true; + } else if (!hasSeenPageloadSpan) { + // No pageload span detected, so we're not in initial pageload mode + isInitialPageloadComplete = true; + } else 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; - // Don't handle this specific callback if it's a POP that marks completion - if (state.historyAction === 'POP') { - return; - } } } @@ -343,7 +350,6 @@ export function createV6CompatibleWrapCreateMemoryRouter< const router = createRouterFunction(routes, wrappedOpts); const basename = opts?.basename; - const activeRootSpan = getActiveRootSpan(); let initialEntry = undefined; const initialEntries = opts?.initialEntries; @@ -364,30 +370,46 @@ 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) => { - // Check if pageload span has ended to mark completion + // Track pageload completion to distinguish POPs during pageload from legitimate back/forward navigation if (!isInitialPageloadComplete) { const currentRootSpan = getActiveRootSpan(); - const isStillInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; + const isCurrentlyInPageload = currentRootSpan && spanToJSON(currentRootSpan).op === 'pageload'; - if (!isStillInPageload) { + if (isCurrentlyInPageload) { + hasSeenPageloadSpan = true; + } else if (!hasSeenPageloadSpan) { + // No pageload span detected, so we're not in initial pageload mode + isInitialPageloadComplete = true; + } else 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; - // Don't handle this specific callback if it's a POP that marks completion - if (state.historyAction === 'POP') { - return; - } } } const location = state.location; + const shouldHandleNavigation = state.historyAction === 'PUSH' || (state.historyAction === 'POP' && isInitialPageloadComplete); @@ -578,8 +600,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, From 1fb8a6d9d2607d38737a6c8dad8192217449c815 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 9 Oct 2025 14:55:41 +0100 Subject: [PATCH 8/8] Address potential race condition --- .../instrumentation.tsx | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index a17ff2da9029..bf57fdbd74dc 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -268,16 +268,17 @@ export function createV6CompatibleWrapCreateBrowserRouter< if (isCurrentlyInPageload) { hasSeenPageloadSpan = true; - } else if (!hasSeenPageloadSpan) { - // No pageload span detected, so we're not in initial pageload mode - isInitialPageloadComplete = true; - } else 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; + } 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 = @@ -396,16 +397,17 @@ export function createV6CompatibleWrapCreateMemoryRouter< if (isCurrentlyInPageload) { hasSeenPageloadSpan = true; - } else if (!hasSeenPageloadSpan) { - // No pageload span detected, so we're not in initial pageload mode - isInitialPageloadComplete = true; - } else 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; + } 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;