From 5cdb54013aae46fde815fc1ab3a9e8a3dd29fbd8 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 9 Jul 2026 11:04:46 +0200 Subject: [PATCH 1/3] fix(react-router): Correct url attributes for history and relative navigations The url.* attribute work surfaced incorrect destination URLs for navigate(-1), navigate(1), and relative navigate() targets. Update url.path/url.full after numeric navigation completes and match route parameterization against url.path for relative navigations. Co-Authored-By: Cursor --- .../performance/navigation.client.test.ts | 12 ++- .../app/routes/performance/ssr.tsx | 7 +- .../performance/navigation.client.test.ts | 69 +++++++++++++++++- .../performance/navigation.client.test.ts | 12 ++- .../performance/navigation.client.test.ts | 6 +- .../performance/navigation.client.test.ts | 6 +- .../app/routes/performance/ssr.tsx | 7 ++ .../performance/navigation.client.test.ts | 73 ++++++++++++++++++- .../app/routes/performance/ssr.tsx | 7 ++ .../performance/navigation.client.test.ts | 73 ++++++++++++++++++- .../src/client/createClientInstrumentation.ts | 6 +- .../react-router/src/client/hydratedRouter.ts | 72 +++++++++++++++--- packages/react-router/src/client/utils.ts | 20 +++++ .../createClientInstrumentation.test.ts | 40 +++++++++- .../test/client/hydratedRouter.test.ts | 66 ++++++++++++++++- .../react-router/test/client/utils.test.ts | 36 ++++++++- 16 files changed, 473 insertions(+), 39 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/performance/navigation.client.test.ts index 265857b35afb..3432b95ddae3 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/performance/navigation.client.test.ts @@ -10,8 +10,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'SSR Page' }).click(); // navigation const transaction = await navigationPromise; @@ -67,8 +71,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'With Param Page' }).click(); // navigation const transaction = await txPromise; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/ssr.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/ssr.tsx index 0b4831496c3c..41c006631bf7 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/ssr.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/ssr.tsx @@ -1,12 +1,17 @@ -import { Link } from 'react-router'; +import { Link, useNavigate } from 'react-router'; export default function SsrPage() { + const navigate = useNavigate(); + return (

SSR Page

+
); } diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts index fd2982432b7b..607646ddfd17 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts @@ -14,9 +14,12 @@ test.describe('client - hybrid navigation (instrumentation API span + legacy par test('should create navigation span via instrumentation API and parameterize via legacy subscribe', async ({ page, }) => { - // First load the performance page + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); - await page.waitForTimeout(1000); + await pageloadTxPromise; const navigationTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { return ( @@ -92,8 +95,12 @@ test.describe('client - hybrid navigation (instrumentation API span + legacy par }); test('should parameterize navigation transaction for dynamic routes', async ({ page }) => { + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); - await page.waitForTimeout(1000); + await pageloadTxPromise; const navigationTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { return ( @@ -126,8 +133,12 @@ test.describe('client - hybrid navigation (instrumentation API span + legacy par }); test('should send multiple navigation transactions in sequence', async ({ page }) => { + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); - await page.waitForTimeout(1000); + await pageloadTxPromise; // First navigation: /performance -> /performance/ssr const firstNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { @@ -171,6 +182,56 @@ test.describe('client - hybrid navigation (instrumentation API span + legacy par type: 'transaction', }); }); + + test('should create navigation transaction for navigate(-1) with correct url attributes', async ({ page }) => { + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/performance`); + await pageloadTxPromise; + + const forwardNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance/ssr' && transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + await page.getByRole('link', { name: 'SSR Page' }).click(); + await forwardNavPromise; + + const backNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + await page.getByRole('button', { name: 'History Back Navigate' }).click(); + + const transaction = await backNavPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react_router.instrumentation_api', + data: { + 'sentry.source': 'route', + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.react_router.instrumentation_api', + 'navigation.type': 'router.back', + 'url.template': '/performance', + // react-router-serve 301-redirects the bare index route to a trailing slash + 'url.path': '/performance/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/$/), + }, + }, + }, + transaction: '/performance', + type: 'transaction', + transaction_info: { source: 'route' }, + }); + }); }); // Tests for instrumentation API navigation - expected to fail until React Router fixes upstream diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/performance/navigation.client.test.ts index 265857b35afb..3432b95ddae3 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/performance/navigation.client.test.ts @@ -10,8 +10,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'SSR Page' }).click(); // navigation const transaction = await navigationPromise; @@ -67,8 +71,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'With Param Page' }).click(); // navigation const transaction = await txPromise; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/navigation.client.test.ts index 37bdf3596f5d..5c1b7c493e67 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/navigation.client.test.ts @@ -11,8 +11,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'With Param Page' }).click(); // navigation const transaction = await txPromise; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/navigation.client.test.ts index 37bdf3596f5d..5c1b7c493e67 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/navigation.client.test.ts @@ -11,8 +11,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'With Param Page' }).click(); // navigation const transaction = await txPromise; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx index 253e964ff15d..8226e68f3be0 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx @@ -1,7 +1,14 @@ +import { useNavigate } from 'react-router'; + export default function SsrPage() { + const navigate = useNavigate(); + return (

SSR Page

+
); } diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts index 3dd5974afb3a..7b3ac5633c48 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts @@ -10,8 +10,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'SSR Page' }).click(); // navigation const transaction = await navigationPromise; @@ -67,8 +71,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'Object Navigate' }).click(); // navigation with object to const transaction = await txPromise; @@ -97,8 +105,12 @@ test.describe('client - navigation performance', () => { return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation'; }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'Search Only Navigate' }).click(); // navigation with search-only object to const transaction = await txPromise; @@ -129,8 +141,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'With Param Page' }).click(); // navigation const transaction = await txPromise; @@ -177,4 +193,53 @@ test.describe('client - navigation performance', () => { tags: { runtime: 'browser' }, }); }); + + test('should create navigation transaction for navigate(-1) with correct url attributes', async ({ page }) => { + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/performance`); + await pageloadTxPromise; + + const forwardNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance/ssr' && transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + await page.getByRole('link', { name: 'SSR Page' }).click(); + await forwardNavPromise; + + const backNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + await page.getByRole('button', { name: 'History Back Navigate' }).click(); + + const transaction = await backNavPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react_router', + data: { + 'sentry.source': 'route', + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.react_router', + 'url.template': '/performance', + // react-router-serve 301-redirects the bare index route to a trailing slash + 'url.path': '/performance/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/$/), + }, + }, + }, + transaction: '/performance', + type: 'transaction', + transaction_info: { source: 'route' }, + }); + }); }); diff --git a/dev-packages/e2e-tests/test-applications/react-router-8-framework/app/routes/performance/ssr.tsx b/dev-packages/e2e-tests/test-applications/react-router-8-framework/app/routes/performance/ssr.tsx index 253e964ff15d..8226e68f3be0 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-8-framework/app/routes/performance/ssr.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-8-framework/app/routes/performance/ssr.tsx @@ -1,7 +1,14 @@ +import { useNavigate } from 'react-router'; + export default function SsrPage() { + const navigate = useNavigate(); + return (

SSR Page

+
); } diff --git a/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts index 3dd5974afb3a..7b3ac5633c48 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts @@ -10,8 +10,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'SSR Page' }).click(); // navigation const transaction = await navigationPromise; @@ -67,8 +71,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'Object Navigate' }).click(); // navigation with object to const transaction = await txPromise; @@ -97,8 +105,12 @@ test.describe('client - navigation performance', () => { return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation'; }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'Search Only Navigate' }).click(); // navigation with search-only object to const transaction = await txPromise; @@ -129,8 +141,12 @@ test.describe('client - navigation performance', () => { ); }); + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + await page.goto(`/performance`); // pageload - await page.waitForTimeout(1000); // give it a sec before navigation + await pageloadTxPromise; await page.getByRole('link', { name: 'With Param Page' }).click(); // navigation const transaction = await txPromise; @@ -177,4 +193,53 @@ test.describe('client - navigation performance', () => { tags: { runtime: 'browser' }, }); }); + + test('should create navigation transaction for navigate(-1) with correct url attributes', async ({ page }) => { + const pageloadTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/performance`); + await pageloadTxPromise; + + const forwardNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance/ssr' && transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + await page.getByRole('link', { name: 'SSR Page' }).click(); + await forwardNavPromise; + + const backNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + await page.getByRole('button', { name: 'History Back Navigate' }).click(); + + const transaction = await backNavPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react_router', + data: { + 'sentry.source': 'route', + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.react_router', + 'url.template': '/performance', + // react-router-serve 301-redirects the bare index route to a trailing slash + 'url.path': '/performance/', + 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/$/), + }, + }, + }, + transaction: '/performance', + type: 'transaction', + transaction_info: { source: 'route' }, + }); + }); }); diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts index 288ddd51907f..c74ddfb1b148 100644 --- a/packages/react-router/src/client/createClientInstrumentation.ts +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -18,7 +18,7 @@ import { import { DEBUG_BUILD } from '../common/debug-build'; import type { ClientInstrumentation, InstrumentableRoute, InstrumentableRouter } from '../common/types'; import { captureInstrumentationError, getPathFromRequest, getPattern, normalizeRoutePath } from '../common/utils'; -import { resolveNavigateAbsoluteUrl, resolveNavigateArg } from './utils'; +import { resolveNavigateAbsoluteUrl, resolveNavigateArg, updateNavigationSpanUrlFromLocation } from './utils'; import { URL_TEMPLATE } from '@sentry/conventions/attributes'; const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; @@ -89,7 +89,7 @@ export function createSentryClientInstrumentation( // If there's an active numeric navigation span, update it instead of creating a duplicate if (currentNumericNavigationSpan) { if (currentNumericNavigationSpan.isRecording()) { - currentNumericNavigationSpan.updateName(pathname); + updateNavigationSpanUrlFromLocation(currentNumericNavigationSpan); } currentNumericNavigationSpan = undefined; return; @@ -161,7 +161,7 @@ export function createSentryClientInstrumentation( const result = await callNavigate(); if (navigationSpan && WINDOW.location) { - navigationSpan.updateName(WINDOW.location.pathname); + updateNavigationSpanUrlFromLocation(navigationSpan); } if (result.status === 'error' && result.error instanceof Error) { diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts index 583b57e9aec5..677dfeca2eb6 100644 --- a/packages/react-router/src/client/hydratedRouter.ts +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -1,4 +1,4 @@ -import { startBrowserTracingNavigationSpan } from '@sentry/browser'; +import { getAbsoluteUrl, startBrowserTracingNavigationSpan } from '@sentry/browser'; import type { Span } from '@sentry/core'; import { debug, @@ -14,13 +14,15 @@ import { import type { DataRouter, RouterState } from 'react-router'; import { DEBUG_BUILD } from '../common/debug-build'; import { isClientInstrumentationApiUsed } from './createClientInstrumentation'; -import { resolveNavigateAbsoluteUrl, resolveNavigateArg } from './utils'; -import { URL_TEMPLATE } from '@sentry/conventions/attributes'; +import { resolveNavigateAbsoluteUrl, resolveNavigateArg, updateNavigationSpanUrlFromLocation } from './utils'; +import { URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes'; const GLOBAL_OBJ_WITH_DATA_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & { __reactRouterDataRouter?: DataRouter; }; +const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; + const MAX_RETRIES = 40; // 2 seconds at 50ms interval /** @@ -62,11 +64,41 @@ export function instrumentHydratedRouter(): void { router.navigate = function sentryPatchedNavigate(...args) { // Skip if instrumentation API is enabled (it handles navigation spans itself) if (!isClientInstrumentationApiUsed()) { - maybeCreateNavigationTransaction( - resolveNavigateArg(args[0]) || '', - resolveNavigateAbsoluteUrl(args[0]), - 'url', - ); + const target = args[0]; + + if (typeof target === 'number') { + // navigate(0) triggers a reload, not a route change — skip span creation + if (target !== 0) { + const currentPathname = WINDOW.location?.pathname || '/'; + const navigationSpan = maybeCreateNavigationTransaction( + currentPathname, + getAbsoluteUrl(currentPathname), + 'url', + ); + + const result = originalNav(...args); + + if (navigationSpan) { + const finalizeNumericNavigation = (): void => { + updateNavigationSpanDestination(navigationSpan, router.state); + }; + + if (result != null && typeof (result as Promise).then === 'function') { + void (result as Promise).then(finalizeNumericNavigation); + } else { + finalizeNumericNavigation(); + } + } + + return result; + } + } else { + maybeCreateNavigationTransaction( + resolveNavigateArg(target) || '', + resolveNavigateAbsoluteUrl(target), + 'url', + ); + } } return originalNav(...args); }; @@ -96,12 +128,15 @@ export function instrumentHydratedRouter(): void { const rootSpanName = rootSpanJson.description; const parameterizedRoute = getParameterizedRoute(newState); + const spanPathname = rootSpanJson.data?.[URL_PATH] as string | undefined; + const destinationPathname = normalizePathname(newState.location.pathname); if ( rootSpanName && newState.navigation.state === 'idle' && // navigation has completed // this event is for the currently active root span - normalizePathname(newState.location.pathname) === normalizePathname(rootSpanName) + (destinationPathname === normalizePathname(rootSpanName) || + (spanPathname && destinationPathname === normalizePathname(spanPathname))) ) { rootSpan.updateName(parameterizedRoute); rootSpan.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: parameterizedRoute }); @@ -150,6 +185,25 @@ function maybeCreateNavigationTransaction(name: string, url: string, source: 'ur ); } +function updateNavigationSpanDestination(span: Span, routerState: RouterState): void { + updateNavigationSpanUrlFromLocation(span); + + if (!WINDOW.location) { + return; + } + + const { pathname } = WINDOW.location; + + if ( + routerState.navigation?.state === 'idle' && + normalizePathname(routerState.location.pathname) === normalizePathname(pathname) + ) { + const parameterizedRoute = getParameterizedRoute(routerState); + span.updateName(parameterizedRoute); + span.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: parameterizedRoute }); + } +} + function getActiveRootSpan(): Span | undefined { const activeSpan = getActiveSpan(); if (!activeSpan) { diff --git a/packages/react-router/src/client/utils.ts b/packages/react-router/src/client/utils.ts index 46c5a618bfe7..f6e8a1a9429d 100644 --- a/packages/react-router/src/client/utils.ts +++ b/packages/react-router/src/client/utils.ts @@ -1,5 +1,7 @@ import { getAbsoluteUrl } from '@sentry/browser'; +import type { Span } from '@sentry/core'; import { GLOBAL_OBJ } from '@sentry/core'; +import { URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; @@ -83,3 +85,21 @@ export function resolveNavigateAbsoluteUrl(target: unknown, currentUrl?: string) return getAbsoluteUrl(destination); } } + +/** + * Updates a navigation span's name and `url.path`/`url.full` from the current `location`. + */ +export function updateNavigationSpanUrlFromLocation(span: Span): void { + if (!WINDOW.location) { + return; + } + + const { pathname, search = '', hash = '' } = WINDOW.location; + const destinationUrl = getAbsoluteUrl(`${pathname}${search}${hash}`); + + span.updateName(pathname); + span.setAttributes({ + [URL_PATH]: pathname, + [URL_FULL]: destinationUrl, + }); +} diff --git a/packages/react-router/test/client/createClientInstrumentation.test.ts b/packages/react-router/test/client/createClientInstrumentation.test.ts index e6eda03d022a..d5c44f4cf6ca 100644 --- a/packages/react-router/test/client/createClientInstrumentation.test.ts +++ b/packages/react-router/test/client/createClientInstrumentation.test.ts @@ -418,7 +418,7 @@ describe('createSentryClientInstrumentation', () => { return { status: 'success', error: undefined }; }); const mockInstrument = vi.fn(); - const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn() }; + const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn(), setAttributes: vi.fn() }; const mockClient = {}; (core.getClient as any).mockReturnValue(mockClient); @@ -444,6 +444,10 @@ describe('createSentryClientInstrumentation', () => { { url: 'https://example.com/current-page' }, ); expect(mockNavigationSpan.updateName).toHaveBeenCalledWith(destination); + expect(mockNavigationSpan.setAttributes).toHaveBeenCalledWith({ + 'url.path': destination, + 'url.full': `https://example.com${destination}`, + }); }, ); @@ -463,6 +467,30 @@ describe('createSentryClientInstrumentation', () => { expect(mockCallNavigate).toHaveBeenCalled(); }); + it('should update url.path and url.full after numeric navigate completes', async () => { + const mockCallNavigate = vi.fn().mockImplementation(async () => { + (globalThis as any).location.pathname = '/previous-page'; + return { status: 'success', error: undefined }; + }); + const mockInstrument = vi.fn(); + const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn(), setAttributes: vi.fn() }; + const mockClient = {}; + + (core.getClient as any).mockReturnValue(mockClient); + (browser.startBrowserTracingNavigationSpan as any).mockReturnValue(mockNavigationSpan); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.router?.({ instrument: mockInstrument }); + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.navigate(mockCallNavigate, { currentUrl: '/current-page', to: -1 }); + + expect(mockNavigationSpan.setAttributes).toHaveBeenCalledWith({ + 'url.path': '/previous-page', + 'url.full': 'https://example.com/previous-page', + }); + }); + it('should set error status on span for failed numeric navigation', async () => { const mockError = new Error('Navigation failed'); const mockCallNavigate = vi.fn().mockImplementation(async () => { @@ -470,7 +498,7 @@ describe('createSentryClientInstrumentation', () => { return { status: 'error', error: mockError }; }); const mockInstrument = vi.fn(); - const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn() }; + const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn(), setAttributes: vi.fn() }; (core.getClient as any).mockReturnValue({}); (browser.startBrowserTracingNavigationSpan as any).mockReturnValue(mockNavigationSpan); @@ -489,7 +517,7 @@ describe('createSentryClientInstrumentation', () => { it('should set navigate hook invoked flag for numeric navigations but NOT for navigate(0)', async () => { const mockInstrument = vi.fn(); - const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn() }; + const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn(), setAttributes: vi.fn() }; (core.getClient as any).mockReturnValue({}); (browser.startBrowserTracingNavigationSpan as any).mockReturnValue(mockNavigationSpan); @@ -700,6 +728,7 @@ describe('createSentryClientInstrumentation', () => { const mockNavigationSpan = { setStatus: vi.fn(), updateName: vi.fn(), + setAttributes: vi.fn(), isRecording: vi.fn().mockReturnValue(true), }; @@ -721,7 +750,10 @@ describe('createSentryClientInstrumentation', () => { // Only ONE span created (not two - no duplicate from popstate) expect(browser.startBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); - expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/previous-page'); + expect(mockNavigationSpan.setAttributes).toHaveBeenLastCalledWith({ + 'url.path': '/previous-page', + 'url.full': 'https://example.com/previous-page', + }); }); it('should create new span on popstate when no numeric navigation is in progress', () => { diff --git a/packages/react-router/test/client/hydratedRouter.test.ts b/packages/react-router/test/client/hydratedRouter.test.ts index 00a081da0278..301a86d14bd9 100644 --- a/packages/react-router/test/client/hydratedRouter.test.ts +++ b/packages/react-router/test/client/hydratedRouter.test.ts @@ -250,18 +250,78 @@ describe('instrumentHydratedRouter', () => { ); }); - it('creates navigation transaction with correct name when navigate is called with a number', () => { + it('parameterizes relative navigation via subscribe when url.path matches destination', () => { + instrumentHydratedRouter(); + mockRouter.navigate('settings'); + + (core.getActiveSpan as any).mockReturnValue(mockNavigationSpan); + (core.spanToJSON as any).mockImplementation((span: any) => ({ + description: 'settings', + op: span === mockNavigationSpan ? 'navigation' : 'pageload', + data: { 'url.path': '/foo/bar/settings' }, + })); + + const callback = mockRouter.subscribe.mock.calls[0][0]; + callback({ + location: { pathname: '/foo/bar/settings' }, + matches: [{ route: { path: '/foo/bar/settings' } }], + navigation: { state: 'idle' }, + }); + + expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/foo/bar/settings'); + expect(mockNavigationSpan.setAttributes).toHaveBeenCalledWith({ + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + 'url.template': '/foo/bar/settings', + }); + }); + + it('creates navigation transaction with current pathname when navigate is called with a number', () => { instrumentHydratedRouter(); mockRouter.navigate(-1); expect(browser.startBrowserTracingNavigationSpan).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - name: '-1', + name: '/foo/bar', }), - { url: 'https://example.com/foo/bar/-1' }, + { url: 'https://example.com/foo/bar' }, ); }); + it('updates navigation span to destination pathname after numeric navigate completes', async () => { + const navigateResult = Promise.resolve(); + mockRouter.navigate = vi.fn().mockImplementation(() => { + (globalThis as any).location.pathname = '/foo'; + mockRouter.state = { + location: { pathname: '/foo' }, + matches: [{ route: { path: '/foo/:id' } }], + navigation: { state: 'idle' }, + }; + return navigateResult; + }); + + instrumentHydratedRouter(); + mockRouter.navigate(-1); + + await navigateResult; + + expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/foo'); + expect(mockNavigationSpan.setAttributes).toHaveBeenCalledWith({ + 'url.path': '/foo', + 'url.full': 'https://example.com/foo', + }); + expect(mockNavigationSpan.updateName).toHaveBeenLastCalledWith('/foo/:id'); + expect(mockNavigationSpan.setAttributes).toHaveBeenLastCalledWith({ + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + 'url.template': '/foo/:id', + }); + }); + + it('does not create navigation span for navigate(0)', () => { + instrumentHydratedRouter(); + mockRouter.navigate(0); + expect(browser.startBrowserTracingNavigationSpan).not.toHaveBeenCalled(); + }); + it('creates navigation span when client instrumentation API is not enabled', () => { // Ensure the flag is not set (default state - instrumentation API not used) delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; diff --git a/packages/react-router/test/client/utils.test.ts b/packages/react-router/test/client/utils.test.ts index 7a87c210f43b..1513d6e5d6c8 100644 --- a/packages/react-router/test/client/utils.test.ts +++ b/packages/react-router/test/client/utils.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { resolveNavigateAbsoluteUrl } from '../../src/client/utils'; +import { resolveNavigateAbsoluteUrl, updateNavigationSpanUrlFromLocation } from '../../src/client/utils'; vi.mock('@sentry/browser', () => ({ getAbsoluteUrl: vi.fn((urlOrPath: string) => { @@ -84,3 +84,37 @@ describe('resolveNavigateAbsoluteUrl', () => { expect(resolveNavigateAbsoluteUrl('ssr')).toBe('https://example.com/performance/ssr'); }); }); + +describe('updateNavigationSpanUrlFromLocation', () => { + const originalLocation = globalThis.location; + + beforeEach(() => { + (globalThis as any).location = { + href: 'https://example.com/foo?bar=1#section', + origin: 'https://example.com', + pathname: '/foo', + search: '?bar=1', + hash: '#section', + }; + }); + + afterEach(() => { + if (originalLocation) { + (globalThis as any).location = originalLocation; + } else { + delete (globalThis as any).location; + } + }); + + it('updates span name and url attributes from location', () => { + const span = { updateName: vi.fn(), setAttributes: vi.fn() } as any; + + updateNavigationSpanUrlFromLocation(span); + + expect(span.updateName).toHaveBeenCalledWith('/foo'); + expect(span.setAttributes).toHaveBeenCalledWith({ + 'url.path': '/foo', + 'url.full': 'https://example.com/foo?bar=1#section', + }); + }); +}); From cd24bfe9bb05819e64e1fff60d4126a6335f95f2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 9 Jul 2026 18:27:53 +0200 Subject: [PATCH 2/3] ref(react-router): Deduplicate router helpers and add edge-case tests Move shared normalizePathname/getParameterizedRoute helpers to utils.ts and add finalizeNavigationSpanFromHydratedRouter so createClientInstrumentation does not need react-router types. Add tests for stale router state during sync numeric navigation. Co-Authored-By: Cursor --- .../src/client/createClientInstrumentation.ts | 9 ++- .../react-router/src/client/hydratedRouter.ts | 46 +++----------- packages/react-router/src/client/utils.ts | 62 ++++++++++++++++++- .../test/client/hydratedRouter.test.ts | 37 +++++++++++ .../react-router/test/client/utils.test.ts | 62 ++++++++++++++++++- 5 files changed, 174 insertions(+), 42 deletions(-) diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts index c74ddfb1b148..9ad9421609a4 100644 --- a/packages/react-router/src/client/createClientInstrumentation.ts +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -18,7 +18,12 @@ import { import { DEBUG_BUILD } from '../common/debug-build'; import type { ClientInstrumentation, InstrumentableRoute, InstrumentableRouter } from '../common/types'; import { captureInstrumentationError, getPathFromRequest, getPattern, normalizeRoutePath } from '../common/utils'; -import { resolveNavigateAbsoluteUrl, resolveNavigateArg, updateNavigationSpanUrlFromLocation } from './utils'; +import { + resolveNavigateAbsoluteUrl, + resolveNavigateArg, + finalizeNavigationSpanFromHydratedRouter, + updateNavigationSpanUrlFromLocation, +} from './utils'; import { URL_TEMPLATE } from '@sentry/conventions/attributes'; const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; @@ -161,7 +166,7 @@ export function createSentryClientInstrumentation( const result = await callNavigate(); if (navigationSpan && WINDOW.location) { - updateNavigationSpanUrlFromLocation(navigationSpan); + finalizeNavigationSpanFromHydratedRouter(navigationSpan); } if (result.status === 'error' && result.error instanceof Error) { diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts index 677dfeca2eb6..68a54838b47e 100644 --- a/packages/react-router/src/client/hydratedRouter.ts +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -11,10 +11,16 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON, } from '@sentry/core'; -import type { DataRouter, RouterState } from 'react-router'; +import type { DataRouter } from 'react-router'; import { DEBUG_BUILD } from '../common/debug-build'; import { isClientInstrumentationApiUsed } from './createClientInstrumentation'; -import { resolveNavigateAbsoluteUrl, resolveNavigateArg, updateNavigationSpanUrlFromLocation } from './utils'; +import { + finalizeNavigationSpanFromRouterState, + getParameterizedRoute, + normalizePathname, + resolveNavigateAbsoluteUrl, + resolveNavigateArg, +} from './utils'; import { URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes'; const GLOBAL_OBJ_WITH_DATA_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & { @@ -80,7 +86,7 @@ export function instrumentHydratedRouter(): void { if (navigationSpan) { const finalizeNumericNavigation = (): void => { - updateNavigationSpanDestination(navigationSpan, router.state); + finalizeNavigationSpanFromRouterState(navigationSpan, router.state); }; if (result != null && typeof (result as Promise).then === 'function') { @@ -185,25 +191,6 @@ function maybeCreateNavigationTransaction(name: string, url: string, source: 'ur ); } -function updateNavigationSpanDestination(span: Span, routerState: RouterState): void { - updateNavigationSpanUrlFromLocation(span); - - if (!WINDOW.location) { - return; - } - - const { pathname } = WINDOW.location; - - if ( - routerState.navigation?.state === 'idle' && - normalizePathname(routerState.location.pathname) === normalizePathname(pathname) - ) { - const parameterizedRoute = getParameterizedRoute(routerState); - span.updateName(parameterizedRoute); - span.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: parameterizedRoute }); - } -} - function getActiveRootSpan(): Span | undefined { const activeSpan = getActiveSpan(); if (!activeSpan) { @@ -217,18 +204,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; } - -function getParameterizedRoute(routerState: RouterState): string { - const lastMatch = routerState.matches[routerState.matches.length - 1]; - return normalizePathname(lastMatch?.route.path ?? routerState.location.pathname); -} - -function normalizePathname(pathname: string): string { - // Ensure it starts with a single slash - let normalized = pathname.startsWith('/') ? pathname : `/${pathname}`; - // Remove trailing slash unless it's the root - if (normalized.length > 1 && normalized.endsWith('/')) { - normalized = normalized.slice(0, -1); - } - return normalized; -} diff --git a/packages/react-router/src/client/utils.ts b/packages/react-router/src/client/utils.ts index f6e8a1a9429d..d0f415458ff4 100644 --- a/packages/react-router/src/client/utils.ts +++ b/packages/react-router/src/client/utils.ts @@ -1,10 +1,15 @@ import { getAbsoluteUrl } from '@sentry/browser'; import type { Span } from '@sentry/core'; -import { GLOBAL_OBJ } from '@sentry/core'; -import { URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; +import { GLOBAL_OBJ, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { URL_FULL, URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes'; +import type { RouterState } from 'react-router'; const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; +const GLOBAL_OBJ_WITH_DATA_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + __reactRouterDataRouter?: { state: RouterState }; +}; + /** * Resolves a navigate argument to a pathname string. * @@ -103,3 +108,56 @@ export function updateNavigationSpanUrlFromLocation(span: Span): void { [URL_FULL]: destinationUrl, }); } + +export function normalizePathname(pathname: string): string { + let normalized = pathname.startsWith('/') ? pathname : `/${pathname}`; + if (normalized.length > 1 && normalized.endsWith('/')) { + normalized = normalized.slice(0, -1); + } + return normalized; +} + +export function getParameterizedRoute(routerState: RouterState): string { + const lastMatch = routerState.matches[routerState.matches.length - 1]; + return normalizePathname(lastMatch?.route.path || routerState.location.pathname); +} + +/** + * Updates a navigation span's URL attributes and parameterizes its name from the router state. + * Used after numeric navigations (`navigate(-1)` / `navigate(1)`) where route hooks may not + * supply a pattern (e.g. index routes). + * + * `url.path` reflects raw `location.pathname` (may include a trailing slash when the server + * redirects index routes), while `url.template` is normalized without trailing slashes. + */ +export function finalizeNavigationSpanFromRouterState(span: Span, routerState: RouterState): void { + updateNavigationSpanUrlFromLocation(span); + + if (!WINDOW.location) { + return; + } + + const { pathname } = WINDOW.location; + + if ( + routerState.navigation?.state === 'idle' && + normalizePathname(routerState.location.pathname) === normalizePathname(pathname) + ) { + const parameterizedRoute = getParameterizedRoute(routerState); + span.updateName(parameterizedRoute); + span.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: parameterizedRoute }); + } +} + +/** + * Finalizes a navigation span after numeric navigation using the hydrated data router when + * available, otherwise falls back to URL attributes from `location` alone. + */ +export function finalizeNavigationSpanFromHydratedRouter(span: Span): void { + const router = GLOBAL_OBJ_WITH_DATA_ROUTER.__reactRouterDataRouter; + if (router) { + finalizeNavigationSpanFromRouterState(span, router.state); + } else { + updateNavigationSpanUrlFromLocation(span); + } +} diff --git a/packages/react-router/test/client/hydratedRouter.test.ts b/packages/react-router/test/client/hydratedRouter.test.ts index 301a86d14bd9..25f9548d1818 100644 --- a/packages/react-router/test/client/hydratedRouter.test.ts +++ b/packages/react-router/test/client/hydratedRouter.test.ts @@ -316,6 +316,43 @@ describe('instrumentHydratedRouter', () => { }); }); + it('parameterizes numeric navigation via subscribe when router state is stale on sync finalize', () => { + mockRouter.navigate = vi.fn().mockImplementation(() => { + (globalThis as any).location.pathname = '/foo'; + return undefined; + }); + + instrumentHydratedRouter(); + mockRouter.navigate(-1); + + expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/foo'); + expect(mockNavigationSpan.updateName).toHaveBeenCalledTimes(1); + expect(mockNavigationSpan.setAttributes).toHaveBeenCalledWith({ + 'url.path': '/foo', + 'url.full': 'https://example.com/foo', + }); + + (core.getActiveSpan as any).mockReturnValue(mockNavigationSpan); + (core.spanToJSON as any).mockImplementation((span: any) => ({ + description: '/foo/bar', + op: span === mockNavigationSpan ? 'navigation' : 'pageload', + data: { 'url.path': '/foo' }, + })); + + const callback = mockRouter.subscribe.mock.calls[0][0]; + callback({ + location: { pathname: '/foo' }, + matches: [{ route: { path: '/foo/:id' } }], + navigation: { state: 'idle' }, + }); + + expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/foo/:id'); + expect(mockNavigationSpan.setAttributes).toHaveBeenLastCalledWith({ + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + 'url.template': '/foo/:id', + }); + }); + it('does not create navigation span for navigate(0)', () => { instrumentHydratedRouter(); mockRouter.navigate(0); diff --git a/packages/react-router/test/client/utils.test.ts b/packages/react-router/test/client/utils.test.ts index 1513d6e5d6c8..858f1a595e59 100644 --- a/packages/react-router/test/client/utils.test.ts +++ b/packages/react-router/test/client/utils.test.ts @@ -1,5 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { resolveNavigateAbsoluteUrl, updateNavigationSpanUrlFromLocation } from '../../src/client/utils'; +import { + finalizeNavigationSpanFromRouterState, + resolveNavigateAbsoluteUrl, + updateNavigationSpanUrlFromLocation, +} from '../../src/client/utils'; vi.mock('@sentry/browser', () => ({ getAbsoluteUrl: vi.fn((urlOrPath: string) => { @@ -118,3 +122,59 @@ describe('updateNavigationSpanUrlFromLocation', () => { }); }); }); + +describe('finalizeNavigationSpanFromRouterState', () => { + const originalLocation = globalThis.location; + + beforeEach(() => { + (globalThis as any).location = { + href: 'https://example.com/performance/', + origin: 'https://example.com', + pathname: '/performance/', + search: '', + hash: '', + }; + }); + + afterEach(() => { + if (originalLocation) { + (globalThis as any).location = originalLocation; + } else { + delete (globalThis as any).location; + } + }); + + it('parameterizes index-route navigations after numeric back', () => { + const span = { updateName: vi.fn(), setAttributes: vi.fn() } as any; + + finalizeNavigationSpanFromRouterState(span, { + location: { pathname: '/performance/' }, + matches: [{ route: { path: '' } }], + navigation: { state: 'idle' }, + } as any); + + expect(span.updateName).toHaveBeenLastCalledWith('/performance'); + expect(span.setAttributes).toHaveBeenLastCalledWith({ + 'sentry.source': 'route', + 'url.template': '/performance', + }); + }); + + it('sets url attributes but skips parameterization when router state is stale', () => { + const span = { updateName: vi.fn(), setAttributes: vi.fn() } as any; + + finalizeNavigationSpanFromRouterState(span, { + location: { pathname: '/performance/ssr' }, + matches: [{ route: { path: '/performance/ssr' } }], + navigation: { state: 'idle' }, + } as any); + + expect(span.updateName).toHaveBeenCalledWith('/performance/'); + expect(span.updateName).toHaveBeenCalledTimes(1); + expect(span.setAttributes).toHaveBeenCalledWith({ + 'url.path': '/performance/', + 'url.full': 'https://example.com/performance/', + }); + expect(span.setAttributes).toHaveBeenCalledTimes(1); + }); +}); From 0a6137729789b3a28c2a425a7590168efae7769e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 9 Jul 2026 19:02:18 +0200 Subject: [PATCH 3/3] format --- .../tests/performance/navigation.client.test.ts | 4 +--- .../tests/performance/navigation.client.test.ts | 4 +--- .../tests/performance/navigation.client.test.ts | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts index 607646ddfd17..347662a9ef9d 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts @@ -201,9 +201,7 @@ test.describe('client - hybrid navigation (instrumentation API span + legacy par await forwardNavPromise; const backNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { - return ( - transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation' - ); + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation'; }); await page.getByRole('button', { name: 'History Back Navigate' }).click(); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts index 7b3ac5633c48..6555db3766c3 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts @@ -212,9 +212,7 @@ test.describe('client - navigation performance', () => { await forwardNavPromise; const backNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { - return ( - transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation' - ); + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation'; }); await page.getByRole('button', { name: 'History Back Navigate' }).click(); diff --git a/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts index 7b3ac5633c48..6555db3766c3 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-8-framework/tests/performance/navigation.client.test.ts @@ -212,9 +212,7 @@ test.describe('client - navigation performance', () => { await forwardNavPromise; const backNavPromise = waitForTransaction(APP_NAME, async transactionEvent => { - return ( - transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation' - ); + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'navigation'; }); await page.getByRole('button', { name: 'History Back Navigate' }).click();