fix(react): Prevent lazy route handlers from updating wrong navigation span by onurtemizkan · Pull Request #18898 · getsentry/sentry-javascript

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan · 2026-01-20T10:54:23Z

Following up on #18346

Fixes a remaining race condition where the args.patch callback and completion handler in wrapPatchRoutesOnNavigation were still calling getActiveRootSpan() at resolution time instead of using the captured span. This caused wrong span updates when users navigated away during lazy route loading.

The fix uses the captured activeRootSpan consistently, adds a !spanJson.timestamp check to skip updates on ended spans, and removes the WINDOW.location fallback. In captureCurrentLocation, returns null when navigation context exists but targetPath is undefined, instead of falling back to stale WINDOW.location.

Also added E2E tests that simulate rapid navigation scenarios where a slow lazy route handler completes after the user navigated elsewhere.

Copilot

Pull request overview

This PR fixes a race condition in React Router lazy route handling where navigation spans could be incorrectly updated when users navigate away during lazy route loading.

Changes:

Use captured navigation span consistently instead of calling getActiveRootSpan() at resolution time in wrapPatchRoutesOnNavigation
Add !spanJson.timestamp guard to prevent updates on already-ended spans
Remove WINDOW.location fallback when navigation context exists but targetPath is undefined to avoid using stale location data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/react/src/reactrouter-compat-utils/lazy-routes.tsx	Updated `captureCurrentLocation` to return `null` instead of falling back to `WINDOW.location` when inside navigation context with undefined `targetPath`
packages/react/src/reactrouter-compat-utils/instrumentation.tsx	Modified `wrapPatchRoutesOnNavigation` to use captured `activeRootSpan` consistently in both `args.patch` callback and completion handler, added `!spanJson.timestamp` checks, removed `WINDOW.location` fallback
packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts	Added edge case tests for `captureCurrentLocation` behavior with and without navigation context
packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx	Added unit tests for race condition fix verifying ended spans are not updated
dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts	Added E2E tests simulating rapid navigation scenarios to validate the race condition fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s1gr1d

Looks good, but I have to say the code gets harder and harder to understand 😅

…n span

…es (#19086) This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while. Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277 The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern. This problem has been discussed on React Router: - remix-run/react-router#11113 While this has been [addressed](remix-run/react-router#11626) for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (`document.hidden` guard), hitting timeouts like `idleTimeout`, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard. The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side. This PR proposes a new option: `lazyRouteManifest` specifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state. It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like: ```ts Sentry.reactRouterV7BrowserTracingIntegration({ // ... enableAsyncRouteHandlers: true lazyRouteManifest: [ '/', '/pricing', '/features', '/login', '/signup', '/forgot-password', '/reset-password/:token', '/org/:orgSlug', '/org/:orgSlug/dashboard', '/org/:orgSlug/projects', '/org/:orgSlug/projects/:projectId', '/org/:orgSlug/projects/:projectId/settings', '/org/:orgSlug/projects/:projectId/issues', '/org/:orgSlug/projects/:projectId/issues/:issueId', '/org/:orgSlug/team', '/org/:orgSlug/team/:memberId', '/org/:orgSlug/settings', '/org/:orgSlug/billing', '/admin', '/admin/users', '/admin/users/:userId', '/admin/orgs', '/admin/orgs/:orgId', ], }) ``` - This will only be active when `enableAsyncRouteHandlers` is set to `true` - To match URLs with given routes, we mimic React Router's own implementation. - When this is not provided or fails, it falls back to the current behaviour - This manifest is primarily for lazy routes, but the users can also add their non-lazy routes here for convenience or consistency. - Also added E2E tests that will fail when (if at some point) React Router manifests include the lazy routes before navigation, so we'll be aware and plan depending on that manifest instead. - We can do a cleanup for the race-condition / edge-case handling part of the code in a follow-up PR. Closes#19090 (added automatically)

onurtemizkan force-pushed the onur/fix-wrapPatchRoutesOnNavigation branch from 0c40c63 to 63dcf55Compare January 20, 2026 11:24

onurtemizkan marked this pull request as ready for review January 20, 2026 11:24

onurtemizkan requested a review from Copilot January 20, 2026 12:15

Copilotstarted reviewing on behalf of onurtemizkan January 20, 2026 12:15 View session

CopilotAI reviewed Jan 20, 2026
View reviewed changes

onurtemizkan requested review from chargome and s1gr1d January 20, 2026 14:28

onurtemizkan force-pushed the onur/fix-wrapPatchRoutesOnNavigation branch from 63dcf55 to 97659e9Compare January 21, 2026 19:39

s1gr1d approved these changes Jan 26, 2026
View reviewed changes

fix(react): Prevent lazy route handlers from updating wrong navigatio…
42a863b
…n span

onurtemizkan force-pushed the onur/fix-wrapPatchRoutesOnNavigation branch from 97659e9 to 42a863bCompare January 28, 2026 11:40

sentryBot reviewed Jan 28, 2026
View reviewed changes

Comment threadpackages/react/src/reactrouter-compat-utils/instrumentation.tsx

Lms24 approved these changes Jan 28, 2026
View reviewed changes

s1gr1d merged commit a2c6253 into developJan 28, 2026
108 checks passed

s1gr1d deleted the onur/fix-wrapPatchRoutesOnNavigation branch January 28, 2026 13:16

onurtemizkan mentioned this pull request Jan 29, 2026
feat(react): Add lazyRouteManifest option to resolve lazy-route names #19086
Merged

github-actionsBot mentioned this pull request Jan 29, 2026
feat(react): Add lazyRouteManifest option to resolve lazy-route names #19090
Closed

onurtemizkan mentioned this pull request Feb 23, 2026
ref(react): Remove lazy route machinery in favour of lazyRouteManifest#19466
Closed