fix(react): Remove handleExistingNavigation

Author: onurtemizkan

packages/react/src/reactrouter-compat-utils/index.ts

@@ -9,7 +9,6 @@ export {
   createV6CompatibleWrapCreateMemoryRouter,
   createV6CompatibleWrapUseRoutes,
   handleNavigation,
-  handleExistingNavigationSpan,
   createNewNavigationSpan,
   addResolvedRoutesToParent,
   processResolvedRoutes,

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

@@ -176,12 +176,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
@@ -199,14 +194,14 @@ export function updateNavigationSpan(
     if (name && !spanJson.timestamp) {
       activeRootSpan.updateName(name);
       activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
-
-      // Mark this span as having its name set to prevent future updates
-      addNonEnumerableProperty(
-        activeRootSpan as { __sentry_navigation_name_set__?: boolean },
-        '__sentry_navigation_name_set__',
-        true,
-      );
     }
+
+    // Mark this span as having its name set to prevent future updates
+    addNonEnumerableProperty(
+      activeRootSpan as { __sentry_navigation_name_set__?: boolean },
+      '__sentry_navigation_name_set__',
+      true,
+    );
   }
 }
 
@@ -355,13 +350,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) => {
@@ -389,11 +378,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,
@@ -532,13 +517,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,
@@ -559,13 +538,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,
@@ -612,9 +585,7 @@ export function handleNavigation(opts: {
     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 {
+    if (!isAlreadyInNavigationSpan) {
       createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
     }
   }
@@ -726,13 +697,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
@@ -766,43 +731,6 @@ function getActiveRootSpan(): Span | undefined {
   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
  */

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

@@ -10,7 +10,6 @@ import {
   addResolvedRoutesToParent,
   createNewNavigationSpan,
   createReactRouterV6CompatibleTracingIntegration,
-  handleExistingNavigationSpan,
   updateNavigationSpan,
 } from '../../src/reactrouter-compat-utils';
 import type { Location, RouteObject } from '../../src/types';
@@ -93,29 +92,6 @@ describe('reactrouter-compat-utils/instrumentation', () => {
       expect(mockUpdateName).not.toHaveBeenCalled();
     });
   });
-
-  describe('handleExistingNavigationSpan', () => {
-    it('should update span name when not already named', () => {
-      const spanJson = { op: 'navigation', data: {}, span_id: 'test', start_timestamp: 0, trace_id: 'test' };
-
-      handleExistingNavigationSpan(mockSpan, spanJson, 'Test Route', 'route', false);
-
-      expect(mockUpdateName).toHaveBeenCalledWith('Test Route');
-      expect(mockSetAttribute).toHaveBeenCalledWith('sentry.source', 'route');
-      expect(addNonEnumerableProperty).toHaveBeenCalledWith(mockSpan, '__sentry_navigation_name_set__', true);
-    });
-
-    it('should not mark as named for lazy routes', () => {
-      const spanJson = { op: 'navigation', data: {}, span_id: 'test', start_timestamp: 0, trace_id: 'test' };
-
-      handleExistingNavigationSpan(mockSpan, spanJson, 'Test Route', 'route', true);
-
-      expect(mockUpdateName).toHaveBeenCalledWith('Test Route');
-      expect(mockSetAttribute).toHaveBeenCalledWith('sentry.source', 'route');
-      expect(addNonEnumerableProperty).not.toHaveBeenCalled();
-    });
-  });
-
   describe('createNewNavigationSpan', () => {
     it('should create new navigation span with correct attributes', () => {
       createNewNavigationSpan(mockClient, 'Test Route', 'route', '6', false);