fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
s1gr1d merged 5 commits into from
Sep 9, 2025
Merged

fix(react): Remove handleExistingNavigation#17534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
385d041
fix(react): Remove `handleExistingNavigation`
onurtemizkan Sep 5, 2025
415d43f
Remove `isLikelyLazyRoute`
onurtemizkan Sep 5, 2025
0a34f8b
Add more e2e testing
onurtemizkan Sep 5, 2025
71a935f
Remove `createNewNavigationSpan`, set update guard selectively
onurtemizkan Sep 5, 2025
b12ad48
Merge branch 'develop' into onur/remove-handleexistingnavigation
s1gr1d Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
<Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
Navigate to Another Lazy Route
</Link>
<Link to="/lazy/inner/1/2/" id="navigate-to-upper">
Navigate to Upper Lazy Route
</Link>
</div>
),
},
Expand Down
121 changes: 112 additions & 9 deletions ...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
page,
}) => {
await page.goto('/');

// Navigate to inner lazy route first
const navigationToInner = page.locator('id=navigation');
await expect(navigationToInner).toBeVisible();

// First, navigate to the inner lazy route
const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
);
});

await page.goto('/');

// Navigate to inner lazy route first
const navigationToInner = page.locator('id=navigation');
await expect(navigationToInner).toBeVisible();
await navigationToInner.click();

const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
expect(firstEvent.type).toBe('transaction');
expect(firstEvent.contexts?.trace?.op).toBe('navigation');

// Click the navigation link from within the inner lazy route to another lazy route
const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
await expect(navigationToAnotherFromInner).toBeVisible();

// Now navigate from the inner lazy route to another lazy route
const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
);
});

// Click the navigation link from within the inner lazy route to another lazy route
const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
await expect(navigationToAnotherFromInner).toBeVisible();
await navigationToAnotherFromInner.click();

const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
expect(secondEvent.type).toBe('transaction');
expect(secondEvent.contexts?.trace?.op).toBe('navigation');

// Go back to the previous page to ensure history navigation works as expected
const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
return (
!!transactionEvent?.transaction &&
transactionEvent.contexts?.trace?.op === 'navigation' &&
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
);
});

await page.goBack();

const goBackEvent = await goBackTransactionPromise;

// Validate the second go back transaction event
expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
expect(goBackEvent.type).toBe('transaction');
expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

// Navigate to the upper route
const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
return (
!!transactionEvent?.transaction &&
transactionEvent.contexts?.trace?.op === 'navigation' &&
transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
);
});

const navigationToUpper = page.locator('id=navigate-to-upper');

await navigationToUpper.click();

const goUpperRouteEvent = await goUpperRouteTransactionPromise;

// Validate the go upper route transaction event
expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
expect(goUpperRouteEvent.type).toBe('transaction');
expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
const transactionNamesList: string[] = [];

// Monitor and add all transaction names sent to Sentry for the navigations
const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
if (transactionEvent?.transaction) {
transactionNamesList.push(transactionEvent.transaction);
}

if (transactionNamesList.length >= 5) {
// Stop monitoring once we have enough transaction names
return true;
}

return false;
});

// Go to root page
await page.goto('/');
page.waitForTimeout(1000);

// Navigate to inner lazy route
const navigationToInner = page.locator('id=navigation');
await expect(navigationToInner).toBeVisible();
await navigationToInner.click();

// Navigate to another lazy route
const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
await expect(navigationToAnother).toBeVisible();
await page.waitForTimeout(1000);

// Click to navigate to another lazy route
await navigationToAnother.click();
const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
await expect(anotherLazyRouteContent).toBeVisible();
await page.waitForTimeout(1000);

// Navigate back to inner lazy route
await page.goBack();
await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
await page.waitForTimeout(1000);

// Navigate to upper inner lazy route
const navigationToUpper = page.locator('id=navigate-to-upper');
await expect(navigationToUpper).toBeVisible();
await navigationToUpper.click();

await page.waitForTimeout(1000);

await allTransactionsPromise;

expect(transactionNamesList.length).toBe(5);
expect(transactionNamesList).toEqual([
'/',
'/lazy/inner/:id/:anotherId/:someAnotherId',
'/another-lazy/sub/:id/:subId',
'/lazy/inner/:id/:anotherId/:someAnotherId',
'/lazy/inner/:id/:anotherId',
]);
});
3 changes: 0 additions & 3 deletions packages/react/src/reactrouter-compat-utils/index.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
createV6CompatibleWrapCreateMemoryRouter,
createV6CompatibleWrapUseRoutes,
handleNavigation,
handleExistingNavigationSpan,
createNewNavigationSpan,
addResolvedRoutesToParent,
processResolvedRoutes,
updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
resolveRouteNameAndSource,
getNormalizedName,
initializeRouterUtils,
isLikelyLazyRouteContext,
locationIsInsideDescendantRoute,
prefixWithSlash,
rebuildRoutePathFromAllRoutes,
Expand Down
128 changes: 15 additions & 113 deletions packages/react/src/reactrouter-compat-utils/instrumentation.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
getNormalizedName,
initializeRouterUtils,
isLikelyLazyRouteContext,
locationIsInsideDescendantRoute,
prefixWithSlash,
rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
// Check if this span has already been named to avoid multiple updates
// But allow updates if this is a forced update (e.g., when lazy routes are loaded)
const hasBeenNamed =
!forceUpdate &&
(
activeRootSpan as {
__sentry_navigation_name_set__?: boolean;
}
)?.__sentry_navigation_name_set__;
!forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

if (!hasBeenNamed) {
// Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
: router.state.location;

if (router.state.historyAction === 'POP' && activeRootSpan) {
updatePageloadTransaction({
activeRootSpan,
location,
routes,
basename,
allRoutes: Array.from(allRoutes),
});
updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
}

router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
version: V6CompatibleVersion,
): Integration {
const integration = browserTracingIntegration({
...options,
instrumentPageLoad: false,
instrumentNavigation: false,
});
const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

const {
useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
updateNavigationSpan(
activeRootSpan,
{
pathname: targetPath,
search: '',
hash: '',
state: null,
key: 'default',
},
{ pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
Array.from(allRoutes),
true, // forceUpdate = true since we're loading lazy routes
_matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
if (pathname) {
updateNavigationSpan(
activeRootSpan,
{
pathname,
search: '',
hash: '',
state: null,
key: 'default',
},
{ pathname, search: '', hash: '', state: null, key: 'default' },
Array.from(allRoutes),
false, // forceUpdate = false since this is after lazy routes are loaded
_matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
basename,
);

// Check if this might be a lazy route context
const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

const activeSpan = getActiveSpan();
const spanJson = activeSpan && spanToJSON(activeSpan);
const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

// Cross usage can result in multiple navigation spans being created without this check
if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
} else {
createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
if (!isAlreadyInNavigationSpan) {

@chargomechargomeSep 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?

@onurtemizkanonurtemizkanSep 5, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).

Turns out it's unnecessary (and also regressed the bug we were trying to fix).

Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.

startBrowserTracingNavigationSpan(client, {
name,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
},
});

@cursorcursorBotSep 5, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans

The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.

Fix in CursorFix in Web

}
}
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
});
isMountRenderPass.current = false;
} else {
handleNavigation({
location,
routes,
navigationType,
version,
allRoutes: Array.from(allRoutes),
});
handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
}
},
// `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
// Only use this root span if it is a pageload or navigation span
return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
* Handles updating an existing navigation span
*/
export function handleExistingNavigationSpan(
activeSpan: Span,
spanJson: ReturnType<typeof spanToJSON>,
name: string,
source: TransactionSource,
isLikelyLazyRoute: boolean,
): void {
// Check if we've already set the name for this span using a custom property
const hasBeenNamed = (
activeSpan as {
__sentry_navigation_name_set__?: boolean;
}
)?.__sentry_navigation_name_set__;

if (!hasBeenNamed) {
// This is the first time we're setting the name for this span
if (!spanJson.timestamp) {
activeSpan?.updateName(name);
}

// For lazy routes, don't mark as named yet so it can be updated later
if (!isLikelyLazyRoute) {
addNonEnumerableProperty(
activeSpan as { __sentry_navigation_name_set__?: boolean },
'__sentry_navigation_name_set__',
true,
);
}
}

// Always set the source attribute to keep it consistent with the current route
activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
* Creates a new navigation span
*/
export function createNewNavigationSpan(
client: Client,
name: string,
source: TransactionSource,
version: string,
isLikelyLazyRoute: boolean,
): void {
const newSpan = startBrowserTracingNavigationSpan(client, {
name,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
},
});

// For lazy routes, don't mark as named yet so it can be updated later when the route loads
if (!isLikelyLazyRoute && newSpan) {
addNonEnumerableProperty(
newSpan as { __sentry_navigation_name_set__?: boolean },
'__sentry_navigation_name_set__',
true,
);
}
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

s1gr1d
merged 5 commits into

from

Sep 9, 2025
 Merged


fix(react): Remove handleExistingNavigation#17534
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
385d041

fix(react): Remove `handleExistingNavigation`


onurtemizkan Sep 5, 2025
415d43f

Remove `isLikelyLazyRoute`


onurtemizkan Sep 5, 2025
0a34f8b

Add more e2e testing


onurtemizkan Sep 5, 2025
71a935f

Remove `createNewNavigationSpan`, set update guard selectively


onurtemizkan Sep 5, 2025
b12ad48

Merge branch 'develop' into onur/remove-handleexistingnavigation


s1gr1d Sep 9, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
 <Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
 Navigate to Another Lazy Route
 </Link>
 <Link to="/lazy/inner/1/2/" id="navigate-to-upper">
 Navigate to Upper Lazy Route
 </Link>
 </div>
 ),
 },
Expand Down

121 changes: 112 additions & 9 deletions
...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
 page,
}) => {
 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();

 // First, navigate to the inner lazy route
 const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(firstEvent.type).toBe('transaction');
 expect(firstEvent.contexts?.trace?.op).toBe('navigation');

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();

 // Now navigate from the inner lazy route to another lazy route
 const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();
 await navigationToAnotherFromInner.click();

 const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
 expect(secondEvent.type).toBe('transaction');
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');

 // Go back to the previous page to ensure history navigation works as expected
 const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
 );
 });

 await page.goBack();

 const goBackEvent = await goBackTransactionPromise;

 // Validate the second go back transaction event
 expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
 expect(goBackEvent.type).toBe('transaction');
 expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

 // Navigate to the upper route
 const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
 );
 });

 const navigationToUpper = page.locator('id=navigate-to-upper');

 await navigationToUpper.click();

 const goUpperRouteEvent = await goUpperRouteTransactionPromise;

 // Validate the go upper route transaction event
 expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
 expect(goUpperRouteEvent.type).toBe('transaction');
 expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
 const transactionNamesList: string[] = [];

 // Monitor and add all transaction names sent to Sentry for the navigations
 const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 if (transactionEvent?.transaction) {
 transactionNamesList.push(transactionEvent.transaction);
 }

 if (transactionNamesList.length >= 5) {
 // Stop monitoring once we have enough transaction names
 return true;
 }

 return false;
 });

 // Go to root page
 await page.goto('/');
 page.waitForTimeout(1000);

 // Navigate to inner lazy route
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 // Navigate to another lazy route
 const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnother).toBeVisible();
 await page.waitForTimeout(1000);

 // Click to navigate to another lazy route
 await navigationToAnother.click();
 const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
 await expect(anotherLazyRouteContent).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate back to inner lazy route
 await page.goBack();
 await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate to upper inner lazy route
 const navigationToUpper = page.locator('id=navigate-to-upper');
 await expect(navigationToUpper).toBeVisible();
 await navigationToUpper.click();

 await page.waitForTimeout(1000);

 await allTransactionsPromise;

 expect(transactionNamesList.length).toBe(5);
 expect(transactionNamesList).toEqual([
 '/',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/another-lazy/sub/:id/:subId',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/lazy/inner/:id/:anotherId',
 ]);
});

3 changes: 0 additions & 3 deletions
packages/react/src/reactrouter-compat-utils/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
 createV6CompatibleWrapCreateMemoryRouter,
 createV6CompatibleWrapUseRoutes,
 handleNavigation,
 handleExistingNavigationSpan,
 createNewNavigationSpan,
 addResolvedRoutesToParent,
 processResolvedRoutes,
 updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
 resolveRouteNameAndSource,
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand Down

128 changes: 15 additions & 113 deletions
packages/react/src/reactrouter-compat-utils/instrumentation.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
 // Check if this span has already been named to avoid multiple updates
 // But allow updates if this is a forced update (e.g., when lazy routes are loaded)
 const hasBeenNamed =
 !forceUpdate &&
 (
 activeRootSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;
 !forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
 : router.state.location;

 if (router.state.historyAction === 'POP' && activeRootSpan) {
 updatePageloadTransaction({
 activeRootSpan,
 location,
 routes,
 basename,
 allRoutes: Array.from(allRoutes),
 });
 updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
 }

 router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
 options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
 version: V6CompatibleVersion,
): Integration {
 const integration = browserTracingIntegration({
 ...options,
 instrumentPageLoad: false,
 instrumentNavigation: false,
 });
 const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

 const {
 useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
 if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname: targetPath,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 true, // forceUpdate = true since we're loading lazy routes
 _matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
 if (pathname) {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 false, // forceUpdate = false since this is after lazy routes are loaded
 _matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
 basename,
 );

 // Check if this might be a lazy route context
 const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

 const activeSpan = getActiveSpan();
 const spanJson = activeSpan && spanToJSON(activeSpan);
 const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

 // Cross usage can result in multiple navigation spans being created without this check
 if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
 handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
 } else {
 createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
 if (!isAlreadyInNavigationSpan) {
@chargomechargomeSep 5, 2025

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?
@onurtemizkanonurtemizkanSep 5, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).
Turns out it's unnecessary (and also regressed the bug we were trying to fix).
Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.
 startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });
@cursorcursorBotSep 5, 2025

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans
The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.
Fix in CursorFix in Web
 }
 }
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
 });
 isMountRenderPass.current = false;
 } else {
 handleNavigation({
 location,
 routes,
 navigationType,
 version,
 allRoutes: Array.from(allRoutes),
 });
 handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
 }
 },
 // `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
 // Only use this root span if it is a pageload or navigation span
 return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
 * Handles updating an existing navigation span
 */
export function handleExistingNavigationSpan(
 activeSpan: Span,
 spanJson: ReturnType<typeof spanToJSON>,
 name: string,
 source: TransactionSource,
 isLikelyLazyRoute: boolean,
): void {
 // Check if we've already set the name for this span using a custom property
 const hasBeenNamed = (
 activeSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // This is the first time we're setting the name for this span
 if (!spanJson.timestamp) {
 activeSpan?.updateName(name);
 }

 // For lazy routes, don't mark as named yet so it can be updated later
 if (!isLikelyLazyRoute) {
 addNonEnumerableProperty(
 activeSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
 }

 // Always set the source attribute to keep it consistent with the current route
 activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
 * Creates a new navigation span
 */
export function createNewNavigationSpan(
 client: Client,
 name: string,
 source: TransactionSource,
 version: string,
 isLikelyLazyRoute: boolean,
): void {
 const newSpan = startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });

 // For lazy routes, don't mark as named yet so it can be updated later when the route loads
 if (!isLikelyLazyRoute && newSpan) {
 addNonEnumerableProperty(
 newSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
}
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

s1gr1d
merged 5 commits into

from

Sep 9, 2025
 Merged


fix(react): Remove handleExistingNavigation#17534
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
385d041

fix(react): Remove `handleExistingNavigation`


onurtemizkan Sep 5, 2025
415d43f

Remove `isLikelyLazyRoute`


onurtemizkan Sep 5, 2025
0a34f8b

Add more e2e testing


onurtemizkan Sep 5, 2025
71a935f

Remove `createNewNavigationSpan`, set update guard selectively


onurtemizkan Sep 5, 2025
b12ad48

Merge branch 'develop' into onur/remove-handleexistingnavigation


s1gr1d Sep 9, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
 <Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
 Navigate to Another Lazy Route
 </Link>
 <Link to="/lazy/inner/1/2/" id="navigate-to-upper">
 Navigate to Upper Lazy Route
 </Link>
 </div>
 ),
 },
Expand Down

121 changes: 112 additions & 9 deletions
...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
 page,
}) => {
 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();

 // First, navigate to the inner lazy route
 const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(firstEvent.type).toBe('transaction');
 expect(firstEvent.contexts?.trace?.op).toBe('navigation');

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();

 // Now navigate from the inner lazy route to another lazy route
 const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();
 await navigationToAnotherFromInner.click();

 const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
 expect(secondEvent.type).toBe('transaction');
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');

 // Go back to the previous page to ensure history navigation works as expected
 const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
 );
 });

 await page.goBack();

 const goBackEvent = await goBackTransactionPromise;

 // Validate the second go back transaction event
 expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
 expect(goBackEvent.type).toBe('transaction');
 expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

 // Navigate to the upper route
 const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
 );
 });

 const navigationToUpper = page.locator('id=navigate-to-upper');

 await navigationToUpper.click();

 const goUpperRouteEvent = await goUpperRouteTransactionPromise;

 // Validate the go upper route transaction event
 expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
 expect(goUpperRouteEvent.type).toBe('transaction');
 expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
 const transactionNamesList: string[] = [];

 // Monitor and add all transaction names sent to Sentry for the navigations
 const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 if (transactionEvent?.transaction) {
 transactionNamesList.push(transactionEvent.transaction);
 }

 if (transactionNamesList.length >= 5) {
 // Stop monitoring once we have enough transaction names
 return true;
 }

 return false;
 });

 // Go to root page
 await page.goto('/');
 page.waitForTimeout(1000);

 // Navigate to inner lazy route
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 // Navigate to another lazy route
 const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnother).toBeVisible();
 await page.waitForTimeout(1000);

 // Click to navigate to another lazy route
 await navigationToAnother.click();
 const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
 await expect(anotherLazyRouteContent).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate back to inner lazy route
 await page.goBack();
 await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate to upper inner lazy route
 const navigationToUpper = page.locator('id=navigate-to-upper');
 await expect(navigationToUpper).toBeVisible();
 await navigationToUpper.click();

 await page.waitForTimeout(1000);

 await allTransactionsPromise;

 expect(transactionNamesList.length).toBe(5);
 expect(transactionNamesList).toEqual([
 '/',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/another-lazy/sub/:id/:subId',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/lazy/inner/:id/:anotherId',
 ]);
});

3 changes: 0 additions & 3 deletions
packages/react/src/reactrouter-compat-utils/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
 createV6CompatibleWrapCreateMemoryRouter,
 createV6CompatibleWrapUseRoutes,
 handleNavigation,
 handleExistingNavigationSpan,
 createNewNavigationSpan,
 addResolvedRoutesToParent,
 processResolvedRoutes,
 updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
 resolveRouteNameAndSource,
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand Down

128 changes: 15 additions & 113 deletions
packages/react/src/reactrouter-compat-utils/instrumentation.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
 // Check if this span has already been named to avoid multiple updates
 // But allow updates if this is a forced update (e.g., when lazy routes are loaded)
 const hasBeenNamed =
 !forceUpdate &&
 (
 activeRootSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;
 !forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
 : router.state.location;

 if (router.state.historyAction === 'POP' && activeRootSpan) {
 updatePageloadTransaction({
 activeRootSpan,
 location,
 routes,
 basename,
 allRoutes: Array.from(allRoutes),
 });
 updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
 }

 router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
 options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
 version: V6CompatibleVersion,
): Integration {
 const integration = browserTracingIntegration({
 ...options,
 instrumentPageLoad: false,
 instrumentNavigation: false,
 });
 const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

 const {
 useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
 if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname: targetPath,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 true, // forceUpdate = true since we're loading lazy routes
 _matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
 if (pathname) {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 false, // forceUpdate = false since this is after lazy routes are loaded
 _matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
 basename,
 );

 // Check if this might be a lazy route context
 const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

 const activeSpan = getActiveSpan();
 const spanJson = activeSpan && spanToJSON(activeSpan);
 const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

 // Cross usage can result in multiple navigation spans being created without this check
 if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
 handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
 } else {
 createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
 if (!isAlreadyInNavigationSpan) {
@chargomechargomeSep 5, 2025

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?
@onurtemizkanonurtemizkanSep 5, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).
Turns out it's unnecessary (and also regressed the bug we were trying to fix).
Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.
 startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });
@cursorcursorBotSep 5, 2025

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans
The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.
Fix in CursorFix in Web
 }
 }
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
 });
 isMountRenderPass.current = false;
 } else {
 handleNavigation({
 location,
 routes,
 navigationType,
 version,
 allRoutes: Array.from(allRoutes),
 });
 handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
 }
 },
 // `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
 // Only use this root span if it is a pageload or navigation span
 return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
 * Handles updating an existing navigation span
 */
export function handleExistingNavigationSpan(
 activeSpan: Span,
 spanJson: ReturnType<typeof spanToJSON>,
 name: string,
 source: TransactionSource,
 isLikelyLazyRoute: boolean,
): void {
 // Check if we've already set the name for this span using a custom property
 const hasBeenNamed = (
 activeSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // This is the first time we're setting the name for this span
 if (!spanJson.timestamp) {
 activeSpan?.updateName(name);
 }

 // For lazy routes, don't mark as named yet so it can be updated later
 if (!isLikelyLazyRoute) {
 addNonEnumerableProperty(
 activeSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
 }

 // Always set the source attribute to keep it consistent with the current route
 activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
 * Creates a new navigation span
 */
export function createNewNavigationSpan(
 client: Client,
 name: string,
 source: TransactionSource,
 version: string,
 isLikelyLazyRoute: boolean,
): void {
 const newSpan = startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });

 // For lazy routes, don't mark as named yet so it can be updated later when the route loads
 if (!isLikelyLazyRoute && newSpan) {
 addNonEnumerableProperty(
 newSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
}
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

s1gr1d
merged 5 commits into

from

Sep 9, 2025
 Merged


fix(react): Remove handleExistingNavigation#17534
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
385d041

fix(react): Remove `handleExistingNavigation`


onurtemizkan Sep 5, 2025
415d43f

Remove `isLikelyLazyRoute`


onurtemizkan Sep 5, 2025
0a34f8b

Add more e2e testing


onurtemizkan Sep 5, 2025
71a935f

Remove `createNewNavigationSpan`, set update guard selectively


onurtemizkan Sep 5, 2025
b12ad48

Merge branch 'develop' into onur/remove-handleexistingnavigation


s1gr1d Sep 9, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
 <Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
 Navigate to Another Lazy Route
 </Link>
 <Link to="/lazy/inner/1/2/" id="navigate-to-upper">
 Navigate to Upper Lazy Route
 </Link>
 </div>
 ),
 },
Expand Down

121 changes: 112 additions & 9 deletions
...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
 page,
}) => {
 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();

 // First, navigate to the inner lazy route
 const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(firstEvent.type).toBe('transaction');
 expect(firstEvent.contexts?.trace?.op).toBe('navigation');

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();

 // Now navigate from the inner lazy route to another lazy route
 const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();
 await navigationToAnotherFromInner.click();

 const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
 expect(secondEvent.type).toBe('transaction');
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');

 // Go back to the previous page to ensure history navigation works as expected
 const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
 );
 });

 await page.goBack();

 const goBackEvent = await goBackTransactionPromise;

 // Validate the second go back transaction event
 expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
 expect(goBackEvent.type).toBe('transaction');
 expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

 // Navigate to the upper route
 const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
 );
 });

 const navigationToUpper = page.locator('id=navigate-to-upper');

 await navigationToUpper.click();

 const goUpperRouteEvent = await goUpperRouteTransactionPromise;

 // Validate the go upper route transaction event
 expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
 expect(goUpperRouteEvent.type).toBe('transaction');
 expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
 const transactionNamesList: string[] = [];

 // Monitor and add all transaction names sent to Sentry for the navigations
 const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 if (transactionEvent?.transaction) {
 transactionNamesList.push(transactionEvent.transaction);
 }

 if (transactionNamesList.length >= 5) {
 // Stop monitoring once we have enough transaction names
 return true;
 }

 return false;
 });

 // Go to root page
 await page.goto('/');
 page.waitForTimeout(1000);

 // Navigate to inner lazy route
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 // Navigate to another lazy route
 const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnother).toBeVisible();
 await page.waitForTimeout(1000);

 // Click to navigate to another lazy route
 await navigationToAnother.click();
 const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
 await expect(anotherLazyRouteContent).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate back to inner lazy route
 await page.goBack();
 await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate to upper inner lazy route
 const navigationToUpper = page.locator('id=navigate-to-upper');
 await expect(navigationToUpper).toBeVisible();
 await navigationToUpper.click();

 await page.waitForTimeout(1000);

 await allTransactionsPromise;

 expect(transactionNamesList.length).toBe(5);
 expect(transactionNamesList).toEqual([
 '/',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/another-lazy/sub/:id/:subId',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/lazy/inner/:id/:anotherId',
 ]);
});

3 changes: 0 additions & 3 deletions
packages/react/src/reactrouter-compat-utils/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
 createV6CompatibleWrapCreateMemoryRouter,
 createV6CompatibleWrapUseRoutes,
 handleNavigation,
 handleExistingNavigationSpan,
 createNewNavigationSpan,
 addResolvedRoutesToParent,
 processResolvedRoutes,
 updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
 resolveRouteNameAndSource,
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand Down

128 changes: 15 additions & 113 deletions
packages/react/src/reactrouter-compat-utils/instrumentation.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
 // Check if this span has already been named to avoid multiple updates
 // But allow updates if this is a forced update (e.g., when lazy routes are loaded)
 const hasBeenNamed =
 !forceUpdate &&
 (
 activeRootSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;
 !forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
 : router.state.location;

 if (router.state.historyAction === 'POP' && activeRootSpan) {
 updatePageloadTransaction({
 activeRootSpan,
 location,
 routes,
 basename,
 allRoutes: Array.from(allRoutes),
 });
 updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
 }

 router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
 options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
 version: V6CompatibleVersion,
): Integration {
 const integration = browserTracingIntegration({
 ...options,
 instrumentPageLoad: false,
 instrumentNavigation: false,
 });
 const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

 const {
 useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
 if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname: targetPath,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 true, // forceUpdate = true since we're loading lazy routes
 _matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
 if (pathname) {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 false, // forceUpdate = false since this is after lazy routes are loaded
 _matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
 basename,
 );

 // Check if this might be a lazy route context
 const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

 const activeSpan = getActiveSpan();
 const spanJson = activeSpan && spanToJSON(activeSpan);
 const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

 // Cross usage can result in multiple navigation spans being created without this check
 if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
 handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
 } else {
 createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
 if (!isAlreadyInNavigationSpan) {
@chargomechargomeSep 5, 2025

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?
@onurtemizkanonurtemizkanSep 5, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).
Turns out it's unnecessary (and also regressed the bug we were trying to fix).
Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.
 startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });
@cursorcursorBotSep 5, 2025

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans
The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.
Fix in CursorFix in Web
 }
 }
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
 });
 isMountRenderPass.current = false;
 } else {
 handleNavigation({
 location,
 routes,
 navigationType,
 version,
 allRoutes: Array.from(allRoutes),
 });
 handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
 }
 },
 // `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
 // Only use this root span if it is a pageload or navigation span
 return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
 * Handles updating an existing navigation span
 */
export function handleExistingNavigationSpan(
 activeSpan: Span,
 spanJson: ReturnType<typeof spanToJSON>,
 name: string,
 source: TransactionSource,
 isLikelyLazyRoute: boolean,
): void {
 // Check if we've already set the name for this span using a custom property
 const hasBeenNamed = (
 activeSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // This is the first time we're setting the name for this span
 if (!spanJson.timestamp) {
 activeSpan?.updateName(name);
 }

 // For lazy routes, don't mark as named yet so it can be updated later
 if (!isLikelyLazyRoute) {
 addNonEnumerableProperty(
 activeSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
 }

 // Always set the source attribute to keep it consistent with the current route
 activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
 * Creates a new navigation span
 */
export function createNewNavigationSpan(
 client: Client,
 name: string,
 source: TransactionSource,
 version: string,
 isLikelyLazyRoute: boolean,
): void {
 const newSpan = startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });

 // For lazy routes, don't mark as named yet so it can be updated later when the route loads
 if (!isLikelyLazyRoute && newSpan) {
 addNonEnumerableProperty(
 newSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
}
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

s1gr1d
merged 5 commits into

from

Sep 9, 2025
 Merged


fix(react): Remove handleExistingNavigation#17534
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
385d041

fix(react): Remove `handleExistingNavigation`


onurtemizkan Sep 5, 2025
415d43f

Remove `isLikelyLazyRoute`


onurtemizkan Sep 5, 2025
0a34f8b

Add more e2e testing


onurtemizkan Sep 5, 2025
71a935f

Remove `createNewNavigationSpan`, set update guard selectively


onurtemizkan Sep 5, 2025
b12ad48

Merge branch 'develop' into onur/remove-handleexistingnavigation


s1gr1d Sep 9, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
 <Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
 Navigate to Another Lazy Route
 </Link>
 <Link to="/lazy/inner/1/2/" id="navigate-to-upper">
 Navigate to Upper Lazy Route
 </Link>
 </div>
 ),
 },
Expand Down

121 changes: 112 additions & 9 deletions
...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
 page,
}) => {
 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();

 // First, navigate to the inner lazy route
 const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(firstEvent.type).toBe('transaction');
 expect(firstEvent.contexts?.trace?.op).toBe('navigation');

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();

 // Now navigate from the inner lazy route to another lazy route
 const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();
 await navigationToAnotherFromInner.click();

 const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
 expect(secondEvent.type).toBe('transaction');
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');

 // Go back to the previous page to ensure history navigation works as expected
 const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
 );
 });

 await page.goBack();

 const goBackEvent = await goBackTransactionPromise;

 // Validate the second go back transaction event
 expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
 expect(goBackEvent.type).toBe('transaction');
 expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

 // Navigate to the upper route
 const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
 );
 });

 const navigationToUpper = page.locator('id=navigate-to-upper');

 await navigationToUpper.click();

 const goUpperRouteEvent = await goUpperRouteTransactionPromise;

 // Validate the go upper route transaction event
 expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
 expect(goUpperRouteEvent.type).toBe('transaction');
 expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
 const transactionNamesList: string[] = [];

 // Monitor and add all transaction names sent to Sentry for the navigations
 const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 if (transactionEvent?.transaction) {
 transactionNamesList.push(transactionEvent.transaction);
 }

 if (transactionNamesList.length >= 5) {
 // Stop monitoring once we have enough transaction names
 return true;
 }

 return false;
 });

 // Go to root page
 await page.goto('/');
 page.waitForTimeout(1000);

 // Navigate to inner lazy route
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 // Navigate to another lazy route
 const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnother).toBeVisible();
 await page.waitForTimeout(1000);

 // Click to navigate to another lazy route
 await navigationToAnother.click();
 const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
 await expect(anotherLazyRouteContent).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate back to inner lazy route
 await page.goBack();
 await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate to upper inner lazy route
 const navigationToUpper = page.locator('id=navigate-to-upper');
 await expect(navigationToUpper).toBeVisible();
 await navigationToUpper.click();

 await page.waitForTimeout(1000);

 await allTransactionsPromise;

 expect(transactionNamesList.length).toBe(5);
 expect(transactionNamesList).toEqual([
 '/',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/another-lazy/sub/:id/:subId',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/lazy/inner/:id/:anotherId',
 ]);
});

3 changes: 0 additions & 3 deletions
packages/react/src/reactrouter-compat-utils/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
 createV6CompatibleWrapCreateMemoryRouter,
 createV6CompatibleWrapUseRoutes,
 handleNavigation,
 handleExistingNavigationSpan,
 createNewNavigationSpan,
 addResolvedRoutesToParent,
 processResolvedRoutes,
 updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
 resolveRouteNameAndSource,
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand Down

128 changes: 15 additions & 113 deletions
packages/react/src/reactrouter-compat-utils/instrumentation.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
 // Check if this span has already been named to avoid multiple updates
 // But allow updates if this is a forced update (e.g., when lazy routes are loaded)
 const hasBeenNamed =
 !forceUpdate &&
 (
 activeRootSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;
 !forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
 : router.state.location;

 if (router.state.historyAction === 'POP' && activeRootSpan) {
 updatePageloadTransaction({
 activeRootSpan,
 location,
 routes,
 basename,
 allRoutes: Array.from(allRoutes),
 });
 updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
 }

 router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
 options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
 version: V6CompatibleVersion,
): Integration {
 const integration = browserTracingIntegration({
 ...options,
 instrumentPageLoad: false,
 instrumentNavigation: false,
 });
 const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

 const {
 useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
 if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname: targetPath,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 true, // forceUpdate = true since we're loading lazy routes
 _matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
 if (pathname) {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 false, // forceUpdate = false since this is after lazy routes are loaded
 _matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
 basename,
 );

 // Check if this might be a lazy route context
 const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

 const activeSpan = getActiveSpan();
 const spanJson = activeSpan && spanToJSON(activeSpan);
 const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

 // Cross usage can result in multiple navigation spans being created without this check
 if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
 handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
 } else {
 createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
 if (!isAlreadyInNavigationSpan) {
@chargomechargomeSep 5, 2025

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?
@onurtemizkanonurtemizkanSep 5, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).
Turns out it's unnecessary (and also regressed the bug we were trying to fix).
Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.
 startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });
@cursorcursorBotSep 5, 2025

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans
The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.
Fix in CursorFix in Web
 }
 }
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
 });
 isMountRenderPass.current = false;
 } else {
 handleNavigation({
 location,
 routes,
 navigationType,
 version,
 allRoutes: Array.from(allRoutes),
 });
 handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
 }
 },
 // `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
 // Only use this root span if it is a pageload or navigation span
 return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
 * Handles updating an existing navigation span
 */
export function handleExistingNavigationSpan(
 activeSpan: Span,
 spanJson: ReturnType<typeof spanToJSON>,
 name: string,
 source: TransactionSource,
 isLikelyLazyRoute: boolean,
): void {
 // Check if we've already set the name for this span using a custom property
 const hasBeenNamed = (
 activeSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // This is the first time we're setting the name for this span
 if (!spanJson.timestamp) {
 activeSpan?.updateName(name);
 }

 // For lazy routes, don't mark as named yet so it can be updated later
 if (!isLikelyLazyRoute) {
 addNonEnumerableProperty(
 activeSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
 }

 // Always set the source attribute to keep it consistent with the current route
 activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
 * Creates a new navigation span
 */
export function createNewNavigationSpan(
 client: Client,
 name: string,
 source: TransactionSource,
 version: string,
 isLikelyLazyRoute: boolean,
): void {
 const newSpan = startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });

 // For lazy routes, don't mark as named yet so it can be updated later when the route loads
 if (!isLikelyLazyRoute && newSpan) {
 addNonEnumerableProperty(
 newSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
}
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

s1gr1d
merged 5 commits into

from

Sep 9, 2025
 Merged


fix(react): Remove handleExistingNavigation#17534
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
385d041

fix(react): Remove `handleExistingNavigation`


onurtemizkan Sep 5, 2025
415d43f

Remove `isLikelyLazyRoute`


onurtemizkan Sep 5, 2025
0a34f8b

Add more e2e testing


onurtemizkan Sep 5, 2025
71a935f

Remove `createNewNavigationSpan`, set update guard selectively


onurtemizkan Sep 5, 2025
b12ad48

Merge branch 'develop' into onur/remove-handleexistingnavigation


s1gr1d Sep 9, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
 <Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
 Navigate to Another Lazy Route
 </Link>
 <Link to="/lazy/inner/1/2/" id="navigate-to-upper">
 Navigate to Upper Lazy Route
 </Link>
 </div>
 ),
 },
Expand Down

121 changes: 112 additions & 9 deletions
...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
 page,
}) => {
 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();

 // First, navigate to the inner lazy route
 const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(firstEvent.type).toBe('transaction');
 expect(firstEvent.contexts?.trace?.op).toBe('navigation');

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();

 // Now navigate from the inner lazy route to another lazy route
 const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();
 await navigationToAnotherFromInner.click();

 const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
 expect(secondEvent.type).toBe('transaction');
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');

 // Go back to the previous page to ensure history navigation works as expected
 const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
 );
 });

 await page.goBack();

 const goBackEvent = await goBackTransactionPromise;

 // Validate the second go back transaction event
 expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
 expect(goBackEvent.type).toBe('transaction');
 expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

 // Navigate to the upper route
 const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
 );
 });

 const navigationToUpper = page.locator('id=navigate-to-upper');

 await navigationToUpper.click();

 const goUpperRouteEvent = await goUpperRouteTransactionPromise;

 // Validate the go upper route transaction event
 expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
 expect(goUpperRouteEvent.type).toBe('transaction');
 expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
 const transactionNamesList: string[] = [];

 // Monitor and add all transaction names sent to Sentry for the navigations
 const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 if (transactionEvent?.transaction) {
 transactionNamesList.push(transactionEvent.transaction);
 }

 if (transactionNamesList.length >= 5) {
 // Stop monitoring once we have enough transaction names
 return true;
 }

 return false;
 });

 // Go to root page
 await page.goto('/');
 page.waitForTimeout(1000);

 // Navigate to inner lazy route
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 // Navigate to another lazy route
 const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnother).toBeVisible();
 await page.waitForTimeout(1000);

 // Click to navigate to another lazy route
 await navigationToAnother.click();
 const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
 await expect(anotherLazyRouteContent).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate back to inner lazy route
 await page.goBack();
 await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate to upper inner lazy route
 const navigationToUpper = page.locator('id=navigate-to-upper');
 await expect(navigationToUpper).toBeVisible();
 await navigationToUpper.click();

 await page.waitForTimeout(1000);

 await allTransactionsPromise;

 expect(transactionNamesList.length).toBe(5);
 expect(transactionNamesList).toEqual([
 '/',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/another-lazy/sub/:id/:subId',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/lazy/inner/:id/:anotherId',
 ]);
});

3 changes: 0 additions & 3 deletions
packages/react/src/reactrouter-compat-utils/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
 createV6CompatibleWrapCreateMemoryRouter,
 createV6CompatibleWrapUseRoutes,
 handleNavigation,
 handleExistingNavigationSpan,
 createNewNavigationSpan,
 addResolvedRoutesToParent,
 processResolvedRoutes,
 updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
 resolveRouteNameAndSource,
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand Down

128 changes: 15 additions & 113 deletions
packages/react/src/reactrouter-compat-utils/instrumentation.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
 // Check if this span has already been named to avoid multiple updates
 // But allow updates if this is a forced update (e.g., when lazy routes are loaded)
 const hasBeenNamed =
 !forceUpdate &&
 (
 activeRootSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;
 !forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
 : router.state.location;

 if (router.state.historyAction === 'POP' && activeRootSpan) {
 updatePageloadTransaction({
 activeRootSpan,
 location,
 routes,
 basename,
 allRoutes: Array.from(allRoutes),
 });
 updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
 }

 router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
 options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
 version: V6CompatibleVersion,
): Integration {
 const integration = browserTracingIntegration({
 ...options,
 instrumentPageLoad: false,
 instrumentNavigation: false,
 });
 const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

 const {
 useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
 if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname: targetPath,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 true, // forceUpdate = true since we're loading lazy routes
 _matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
 if (pathname) {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 false, // forceUpdate = false since this is after lazy routes are loaded
 _matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
 basename,
 );

 // Check if this might be a lazy route context
 const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

 const activeSpan = getActiveSpan();
 const spanJson = activeSpan && spanToJSON(activeSpan);
 const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

 // Cross usage can result in multiple navigation spans being created without this check
 if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
 handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
 } else {
 createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
 if (!isAlreadyInNavigationSpan) {
@chargomechargomeSep 5, 2025

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?
@onurtemizkanonurtemizkanSep 5, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).
Turns out it's unnecessary (and also regressed the bug we were trying to fix).
Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.
 startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });
@cursorcursorBotSep 5, 2025

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans
The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.
Fix in CursorFix in Web
 }
 }
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
 });
 isMountRenderPass.current = false;
 } else {
 handleNavigation({
 location,
 routes,
 navigationType,
 version,
 allRoutes: Array.from(allRoutes),
 });
 handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
 }
 },
 // `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
 // Only use this root span if it is a pageload or navigation span
 return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
 * Handles updating an existing navigation span
 */
export function handleExistingNavigationSpan(
 activeSpan: Span,
 spanJson: ReturnType<typeof spanToJSON>,
 name: string,
 source: TransactionSource,
 isLikelyLazyRoute: boolean,
): void {
 // Check if we've already set the name for this span using a custom property
 const hasBeenNamed = (
 activeSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // This is the first time we're setting the name for this span
 if (!spanJson.timestamp) {
 activeSpan?.updateName(name);
 }

 // For lazy routes, don't mark as named yet so it can be updated later
 if (!isLikelyLazyRoute) {
 addNonEnumerableProperty(
 activeSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
 }

 // Always set the source attribute to keep it consistent with the current route
 activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
 * Creates a new navigation span
 */
export function createNewNavigationSpan(
 client: Client,
 name: string,
 source: TransactionSource,
 version: string,
 isLikelyLazyRoute: boolean,
): void {
 const newSpan = startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });

 // For lazy routes, don't mark as named yet so it can be updated later when the route loads
 if (!isLikelyLazyRoute && newSpan) {
 addNonEnumerableProperty(
 newSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
}
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

s1gr1d
merged 5 commits into

from

Sep 9, 2025
 Merged


fix(react): Remove handleExistingNavigation#17534
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
385d041

fix(react): Remove `handleExistingNavigation`


onurtemizkan Sep 5, 2025
415d43f

Remove `isLikelyLazyRoute`


onurtemizkan Sep 5, 2025
0a34f8b

Add more e2e testing


onurtemizkan Sep 5, 2025
71a935f

Remove `createNewNavigationSpan`, set update guard selectively


onurtemizkan Sep 5, 2025
b12ad48

Merge branch 'develop' into onur/remove-handleexistingnavigation


s1gr1d Sep 9, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
 <Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
 Navigate to Another Lazy Route
 </Link>
 <Link to="/lazy/inner/1/2/" id="navigate-to-upper">
 Navigate to Upper Lazy Route
 </Link>
 </div>
 ),
 },
Expand Down

121 changes: 112 additions & 9 deletions
...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
 page,
}) => {
 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();

 // First, navigate to the inner lazy route
 const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(firstEvent.type).toBe('transaction');
 expect(firstEvent.contexts?.trace?.op).toBe('navigation');

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();

 // Now navigate from the inner lazy route to another lazy route
 const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();
 await navigationToAnotherFromInner.click();

 const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
 expect(secondEvent.type).toBe('transaction');
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');

 // Go back to the previous page to ensure history navigation works as expected
 const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
 );
 });

 await page.goBack();

 const goBackEvent = await goBackTransactionPromise;

 // Validate the second go back transaction event
 expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
 expect(goBackEvent.type).toBe('transaction');
 expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

 // Navigate to the upper route
 const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
 );
 });

 const navigationToUpper = page.locator('id=navigate-to-upper');

 await navigationToUpper.click();

 const goUpperRouteEvent = await goUpperRouteTransactionPromise;

 // Validate the go upper route transaction event
 expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
 expect(goUpperRouteEvent.type).toBe('transaction');
 expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
 const transactionNamesList: string[] = [];

 // Monitor and add all transaction names sent to Sentry for the navigations
 const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 if (transactionEvent?.transaction) {
 transactionNamesList.push(transactionEvent.transaction);
 }

 if (transactionNamesList.length >= 5) {
 // Stop monitoring once we have enough transaction names
 return true;
 }

 return false;
 });

 // Go to root page
 await page.goto('/');
 page.waitForTimeout(1000);

 // Navigate to inner lazy route
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 // Navigate to another lazy route
 const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnother).toBeVisible();
 await page.waitForTimeout(1000);

 // Click to navigate to another lazy route
 await navigationToAnother.click();
 const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
 await expect(anotherLazyRouteContent).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate back to inner lazy route
 await page.goBack();
 await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate to upper inner lazy route
 const navigationToUpper = page.locator('id=navigate-to-upper');
 await expect(navigationToUpper).toBeVisible();
 await navigationToUpper.click();

 await page.waitForTimeout(1000);

 await allTransactionsPromise;

 expect(transactionNamesList.length).toBe(5);
 expect(transactionNamesList).toEqual([
 '/',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/another-lazy/sub/:id/:subId',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/lazy/inner/:id/:anotherId',
 ]);
});

3 changes: 0 additions & 3 deletions
packages/react/src/reactrouter-compat-utils/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
 createV6CompatibleWrapCreateMemoryRouter,
 createV6CompatibleWrapUseRoutes,
 handleNavigation,
 handleExistingNavigationSpan,
 createNewNavigationSpan,
 addResolvedRoutesToParent,
 processResolvedRoutes,
 updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
 resolveRouteNameAndSource,
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand Down

128 changes: 15 additions & 113 deletions
packages/react/src/reactrouter-compat-utils/instrumentation.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
 // Check if this span has already been named to avoid multiple updates
 // But allow updates if this is a forced update (e.g., when lazy routes are loaded)
 const hasBeenNamed =
 !forceUpdate &&
 (
 activeRootSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;
 !forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
 : router.state.location;

 if (router.state.historyAction === 'POP' && activeRootSpan) {
 updatePageloadTransaction({
 activeRootSpan,
 location,
 routes,
 basename,
 allRoutes: Array.from(allRoutes),
 });
 updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
 }

 router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
 options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
 version: V6CompatibleVersion,
): Integration {
 const integration = browserTracingIntegration({
 ...options,
 instrumentPageLoad: false,
 instrumentNavigation: false,
 });
 const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

 const {
 useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
 if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname: targetPath,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 true, // forceUpdate = true since we're loading lazy routes
 _matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
 if (pathname) {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 false, // forceUpdate = false since this is after lazy routes are loaded
 _matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
 basename,
 );

 // Check if this might be a lazy route context
 const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

 const activeSpan = getActiveSpan();
 const spanJson = activeSpan && spanToJSON(activeSpan);
 const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

 // Cross usage can result in multiple navigation spans being created without this check
 if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
 handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
 } else {
 createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
 if (!isAlreadyInNavigationSpan) {
@chargomechargomeSep 5, 2025

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?
@onurtemizkanonurtemizkanSep 5, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).
Turns out it's unnecessary (and also regressed the bug we were trying to fix).
Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.
 startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });
@cursorcursorBotSep 5, 2025

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans
The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.
Fix in CursorFix in Web
 }
 }
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
 });
 isMountRenderPass.current = false;
 } else {
 handleNavigation({
 location,
 routes,
 navigationType,
 version,
 allRoutes: Array.from(allRoutes),
 });
 handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
 }
 },
 // `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
 // Only use this root span if it is a pageload or navigation span
 return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
 * Handles updating an existing navigation span
 */
export function handleExistingNavigationSpan(
 activeSpan: Span,
 spanJson: ReturnType<typeof spanToJSON>,
 name: string,
 source: TransactionSource,
 isLikelyLazyRoute: boolean,
): void {
 // Check if we've already set the name for this span using a custom property
 const hasBeenNamed = (
 activeSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // This is the first time we're setting the name for this span
 if (!spanJson.timestamp) {
 activeSpan?.updateName(name);
 }

 // For lazy routes, don't mark as named yet so it can be updated later
 if (!isLikelyLazyRoute) {
 addNonEnumerableProperty(
 activeSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
 }

 // Always set the source attribute to keep it consistent with the current route
 activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
 * Creates a new navigation span
 */
export function createNewNavigationSpan(
 client: Client,
 name: string,
 source: TransactionSource,
 version: string,
 isLikelyLazyRoute: boolean,
): void {
 const newSpan = startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });

 // For lazy routes, don't mark as named yet so it can be updated later when the route loads
 if (!isLikelyLazyRoute && newSpan) {
 addNonEnumerableProperty(
 newSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
}
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix(react): Remove handleExistingNavigation#17534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

s1gr1d
merged 5 commits into

from

Sep 9, 2025
 Merged


fix(react): Remove handleExistingNavigation#17534
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
385d041

fix(react): Remove `handleExistingNavigation`


onurtemizkan Sep 5, 2025
415d43f

Remove `isLikelyLazyRoute`


onurtemizkan Sep 5, 2025
0a34f8b

Add more e2e testing


onurtemizkan Sep 5, 2025
71a935f

Remove `createNewNavigationSpan`, set update guard selectively


onurtemizkan Sep 5, 2025
b12ad48

Merge branch 'develop' into onur/remove-handleexistingnavigation


s1gr1d Sep 9, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
...ages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,9 @@ export const someMoreNestedRoutes = [
 <Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
 Navigate to Another Lazy Route
 </Link>
 <Link to="/lazy/inner/1/2/" id="navigate-to-upper">
 Navigate to Upper Lazy Route
 </Link>
 </div>
 ),
 },
Expand Down

121 changes: 112 additions & 9 deletions
...ackages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -107,7 +107,15 @@ test('Creates navigation transactions between two different lazy routes', async
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');
});

test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
test('Creates navigation transactions from inner lazy route to another lazy route with history navigation', async ({
 page,
}) => {
 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();

 // First, navigate to the inner lazy route
 const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -117,11 +125,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 await page.goto('/');

 // Navigate to inner lazy route first
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 const firstEvent = await firstTransactionPromise;
Expand All@@ -135,6 +138,10 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(firstEvent.type).toBe('transaction');
 expect(firstEvent.contexts?.trace?.op).toBe('navigation');

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();

 // Now navigate from the inner lazy route to another lazy route
 const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
Expand All@@ -144,9 +151,6 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 );
 });

 // Click the navigation link from within the inner lazy route to another lazy route
 const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnotherFromInner).toBeVisible();
 await navigationToAnotherFromInner.click();

 const secondEvent = await secondTransactionPromise;
Expand All@@ -159,4 +163,103 @@ test('Creates navigation transactions from inner lazy route to another lazy rout
 expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
 expect(secondEvent.type).toBe('transaction');
 expect(secondEvent.contexts?.trace?.op).toBe('navigation');

 // Go back to the previous page to ensure history navigation works as expected
 const goBackTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
 );
 });

 await page.goBack();

 const goBackEvent = await goBackTransactionPromise;

 // Validate the second go back transaction event
 expect(goBackEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
 expect(goBackEvent.type).toBe('transaction');
 expect(goBackEvent.contexts?.trace?.op).toBe('navigation');

 // Navigate to the upper route
 const goUpperRouteTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 return (
 !!transactionEvent?.transaction &&
 transactionEvent.contexts?.trace?.op === 'navigation' &&
 transactionEvent.transaction === '/lazy/inner/:id/:anotherId'
 );
 });

 const navigationToUpper = page.locator('id=navigate-to-upper');

 await navigationToUpper.click();

 const goUpperRouteEvent = await goUpperRouteTransactionPromise;

 // Validate the go upper route transaction event
 expect(goUpperRouteEvent.transaction).toBe('/lazy/inner/:id/:anotherId');
 expect(goUpperRouteEvent.type).toBe('transaction');
 expect(goUpperRouteEvent.contexts?.trace?.op).toBe('navigation');
});

test('Does not send any duplicate navigation transaction names browsing between different routes', async ({ page }) => {
 const transactionNamesList: string[] = [];

 // Monitor and add all transaction names sent to Sentry for the navigations
 const allTransactionsPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
 if (transactionEvent?.transaction) {
 transactionNamesList.push(transactionEvent.transaction);
 }

 if (transactionNamesList.length >= 5) {
 // Stop monitoring once we have enough transaction names
 return true;
 }

 return false;
 });

 // Go to root page
 await page.goto('/');
 page.waitForTimeout(1000);

 // Navigate to inner lazy route
 const navigationToInner = page.locator('id=navigation');
 await expect(navigationToInner).toBeVisible();
 await navigationToInner.click();

 // Navigate to another lazy route
 const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
 await expect(navigationToAnother).toBeVisible();
 await page.waitForTimeout(1000);

 // Click to navigate to another lazy route
 await navigationToAnother.click();
 const anotherLazyRouteContent = page.locator('id=another-lazy-route-deep');
 await expect(anotherLazyRouteContent).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate back to inner lazy route
 await page.goBack();
 await expect(page.locator('id=innermost-lazy-route')).toBeVisible();
 await page.waitForTimeout(1000);

 // Navigate to upper inner lazy route
 const navigationToUpper = page.locator('id=navigate-to-upper');
 await expect(navigationToUpper).toBeVisible();
 await navigationToUpper.click();

 await page.waitForTimeout(1000);

 await allTransactionsPromise;

 expect(transactionNamesList.length).toBe(5);
 expect(transactionNamesList).toEqual([
 '/',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/another-lazy/sub/:id/:subId',
 '/lazy/inner/:id/:anotherId/:someAnotherId',
 '/lazy/inner/:id/:anotherId',
 ]);
});

3 changes: 0 additions & 3 deletions
packages/react/src/reactrouter-compat-utils/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -9,8 +9,6 @@ export {
 createV6CompatibleWrapCreateMemoryRouter,
 createV6CompatibleWrapUseRoutes,
 handleNavigation,
 handleExistingNavigationSpan,
 createNewNavigationSpan,
 addResolvedRoutesToParent,
 processResolvedRoutes,
 updateNavigationSpan,
Expand All@@ -21,7 +19,6 @@ export {
 resolveRouteNameAndSource,
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand Down

128 changes: 15 additions & 113 deletions
packages/react/src/reactrouter-compat-utils/instrumentation.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,6 @@ import { checkRouteForAsyncHandler } from './lazy-routes';
import {
 getNormalizedName,
 initializeRouterUtils,
 isLikelyLazyRouteContext,
 locationIsInsideDescendantRoute,
 prefixWithSlash,
 rebuildRoutePathFromAllRoutes,
Expand DownExpand Up@@ -176,12 +175,7 @@ export function updateNavigationSpan(
 // Check if this span has already been named to avoid multiple updates
 // But allow updates if this is a forced update (e.g., when lazy routes are loaded)
 const hasBeenNamed =
 !forceUpdate &&
 (
 activeRootSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;
 !forceUpdate && (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // Get fresh branches for the current location with all loaded routes
Expand DownExpand Up@@ -355,13 +349,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
 : router.state.location;

 if (router.state.historyAction === 'POP' && activeRootSpan) {
 updatePageloadTransaction({
 activeRootSpan,
 location,
 routes,
 basename,
 allRoutes: Array.from(allRoutes),
 });
 updatePageloadTransaction({ activeRootSpan, location, routes, basename, allRoutes: Array.from(allRoutes) });
 }

 router.subscribe((state: RouterState) => {
Expand DownExpand Up@@ -389,11 +377,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
 options: Parameters<typeof browserTracingIntegration>[0] & ReactRouterOptions,
 version: V6CompatibleVersion,
): Integration {
 const integration = browserTracingIntegration({
 ...options,
 instrumentPageLoad: false,
 instrumentNavigation: false,
 });
 const integration = browserTracingIntegration({ ...options, instrumentPageLoad: false, instrumentNavigation: false });

 const {
 useEffect,
Expand DownExpand Up@@ -532,13 +516,7 @@ function wrapPatchRoutesOnNavigation(
 if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname: targetPath,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname: targetPath, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 true, // forceUpdate = true since we're loading lazy routes
 _matchRoutes,
Expand All@@ -559,13 +537,7 @@ function wrapPatchRoutesOnNavigation(
 if (pathname) {
 updateNavigationSpan(
 activeRootSpan,
 {
 pathname,
 search: '',
 hash: '',
 state: null,
 key: 'default',
 },
 { pathname, search: '', hash: '', state: null, key: 'default' },
 Array.from(allRoutes),
 false, // forceUpdate = false since this is after lazy routes are loaded
 _matchRoutes,
Expand DownExpand Up@@ -604,18 +576,20 @@ export function handleNavigation(opts: {
 basename,
 );

 // Check if this might be a lazy route context
 const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location);

 const activeSpan = getActiveSpan();
 const spanJson = activeSpan && spanToJSON(activeSpan);
 const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';

 // Cross usage can result in multiple navigation spans being created without this check
 if (isAlreadyInNavigationSpan && activeSpan && spanJson) {
 handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
 } else {
 createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
 if (!isAlreadyInNavigationSpan) {
@chargomechargomeSep 5, 2025

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?
@onurtemizkanonurtemizkanSep 5, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).
Turns out it's unnecessary (and also regressed the bug we were trying to fix).
Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.
 startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });
@cursorcursorBotSep 5, 2025

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation Tracking Fails on Active Spans
The handleNavigation function no longer updates active navigation spans with new route information. If a span is already active, subsequent navigation events are ignored. This can lead to missed navigation tracking and incorrect transaction names, especially with lazy routes or quick user navigation.
Fix in CursorFix in Web
 }
 }
}
Expand DownExpand Up@@ -726,13 +700,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
 });
 isMountRenderPass.current = false;
 } else {
 handleNavigation({
 location,
 routes,
 navigationType,
 version,
 allRoutes: Array.from(allRoutes),
 });
 handleNavigation({ location, routes, navigationType, version, allRoutes: Array.from(allRoutes) });
 }
 },
 // `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
Expand DownExpand Up@@ -765,69 +733,3 @@ function getActiveRootSpan(): Span | undefined {
 // Only use this root span if it is a pageload or navigation span
 return op === 'navigation' || op === 'pageload' ? rootSpan : undefined;
}

/**
 * Handles updating an existing navigation span
 */
export function handleExistingNavigationSpan(
 activeSpan: Span,
 spanJson: ReturnType<typeof spanToJSON>,
 name: string,
 source: TransactionSource,
 isLikelyLazyRoute: boolean,
): void {
 // Check if we've already set the name for this span using a custom property
 const hasBeenNamed = (
 activeSpan as {
 __sentry_navigation_name_set__?: boolean;
 }
 )?.__sentry_navigation_name_set__;

 if (!hasBeenNamed) {
 // This is the first time we're setting the name for this span
 if (!spanJson.timestamp) {
 activeSpan?.updateName(name);
 }

 // For lazy routes, don't mark as named yet so it can be updated later
 if (!isLikelyLazyRoute) {
 addNonEnumerableProperty(
 activeSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
 }

 // Always set the source attribute to keep it consistent with the current route
 activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
}

/**
 * Creates a new navigation span
 */
export function createNewNavigationSpan(
 client: Client,
 name: string,
 source: TransactionSource,
 version: string,
 isLikelyLazyRoute: boolean,
): void {
 const newSpan = startBrowserTracingNavigationSpan(client, {
 name,
 attributes: {
 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
 [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
 [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
 },
 });

 // For lazy routes, don't mark as named yet so it can be updated later when the route loads
 if (!isLikelyLazyRoute && newSpan) {
 addNonEnumerableProperty(
 newSpan as { __sentry_navigation_name_set__?: boolean },
 '__sentry_navigation_name_set__',
 true,
 );
 }
}
Loading
Loading