Skip to content
Open
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 @@ -74,11 +74,41 @@ const ProjectsRoutes = () => (
</SentryRoutes>
);

// Descendant <Routes> whose matched route (`:id` via its index) sits above further non-wildcard nested
// child routes (`:sub`). The nested subtree must not steal the transaction name from the `child/*`
// parent - the pageload/navigation name should stay `/child/:id`, not `/:id/:sub` (see issue #22194).
const ChildRoutes = () => (
<SentryRoutes>
<Route path=":id">
<Route index element={<div id="child">Child</div>} />
<Route path=":sub">
<Route index element={<div id="sub">Sub</div>} />
</Route>
</Route>
</SentryRoutes>
);

// Deep wildcard chain: three levels of `/*` nesting.
// workspace/* → :teamId/* → :memberId
const DeepMemberRoutes = () => (
<SentryRoutes>
<Route path=":memberId" element={<div id="deep-member">Deep Member</div>} />
</SentryRoutes>
);

const DeepTeamRoutes = () => (
<SentryRoutes>
<Route path=":teamId/*" element={<DeepMemberRoutes />} />
</SentryRoutes>
);

const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement);
root.render(
<BrowserRouter>
<SentryRoutes>
<Route path="/" element={<Index />} />
<Route path="child/*" element={<ChildRoutes />} />
<Route path="workspace/*" element={<DeepTeamRoutes />} />
<Route path="/*" element={<ProjectsRoutes />} />
</SentryRoutes>
</BrowserRouter>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ const Index = () => {
<Link to="/projects/123/old-views/345/654" id="old-navigation">
navigate old
</Link>
<Link to="/child/abc123" id="child-navigation">
navigate child
</Link>
<Link to="/workspace/team/u123" id="deep-member-navigation">
navigate deep member
</Link>
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,38 @@ test('sends a pageload transaction with a parameterized URL - alternative route'
});
});

test('keeps the parent path prefix for a descendant route with non-wildcard nested children - pageload', async ({
page,
}) => {
const transactionPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
});

await page.goto(`/child/abc123`);

const rootSpan = await transactionPromise;

expect((await page.innerHTML('#root')).includes('Child')).toBe(true);
expect(rootSpan).toMatchObject({
contexts: {
trace: {
op: 'pageload',
origin: 'auto.pageload.react.reactrouter_v6',
data: {
'sentry.source': 'route',
'url.template': '/child/:id',
'url.path': '/child/abc123',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/child\/abc123$/),
},
},
},
transaction: '/child/:id',
transaction_info: {
source: 'route',
},
});
});

test('sends a navigation transaction with a parameterized URL', async ({ page }) => {
const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
Expand Down Expand Up @@ -172,3 +204,128 @@ test('sends a navigation transaction with a parameterized URL - alternative rout
},
});
});

test('keeps the parent path prefix for a descendant route with non-wildcard nested children - navigation', async ({
page,
}) => {
const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
});

const navigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
});

await page.goto(`/`);
const pageloadTxn = await pageloadTxnPromise;

expect(pageloadTxn).toMatchObject({
contexts: {
trace: {
op: 'pageload',
origin: 'auto.pageload.react.reactrouter_v6',
data: {
'sentry.source': 'route',
'url.template': '/',
'url.path': '/',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/),
},
},
},
transaction: '/',
transaction_info: {
source: 'route',
},
});

const linkElement = page.locator('id=child-navigation');

const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]);

expect((await page.innerHTML('#root')).includes('Child')).toBe(true);
expect(navigationTxn).toMatchObject({
contexts: {
trace: {
op: 'navigation',
origin: 'auto.navigation.react.reactrouter_v6',
data: {
'sentry.source': 'route',
'url.template': '/child/:id',
'url.path': '/child/abc123',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/child\/abc123$/),
},
},
},
transaction: '/child/:id',
transaction_info: {
source: 'route',
},
});
});

test('resolves deep wildcard chain with three levels of nesting - pageload', async ({ page }) => {
const transactionPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
});

await page.goto(`/workspace/team/u123`);

const rootSpan = await transactionPromise;

expect((await page.innerHTML('#root')).includes('Deep Member')).toBe(true);
expect(rootSpan).toMatchObject({
contexts: {
trace: {
op: 'pageload',
origin: 'auto.pageload.react.reactrouter_v6',
data: {
'sentry.source': 'route',
'url.template': '/workspace/:teamId/:memberId',
'url.path': '/workspace/team/u123',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/workspace\/team\/u123$/),
},
},
},
transaction: '/workspace/:teamId/:memberId',
transaction_info: {
source: 'route',
},
});
});

test('resolves deep wildcard chain with three levels of nesting - navigation', async ({ page }) => {
const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
});

const navigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
});

await page.goto(`/`);
await pageloadTxnPromise;

const linkElement = page.locator('id=deep-member-navigation');

const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]);

expect((await page.innerHTML('#root')).includes('Deep Member')).toBe(true);
expect(navigationTxn).toMatchObject({
contexts: {
trace: {
op: 'navigation',
origin: 'auto.navigation.react.reactrouter_v6',
data: {
'sentry.source': 'route',
'url.template': '/workspace/:teamId/:memberId',
'url.path': '/workspace/team/u123',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/workspace\/team\/u123$/),
},
},
},
transaction: '/workspace/:teamId/:memberId',
transaction_info: {
source: 'route',
},
});
});
53 changes: 53 additions & 0 deletions packages/react/src/reactrouter-compat-utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,52 @@ export function rebuildRoutePathFromAllRoutes(allRoutes: RouteObject[], location
return '';
}

/**
* Recovers the parent prefix for descendant `<Routes>` names.
*
* `allRoutes` flattens every mounted `<Routes>` into one set, so an orphaned descendant subtree can
* outscore the `.../*` route that anchors the location and drop its prefix (e.g. `/:id/:sub` instead of `/child/:id`).
* Matching only the descendant-parent routes recovers the true anchor.
*/
function reconstructNameFromDescendantParent(
location: Location,
allRoutes: RouteObject[],
currentName: string | undefined,
): string | undefined {
const descendantParents = allRoutes.filter(routeIsDescendant);
if (!descendantParents.length) {
return undefined;
}

const matchedParents = _matchRoutes(descendantParents, location) as RouteMatch[] | null;
const parentMatch = matchedParents?.[matchedParents.length - 1];
if (!parentMatch || !pickSplat(parentMatch)) {
return undefined;
}

const parentTemplate = trimSlash(trimWildcard(parentMatch.route.path || ''));

// Only a static leading segment (e.g. `child/*`) has a fixed position in the absolute URL; dynamic
// leads (`:projectId/*`) are relative and stay with the existing wildcard-rebuild path.
if (!parentTemplate || parentTemplate.startsWith(':')) {
return undefined;
}

const expectedPrefix = prefixWithSlash(parentTemplate);
if (currentName === expectedPrefix || currentName?.startsWith(`${expectedPrefix}/`)) {
return undefined;
}

const remainingPathname =
stripBasenameFromPathname(location.pathname, prefixWithSlash(parentMatch.pathnameBase)) || '/';
const remainingName = rebuildRoutePathFromAllRoutes(
allRoutes.filter(route => route !== parentMatch.route),
{ pathname: remainingPathname },
);

return remainingName ? prefixWithSlash(`${parentTemplate}${prefixWithSlash(remainingName)}`) : undefined;
}

/**
Comment thread
s1gr1d marked this conversation as resolved.
* Checks if the current location is inside a descendant route (route with splat parameter).
*/
Expand Down Expand Up @@ -303,6 +349,13 @@ export function resolveRouteNameAndSource(
[name, source] = getNormalizedName(routes, location, branches, basename);
}

// Guard against orphaned descendant subtrees stealing the transaction name: if the location is
// anchored by a descendant-parent route (`.../*`) whose prefix was dropped, reconstruct with it.
const anchoredName = reconstructNameFromDescendantParent(location, allRoutes, name);
if (anchoredName) {
return [anchoredName, 'route'];
}

return [name || location.pathname, source];
}

Expand Down
Loading
Loading