Skip to content

Commit e9cab06

Browse files
committed
Edge case fixes
1 parent 0cfe264 commit e9cab06

File tree

2 files changed

+25
-11
lines changed

2 files changed

+25
-11
lines changed

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -706,15 +706,22 @@ export function handleNavigation(opts: {
706706
basename,
707707
);
708708

709-
const activeSpan = getActiveSpan();
710-
const spanJson = activeSpan && spanToJSON(activeSpan);
711-
const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';
712-
const isSpanForSameRoute = isAlreadyInNavigationSpan && spanJson?.description === name;
713-
714709
const currentNavigationKey = getNavigationKey(location);
715710
const isNavDuplicate = isDuplicateNavigation(client, currentNavigationKey);
716711

717-
if (!isSpanForSameRoute && !isNavDuplicate) {
712+
if (isNavDuplicate) {
713+
// Cross-usage duplicate - update existing span name if better
714+
const activeSpan = getActiveSpan();
715+
const spanJson = activeSpan && spanToJSON(activeSpan);
716+
const isAlreadyInNavigationSpan = spanJson?.op === 'navigation';
717+
718+
if (isAlreadyInNavigationSpan && activeSpan) {
719+
tryUpdateSpanName(activeSpan, spanJson?.description, name, source);
720+
}
721+
} else {
722+
// Not a cross-usage duplicate - create new span
723+
// This handles: different routes, same route with different params (/user/2 → /user/3)
724+
// startBrowserTracingNavigationSpan will end any active navigation span
718725
createNavigationSpan({
719726
client,
720727
name,
@@ -726,8 +733,6 @@ export function handleNavigation(opts: {
726733
allRoutes,
727734
navigationKey: currentNavigationKey,
728735
});
729-
} else if (isNavDuplicate && isAlreadyInNavigationSpan && activeSpan) {
730-
tryUpdateSpanName(activeSpan, spanJson?.description, name, source);
731736
}
732737
}
733738
}

packages/react/test/reactrouter-cross-usage.test.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ describe('React Router cross usage of wrappers', () => {
707707
expect(calls[1]![1].name).toBe('/c');
708708
});
709709

710-
it('should NOT create duplicate spans for same route name (even with different params)', async () => {
710+
it('should create separate spans for same route with different params', async () => {
711711
const client = createMockBrowserClient();
712712
setCurrentClient(client);
713713

@@ -755,10 +755,19 @@ describe('React Router cross usage of wrappers', () => {
755755

756756
await act(async () => {
757757
router.navigate('/user/3');
758-
await new Promise(resolve => setTimeout(resolve, 100));
758+
await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2));
759759
});
760760

761-
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
761+
// Should create 2 spans - different concrete paths are different user actions
762+
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2);
763+
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenNthCalledWith(2, expect.any(BrowserClient), {
764+
name: '/user/:id',
765+
attributes: {
766+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
767+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
768+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
769+
},
770+
});
762771
});
763772

764773
it('should handle mixed cross-usage and consecutive navigations correctly', async () => {

0 commit comments

Comments
 (0)