Address reviews further

Author: onurtemizkan

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

@@ -85,7 +85,7 @@ test('Creates a navigation transaction inside a lazy route', async ({ page }) =>
 });
 
 test('Creates navigation transactions between two different lazy routes', async ({ page }) => {
-  // First, navigate to the "another-lazy" route
+  // Set up transaction listeners for both navigations
   const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
     return (
       !!transactionEvent?.transaction &&
@@ -94,6 +94,14 @@ test('Creates navigation transactions between two different lazy routes', async
     );
   });
 
+  const secondTransactionPromise = 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.goto('/');
 
   // Navigate to another lazy route first
@@ -113,14 +121,6 @@ test('Creates navigation transactions between two different lazy routes', async
   expect(firstEvent.contexts?.trace?.op).toBe('navigation');
 
   // Now navigate from the first lazy route to the second lazy route
-  const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
-    return (
-      !!transactionEvent?.transaction &&
-      transactionEvent.contexts?.trace?.op === 'navigation' &&
-      transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
-    );
-  });
-
   // Click the navigation link from within the first lazy route to the second lazy route
   const navigationToInnerFromDeep = page.locator('id=navigate-to-inner-from-deep');
   await expect(navigationToInnerFromDeep).toBeVisible();
@@ -253,7 +253,7 @@ test('Does not send any duplicate navigation transaction names browsing between
 
   // Go to root page
   await page.goto('/');
-  page.waitForTimeout(1000);
+  await page.waitForTimeout(1000);
 
   // Navigate to inner lazy route
   const navigationToInner = page.locator('id=navigation');
@@ -337,6 +337,7 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
   const navigationToLongRunning = page.locator('id=navigation-to-long-running');
   await expect(navigationToLongRunning).toBeVisible();
 
+  // Set up transaction listeners for both navigations
   const firstNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
     return (
       !!transactionEvent?.transaction &&
@@ -345,6 +346,14 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
     );
   });
 
+  const backNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
+    return (
+      !!transactionEvent?.transaction &&
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.transaction === '/'
+    );
+  });
+
   await navigationToLongRunning.click();
 
   const slowLoadingContent = page.locator('id=slow-loading-content');
@@ -357,14 +366,6 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
 
   // Now navigate back using browser back button (POP event)
   // This should create a navigation transaction since pageload is complete
-  const backNavigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
-    return (
-      !!transactionEvent?.transaction &&
-      transactionEvent.contexts?.trace?.op === 'navigation' &&
-      transactionEvent.transaction === '/'
-    );
-  });
-
   await page.goBack();
 
   // Verify we're back at home

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

@@ -54,7 +54,7 @@ const CLIENTS_WITH_INSTRUMENT_NAVIGATION = new WeakSet<Client>();
 
 /**
  * Tracks last navigation per client to prevent duplicate spans in cross-usage scenarios.
- * Entry persists until next different navigation, handling delayed wrapper execution.
+ * Entry persists until the navigation span ends, allowing cross-usage detection during delayed wrapper execution.
  */
 const LAST_NAVIGATION_PER_CLIENT = new WeakMap<Client, string>();
 
@@ -628,7 +628,9 @@ function tryUpdateSpanName(
   newName: string,
   newSource: string,
 ): void {
-  const isNewNameParameterized = newName !== currentSpanName && newName.includes(':');
+  // Check if the new name contains React Router parameter syntax (/:param/)
+  const isReactRouterParam = /\/:[a-zA-Z0-9_]+/.test(newName);
+  const isNewNameParameterized = newName !== currentSpanName && isReactRouterParam;
   if (isNewNameParameterized) {
     activeSpan.updateName(newName);
     activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, newSource as 'route' | 'url' | 'custom');