Skip to content

Commit f8450d2

Browse files
onurtemizkanonurtemizkan
committed
Apply multiple navigations fix
1 parent 156600c commit f8450d2

File tree

6 files changed

+210
-9
lines changed

6 files changed

+210
-9
lines changed

‎dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ const router = sentryCreateBrowserRouter(
4949
lazyChildren: () => import('./pages/InnerLazyRoutes').then(module => module.someMoreNestedRoutes),
5050
},
5151
},
52+
{
53+
path: '/another-lazy',
54+
handle: {
55+
lazyChildren: () => import('./pages/AnotherLazyRoutes').then(module => module.anotherNestedRoutes),
56+
},
57+
},
5258
{
5359
path: '/static',
5460
element: <>Hello World</>,

‎dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/AnotherLazyRoutes.tsx

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { Link } from 'react-router-dom';
2+
3+
export const anotherNestedRoutes = [
4+
{
5+
path: 'sub',
6+
children: [
7+
{
8+
index: true,
9+
element: (
10+
<div id="another-lazy-route">
11+
Another Lazy Route
12+
<Link to="/lazy/inner/999/888/777" id="navigate-to-inner">
13+
Navigate to Inner Lazy Route
14+
</Link>
15+
</div>
16+
),
17+
},
18+
{
19+
path: ':id',
20+
children: [
21+
{
22+
index: true,
23+
element: <div id="another-lazy-route-with-id">Another Lazy Route with ID</div>,
24+
},
25+
{
26+
path: ':subId',
27+
element: (
28+
<div id="another-lazy-route-deep">
29+
Another Deep Lazy Route
30+
<Link to="/lazy/inner/111/222/333" id="navigate-to-inner-from-deep">
31+
Navigate to Inner from Deep
32+
</Link>
33+
</div>
34+
),
35+
},
36+
],
37+
},
38+
],
39+
},
40+
];
41+
42+
export default null as unknown as void;

‎dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ const Index = () => {
66
<Link to="/lazy/inner/123/456/789" id="navigation">
77
navigate
88
</Link>
9+
<br />
10+
<Link to="/another-lazy/sub" id="navigation-to-another">
11+
Navigate to Another Lazy Route
12+
</Link>
13+
<br />
14+
<Link to="/another-lazy/sub/555/666" id="navigation-to-another-deep">
15+
Navigate to Another Deep Lazy Route
16+
</Link>
917
</>
1018
);
1119
};

‎dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { Link } from 'react-router-dom';
2+
13
export const someMoreNestedRoutes = [
24
{
35
path: 'inner',
@@ -22,7 +24,15 @@ export const someMoreNestedRoutes = [
2224
},
2325
{
2426
path: ':someAnotherId',
25-
element: <div id="innermost-lazy-route">Rendered</div>,
27+
element: (
28+
<div id="innermost-lazy-route">
29+
Rendered
30+
<br />
31+
<Link to="/another-lazy/sub/888/999" id="navigate-to-another-from-inner">
32+
Navigate to Another Lazy Route
33+
</Link>
34+
</div>
35+
),
2636
},
2737
],
2838
},

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,111 @@ test('Creates a navigation transaction inside a lazy route', async ({ page }) =>
5252
expect(event.type).toBe('transaction');
5353
expect(event.contexts?.trace?.op).toBe('navigation');
5454
});
55+
56+
test('Creates navigation transactions between two different lazy routes', async ({ page }) => {
57+
// First, navigate to the "another-lazy" route
58+
const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
59+
return (
60+
!!transactionEvent?.transaction &&
61+
transactionEvent.contexts?.trace?.op === 'navigation' &&
62+
transactionEvent.transaction === '/another-lazy/sub/:id/:subId'
63+
);
64+
});
65+
66+
await page.goto('/');
67+
68+
// Navigate to another lazy route first
69+
const navigationToAnotherDeep = page.locator('id=navigation-to-another-deep');
70+
await expect(navigationToAnotherDeep).toBeVisible();
71+
await navigationToAnotherDeep.click();
72+
73+
const firstEvent = await firstTransactionPromise;
74+
75+
// Check if the first lazy route content is rendered
76+
const anotherLazyContent = page.locator('id=another-lazy-route-deep');
77+
await expect(anotherLazyContent).toBeVisible();
78+
79+
// Validate the first transaction event
80+
expect(firstEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
81+
expect(firstEvent.type).toBe('transaction');
82+
expect(firstEvent.contexts?.trace?.op).toBe('navigation');
83+
84+
// Now navigate from the first lazy route to the second lazy route
85+
const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
86+
return (
87+
!!transactionEvent?.transaction &&
88+
transactionEvent.contexts?.trace?.op === 'navigation' &&
89+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
90+
);
91+
});
92+
93+
// Click the navigation link from within the first lazy route to the second lazy route
94+
const navigationToInnerFromDeep = page.locator('id=navigate-to-inner-from-deep');
95+
await expect(navigationToInnerFromDeep).toBeVisible();
96+
await navigationToInnerFromDeep.click();
97+
98+
const secondEvent = await secondTransactionPromise;
99+
100+
// Check if the second lazy route content is rendered
101+
const innerLazyContent = page.locator('id=innermost-lazy-route');
102+
await expect(innerLazyContent).toBeVisible();
103+
104+
// Validate the second transaction event
105+
expect(secondEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
106+
expect(secondEvent.type).toBe('transaction');
107+
expect(secondEvent.contexts?.trace?.op).toBe('navigation');
108+
});
109+
110+
test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => {
111+
// First, navigate to the inner lazy route
112+
const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
113+
return (
114+
!!transactionEvent?.transaction &&
115+
transactionEvent.contexts?.trace?.op === 'navigation' &&
116+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
117+
);
118+
});
119+
120+
await page.goto('/');
121+
122+
// Navigate to inner lazy route first
123+
const navigationToInner = page.locator('id=navigation');
124+
await expect(navigationToInner).toBeVisible();
125+
await navigationToInner.click();
126+
127+
const firstEvent = await firstTransactionPromise;
128+
129+
// Check if the inner lazy route content is rendered
130+
const innerLazyContent = page.locator('id=innermost-lazy-route');
131+
await expect(innerLazyContent).toBeVisible();
132+
133+
// Validate the first transaction event
134+
expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
135+
expect(firstEvent.type).toBe('transaction');
136+
expect(firstEvent.contexts?.trace?.op).toBe('navigation');
137+
138+
// Now navigate from the inner lazy route to another lazy route
139+
const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
140+
return (
141+
!!transactionEvent?.transaction &&
142+
transactionEvent.contexts?.trace?.op === 'navigation' &&
143+
transactionEvent.transaction === '/another-lazy/sub/:id/:subId'
144+
);
145+
});
146+
147+
// Click the navigation link from within the inner lazy route to another lazy route
148+
const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner');
149+
await expect(navigationToAnotherFromInner).toBeVisible();
150+
await navigationToAnotherFromInner.click();
151+
152+
const secondEvent = await secondTransactionPromise;
153+
154+
// Check if the another lazy route content is rendered
155+
const anotherLazyContent = page.locator('id=another-lazy-route-deep');
156+
await expect(anotherLazyContent).toBeVisible();
157+
158+
// Validate the second transaction event
159+
expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId');
160+
expect(secondEvent.type).toBe('transaction');
161+
expect(secondEvent.contexts?.trace?.op).toBe('navigation');
162+
});

‎packages/react/src/reactrouterv6-compat-utils.tsx

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -529,14 +529,41 @@ export function handleNavigation(opts: {
529529
[name, source] = getNormalizedName(routes, location, branches, basename);
530530
}
531531

532-
startBrowserTracingNavigationSpan(client, {
533-
name,
534-
attributes: {
535-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
536-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
537-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
538-
},
539-
});
532+
// Avoid updating navigation span name multiple times when cross-usage occurs
533+
const activeSpan = getActiveSpan();
534+
const spanJson = activeSpan ? spanToJSON(activeSpan) : undefined;
535+
const isAlreadyInNavigationSpan = spanJson && spanJson.op === 'navigation';
536+
537+
if (isAlreadyInNavigationSpan && activeSpan) {
538+
// Only update the span name once. Use a non-enumerable property to mark
539+
// that we've already set the navigation name to avoid repeated updates.
540+
const hasBeenNamed = (
541+
activeSpan as {
542+
__sentry_navigation_name_set__?: boolean;
543+
}
544+
).__sentry_navigation_name_set__;
545+
546+
if (!hasBeenNamed) {
547+
// If the span hasn't finished (no timestamp), update the name.
548+
if (!spanJson.timestamp) {
549+
activeSpan.updateName(name);
550+
}
551+
// Mark as named so subsequent navigations won't rename it.
552+
addNonEnumerableProperty(activeSpan as any, '__sentry_navigation_name_set__', true);
553+
}
554+
555+
// Always keep the source attribute up to date.
556+
activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
557+
} else {
558+
startBrowserTracingNavigationSpan(client, {
559+
name,
560+
attributes: {
561+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
562+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
563+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
564+
},
565+
});
566+
}
540567
}
541568
}
542569

0 commit comments

Comments
 (0)