Skip to content

Commit c04ebda

Browse files
authored
fix(react): Preserve parent path prefix for descendant route names in react-router (#22225)
We store the routes in `allRoutes` which is a flat `Set` mixing every mounted `<Routes>` subtree. Because child effects fire before parent effects, the descendant `<SentryRoutes>`'s routes (`:id` **with** its nested `.children` `:sub`) are flattened into `allRoutes`, keeping `.children`. This PR adds `reconstructNameFromDescendantParent` that's basically a band-aid for this specific problem. The architecture of how we save this (in a flat `allRoutes`) is the underlying problem. I will modify this architecture in a follow-up PR as this would go beyond scope of this fix. Closes #22194
1 parent 2cbacb5 commit c04ebda

5 files changed

Lines changed: 476 additions & 0 deletions

File tree

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,41 @@ const ProjectsRoutes = () => (
7474
</SentryRoutes>
7575
);
7676

77+
// Descendant <Routes> whose matched route (`:id` via its index) sits above further non-wildcard nested
78+
// child routes (`:sub`). The nested subtree must not steal the transaction name from the `child/*`
79+
// parent - the pageload/navigation name should stay `/child/:id`, not `/:id/:sub` (see issue #22194).
80+
const ChildRoutes = () => (
81+
<SentryRoutes>
82+
<Route path=":id">
83+
<Route index element={<div id="child">Child</div>} />
84+
<Route path=":sub">
85+
<Route index element={<div id="sub">Sub</div>} />
86+
</Route>
87+
</Route>
88+
</SentryRoutes>
89+
);
90+
91+
// Deep wildcard chain: three levels of `/*` nesting.
92+
// workspace/* → :teamId/* → :memberId
93+
const DeepMemberRoutes = () => (
94+
<SentryRoutes>
95+
<Route path=":memberId" element={<div id="deep-member">Deep Member</div>} />
96+
</SentryRoutes>
97+
);
98+
99+
const DeepTeamRoutes = () => (
100+
<SentryRoutes>
101+
<Route path=":teamId/*" element={<DeepMemberRoutes />} />
102+
</SentryRoutes>
103+
);
104+
77105
const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement);
78106
root.render(
79107
<BrowserRouter>
80108
<SentryRoutes>
81109
<Route path="/" element={<Index />} />
110+
<Route path="child/*" element={<ChildRoutes />} />
111+
<Route path="workspace/*" element={<DeepTeamRoutes />} />
82112
<Route path="/*" element={<ProjectsRoutes />} />
83113
</SentryRoutes>
84114
</BrowserRouter>,

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ const Index = () => {
1010
<Link to="/projects/123/old-views/345/654" id="old-navigation">
1111
navigate old
1212
</Link>
13+
<Link to="/child/abc123" id="child-navigation">
14+
navigate child
15+
</Link>
16+
<Link to="/workspace/team/u123" id="deep-member-navigation">
17+
navigate deep member
18+
</Link>
1319
</>
1420
);
1521
};

‎dev-packages/e2e-tests/test-applications/react-router-6-descendant-routes/tests/transactions.test.ts‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,38 @@ test('sends a pageload transaction with a parameterized URL - alternative route'
6161
});
6262
});
6363

64+
test('keeps the parent path prefix for a descendant route with non-wildcard nested children - pageload', async ({
65+
page,
66+
}) => {
67+
const transactionPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
68+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
69+
});
70+
71+
await page.goto(`/child/abc123`);
72+
73+
const rootSpan = await transactionPromise;
74+
75+
expect((await page.innerHTML('#root')).includes('Child')).toBe(true);
76+
expect(rootSpan).toMatchObject({
77+
contexts: {
78+
trace: {
79+
op: 'pageload',
80+
origin: 'auto.pageload.react.reactrouter_v6',
81+
data: {
82+
'sentry.source': 'route',
83+
'url.template': '/child/:id',
84+
'url.path': '/child/abc123',
85+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/child\/abc123$/),
86+
},
87+
},
88+
},
89+
transaction: '/child/:id',
90+
transaction_info: {
91+
source: 'route',
92+
},
93+
});
94+
});
95+
6496
test('sends a navigation transaction with a parameterized URL', async ({ page }) => {
6597
const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
6698
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
@@ -172,3 +204,128 @@ test('sends a navigation transaction with a parameterized URL - alternative rout
172204
},
173205
});
174206
});
207+
208+
test('keeps the parent path prefix for a descendant route with non-wildcard nested children - navigation', async ({
209+
page,
210+
}) => {
211+
const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
212+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
213+
});
214+
215+
const navigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
216+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
217+
});
218+
219+
await page.goto(`/`);
220+
const pageloadTxn = await pageloadTxnPromise;
221+
222+
expect(pageloadTxn).toMatchObject({
223+
contexts: {
224+
trace: {
225+
op: 'pageload',
226+
origin: 'auto.pageload.react.reactrouter_v6',
227+
data: {
228+
'sentry.source': 'route',
229+
'url.template': '/',
230+
'url.path': '/',
231+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/),
232+
},
233+
},
234+
},
235+
transaction: '/',
236+
transaction_info: {
237+
source: 'route',
238+
},
239+
});
240+
241+
const linkElement = page.locator('id=child-navigation');
242+
243+
const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]);
244+
245+
expect((await page.innerHTML('#root')).includes('Child')).toBe(true);
246+
expect(navigationTxn).toMatchObject({
247+
contexts: {
248+
trace: {
249+
op: 'navigation',
250+
origin: 'auto.navigation.react.reactrouter_v6',
251+
data: {
252+
'sentry.source': 'route',
253+
'url.template': '/child/:id',
254+
'url.path': '/child/abc123',
255+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/child\/abc123$/),
256+
},
257+
},
258+
},
259+
transaction: '/child/:id',
260+
transaction_info: {
261+
source: 'route',
262+
},
263+
});
264+
});
265+
266+
test('resolves deep wildcard chain with three levels of nesting - pageload', async ({ page }) => {
267+
const transactionPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
268+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
269+
});
270+
271+
await page.goto(`/workspace/team/u123`);
272+
273+
const rootSpan = await transactionPromise;
274+
275+
expect((await page.innerHTML('#root')).includes('Deep Member')).toBe(true);
276+
expect(rootSpan).toMatchObject({
277+
contexts: {
278+
trace: {
279+
op: 'pageload',
280+
origin: 'auto.pageload.react.reactrouter_v6',
281+
data: {
282+
'sentry.source': 'route',
283+
'url.template': '/workspace/:teamId/:memberId',
284+
'url.path': '/workspace/team/u123',
285+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/workspace\/team\/u123$/),
286+
},
287+
},
288+
},
289+
transaction: '/workspace/:teamId/:memberId',
290+
transaction_info: {
291+
source: 'route',
292+
},
293+
});
294+
});
295+
296+
test('resolves deep wildcard chain with three levels of nesting - navigation', async ({ page }) => {
297+
const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
298+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
299+
});
300+
301+
const navigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
302+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
303+
});
304+
305+
await page.goto(`/`);
306+
await pageloadTxnPromise;
307+
308+
const linkElement = page.locator('id=deep-member-navigation');
309+
310+
const [_, navigationTxn] = await Promise.all([linkElement.click(), navigationTxnPromise]);
311+
312+
expect((await page.innerHTML('#root')).includes('Deep Member')).toBe(true);
313+
expect(navigationTxn).toMatchObject({
314+
contexts: {
315+
trace: {
316+
op: 'navigation',
317+
origin: 'auto.navigation.react.reactrouter_v6',
318+
data: {
319+
'sentry.source': 'route',
320+
'url.template': '/workspace/:teamId/:memberId',
321+
'url.path': '/workspace/team/u123',
322+
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/workspace\/team\/u123$/),
323+
},
324+
},
325+
},
326+
transaction: '/workspace/:teamId/:memberId',
327+
transaction_info: {
328+
source: 'route',
329+
},
330+
});
331+
});

‎packages/react/src/reactrouter-compat-utils/utils.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,57 @@ export function rebuildRoutePathFromAllRoutes(allRoutes: RouteObject[], location
180180
return '';
181181
}
182182

183+
/**
184+
* Recovers the parent prefix for descendant `<Routes>` names.
185+
*
186+
* `allRoutes` flattens every mounted `<Routes>` into one set, so an orphaned descendant subtree can
187+
* outscore the `.../*` route that anchors the location and drop its prefix (e.g. `/:id/:sub` instead of `/child/:id`).
188+
* Matching only the descendant-parent routes recovers the true anchor.
189+
*/
190+
function reconstructNameFromDescendantParent(
191+
location: Location,
192+
allRoutes: RouteObject[],
193+
currentName: string | undefined,
194+
): string | undefined {
195+
const descendantParents = allRoutes.filter(routeIsDescendant);
196+
if (!descendantParents.length) {
197+
return undefined;
198+
}
199+
200+
const matchedParents = _matchRoutes(descendantParents, location) as RouteMatch[] | null;
201+
const parentMatch = matchedParents?.[matchedParents.length - 1];
202+
if (!parentMatch || !pickSplat(parentMatch)) {
203+
return undefined;
204+
}
205+
206+
const parentTemplate = trimSlash(trimWildcard(parentMatch.route.path || ''));
207+
208+
if (!parentTemplate) {
209+
return undefined;
210+
}
211+
212+
const expectedPrefix = prefixWithSlash(parentTemplate);
213+
214+
// Child `<Routes>` resolve against the matched parent, but flattened route matching can already retain
215+
// a dynamic leading parameter. Adding the parent again would duplicate it (e.g. `/:id/:id`)
216+
const firstElement = parentTemplate.split('/')[0];
217+
const hasDynamicLead = firstElement?.startsWith(':') && currentName?.split('/').includes(firstElement);
218+
219+
// Rebuild only when matching the descendant subtree discarded its parent route template
220+
if (currentName === expectedPrefix || currentName?.startsWith(`${expectedPrefix}/`) || hasDynamicLead) {
221+
return undefined;
222+
}
223+
224+
const remainingPathname =
225+
stripBasenameFromPathname(location.pathname, prefixWithSlash(parentMatch.pathnameBase)) || '/';
226+
const remainingName = rebuildRoutePathFromAllRoutes(
227+
allRoutes.filter(route => route !== parentMatch.route),
228+
{ pathname: remainingPathname },
229+
);
230+
231+
return remainingName ? prefixWithSlash(`${parentTemplate}${prefixWithSlash(remainingName)}`) : undefined;
232+
}
233+
183234
/**
184235
* Checks if the current location is inside a descendant route (route with splat parameter).
185236
*/
@@ -303,6 +354,13 @@ export function resolveRouteNameAndSource(
303354
[name, source] = getNormalizedName(routes, location, branches, basename);
304355
}
305356

357+
// Guard against orphaned descendant subtrees stealing the transaction name: if the location is
358+
// anchored by a descendant-parent route (`.../*`) whose prefix was dropped, reconstruct with it.
359+
const anchoredName = reconstructNameFromDescendantParent(location, allRoutes, name);
360+
if (anchoredName) {
361+
return [anchoredName, 'route'];
362+
}
363+
306364
return [name || location.pathname, source];
307365
}
308366

0 commit comments

Comments
 (0)