Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
34 changes: 20 additions & 14 deletions packages/react/src/reactrouter-compat-utils/instrumentation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
}
}
});
Expand Down Expand Up @@ -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)) {
Expand All @@ -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,
Expand All @@ -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: {
Expand Down Expand Up @@ -727,7 +727,13 @@ 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 || '/');

Expand Down Expand Up @@ -780,7 +786,7 @@ function patchSpanEnd(
if (branches) {
const [name, source] = resolveRouteNameAndSource(
location,
routes,
currentAllRoutes.length > 0 ? currentAllRoutes : routes,
currentAllRoutes.length > 0 ? currentAllRoutes : routes,
branches,
basename,
Expand Down
Loading
Loading