fix(react): Prevent navigation span leaks for consecutive navigations

Author: onurtemizkan

dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts

@@ -498,3 +498,87 @@ test('Updates navigation transaction name correctly when span is cancelled early
     expect(['externalFinish', 'cancelled']).toContain(idleSpanFinishReason);
   }
 });
+
+test('Creates separate transactions for rapid consecutive navigations', async ({ page }) => {
+  await page.goto('/');
+
+  // First navigation: / -> /lazy/inner/:id/:anotherId/:someAnotherId
+  const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    return (
+      !!transactionEvent?.transaction &&
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
+    );
+  });
+
+  const navigationToInner = page.locator('id=navigation');
+  await expect(navigationToInner).toBeVisible();
+  await navigationToInner.click();
+
+  const firstEvent = await firstTransactionPromise;
+
+  // Verify first transaction
+  expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
+  expect(firstEvent.contexts?.trace?.op).toBe('navigation');
+  const firstTraceId = firstEvent.contexts?.trace?.trace_id;
+  const firstSpanId = firstEvent.contexts?.trace?.span_id;
+
+  // Second navigation: /lazy/inner -> /another-lazy/sub/:id/:subId
+  const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    return (
+      !!transactionEvent?.transaction &&
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.transaction === '/another-lazy/sub/:id/:subId'
+    );
+  });
+
+  const navigationToAnother = page.locator('id=navigate-to-another-from-inner');
+  await expect(navigationToAnother).toBeVisible();
+  await navigationToAnother.click();
+
+  const secondEvent = await secondTransactionPromise;
+
+  // Verify second transaction
+  expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
+  expect(secondEvent.contexts?.trace?.op).toBe('navigation');
+  const secondTraceId = secondEvent.contexts?.trace?.trace_id;
+  const secondSpanId = secondEvent.contexts?.trace?.span_id;
+
+  // Third navigation: /another-lazy -> /lazy/inner/:id/:anotherId/:someAnotherId (back to same route as first)
+  const thirdTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    return (
+      !!transactionEvent?.transaction &&
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' &&
+      // Ensure we're not matching the first transaction again
+      transactionEvent.contexts?.trace?.trace_id !== firstTraceId
+    );
+  });
+
+  const navigationBackToInner = page.locator('id=navigate-to-inner-from-deep');
+  await expect(navigationBackToInner).toBeVisible();
+  await navigationBackToInner.click();
+
+  const thirdEvent = await thirdTransactionPromise;
+
+  // Verify third transaction
+  expect(thirdEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
+  expect(thirdEvent.contexts?.trace?.op).toBe('navigation');
+  const thirdTraceId = thirdEvent.contexts?.trace?.trace_id;
+  const thirdSpanId = thirdEvent.contexts?.trace?.span_id;
+
+  // Verify each navigation created a separate transaction with unique trace and span IDs
+  expect(firstTraceId).toBeDefined();
+  expect(secondTraceId).toBeDefined();
+  expect(thirdTraceId).toBeDefined();
+
+  // All trace IDs should be unique
+  expect(firstTraceId).not.toBe(secondTraceId);
+  expect(secondTraceId).not.toBe(thirdTraceId);
+  expect(firstTraceId).not.toBe(thirdTraceId);
+
+  // All span IDs should be unique
+  expect(firstSpanId).not.toBe(secondSpanId);
+  expect(secondSpanId).not.toBe(thirdSpanId);
+  expect(firstSpanId).not.toBe(thirdSpanId);
+});

packages/react/src/reactrouter-compat-utils/instrumentation.tsx

@@ -279,7 +279,10 @@ export function createV6CompatibleWrapCreateBrowserRouter<
         state.historyAction === 'PUSH' || (state.historyAction === 'POP' && isInitialPageloadComplete);
 
       if (shouldHandleNavigation) {
-        const navigationHandler = (): void => {
+        // Only handle navigation when it's complete (state is idle).
+        // During 'loading' or 'submitting', state.location may still have the old pathname,
+        // which would cause us to create a span for the wrong route.
+        if (state.navigation.state === 'idle') {
           handleNavigation({
             location: state.location,
             routes,
@@ -288,13 +291,6 @@ export function createV6CompatibleWrapCreateBrowserRouter<
             basename,
             allRoutes: Array.from(allRoutes),
           });
-        };
-
-        // Wait for the next render if loading an unsettled route
-        if (state.navigation.state !== 'idle') {
-          requestAnimationFrame(navigationHandler);
-        } else {
-          navigationHandler();
         }
       }
     });
@@ -632,7 +628,8 @@ export function handleNavigation(opts: {
   allRoutes?: RouteObject[];
 }): void {
   const { location, routes, navigationType, version, matches, basename, allRoutes } = opts;
-  const branches = Array.isArray(matches) ? matches : _matchRoutes(routes, location, basename);
+  // Use allRoutes for matching to include lazy-loaded routes
+  const branches = Array.isArray(matches) ? matches : _matchRoutes(allRoutes || routes, location, basename);
 
   const client = getClient();
   if (!client || !CLIENTS_WITH_INSTRUMENT_NAVIGATION.has(client)) {
@@ -649,7 +646,7 @@ export function handleNavigation(opts: {
   if ((navigationType === 'PUSH' || navigationType === 'POP') && branches) {
     const [name, source] = resolveRouteNameAndSource(
       location,
-      routes,
+      allRoutes || routes,
       allRoutes || routes,
       branches as RouteMatch[],
       basename,
@@ -659,8 +656,11 @@ export function handleNavigation(opts: {
     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) {
+    // Only skip creating a new span if we're already in a navigation span AND the route name matches.
+    // This handles cross-usage (multiple wrappers for same navigation) while allowing consecutive navigations.
+    const isSpanForSameRoute = isAlreadyInNavigationSpan && spanJson?.description === name;
+
+    if (!isSpanForSameRoute) {
       const navigationSpan = startBrowserTracingNavigationSpan(client, {
         name,
         attributes: {
@@ -727,7 +727,7 @@ function updatePageloadTransaction({
     : (_matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]);
 
   if (branches) {
-    const [name, source] = resolveRouteNameAndSource(location, routes, allRoutes || routes, branches, basename);
+    const [name, source] = resolveRouteNameAndSource(location, allRoutes || routes, allRoutes || routes, branches, basename);
 
     getCurrentScope().setTransactionName(name || '/');
 
@@ -780,7 +780,7 @@ function patchSpanEnd(
         if (branches) {
           const [name, source] = resolveRouteNameAndSource(
             location,
-            routes,
+            currentAllRoutes.length > 0 ? currentAllRoutes : routes,
             currentAllRoutes.length > 0 ? currentAllRoutes : routes,
             branches,
             basename,