Skip to content

Commit 8771f29

Browse files
committed
fix(react): Remove routes from shared set on <Routes> unmount
The module-level `allRoutes` set accumulated every route ever mounted and never removed any, so once two independent routers had each been mounted, `matchRoutes` ran over the union of both and could name a transaction with one router's static segment and another's param (e.g. `/bar/:fooId`). Removing a `<Routes>`'s routes when it unmounts keeps the set to what is currently mounted, so stale routes from an unrelated router can't be matched against a later navigation. Fixes #22782
1 parent 4f47000 commit 8771f29

1 file changed

Lines changed: 28 additions & 3 deletions

File tree

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,7 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio
755755
locationArg?: Partial<Location> | string;
756756
}> = (props: { children?: React.ReactNode; routes: RouteObject[]; locationArg?: Partial<Location> | string }) => {
757757
const isMountRenderPass = React.useRef(true);
758+
const addedRoutes = React.useRef<RouteObject[]>([]);
758759
const { routes, locationArg } = props;
759760

760761
const Routes = origUseRoutes(routes, locationArg);
@@ -771,7 +772,7 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio
771772
typeof stableLocationParam === 'string' ? { pathname: stableLocationParam } : stableLocationParam;
772773

773774
if (isMountRenderPass.current) {
774-
addRoutesToAllRoutes(routes);
775+
addedRoutes.current = addRoutesToAllRoutes(routes);
775776

776777
updatePageloadTransaction({
777778
activeRootSpan: getActiveRootSpan(),
@@ -794,6 +795,10 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio
794795
}
795796
}, [navigationType, stableLocationParam]);
796797

798+
// Drop this `<Routes>`'s routes from the shared set when it unmounts, so they don't leak into later
799+
// unrelated navigations (#22782).
800+
useIsomorphicLayoutEffect(() => () => removeRoutesFromAllRoutes(addedRoutes.current), []);
801+
797802
return Routes;
798803
};
799804

@@ -1049,14 +1054,29 @@ export function handleNavigation(opts: {
10491054
}
10501055

10511056
/* Only exported for testing purposes */
1052-
export function addRoutesToAllRoutes(routes: RouteObject[]): void {
1057+
export function addRoutesToAllRoutes(routes: RouteObject[]): RouteObject[] {
1058+
const added: RouteObject[] = [];
10531059
routes.forEach(route => {
10541060
const extractedChildRoutes = getChildRoutesRecursively(route);
10551061

10561062
extractedChildRoutes.forEach(r => {
10571063
allRoutes.add(r);
1064+
added.push(r);
10581065
});
10591066
});
1067+
1068+
return added;
1069+
}
1070+
1071+
/**
1072+
* Removes routes previously added via `addRoutesToAllRoutes` from the shared set. Called when a
1073+
* `<Routes>` unmounts so its routes don't linger and get matched against later, unrelated navigations
1074+
* (which produced hybrid names like `/bar/:fooId` across independent routers - see #22782).
1075+
*/
1076+
function removeRoutesFromAllRoutes(routes: RouteObject[]): void {
1077+
routes.forEach(route => {
1078+
allRoutes.delete(route);
1079+
});
10601080
}
10611081

10621082
function getChildRoutesRecursively(route: RouteObject, allRoutes: Set<RouteObject> = new Set()): Set<RouteObject> {
@@ -1351,6 +1371,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
13511371

13521372
const SentryRoutes: React.FC<P> = (props: P) => {
13531373
const isMountRenderPass = React.useRef(true);
1374+
const addedRoutes = React.useRef<RouteObject[]>([]);
13541375

13551376
const location = _useLocation();
13561377
const navigationType = _useNavigationType();
@@ -1360,7 +1381,7 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
13601381
const routes = _createRoutesFromChildren(props.children) as RouteObject[];
13611382

13621383
if (isMountRenderPass.current) {
1363-
addRoutesToAllRoutes(routes);
1384+
addedRoutes.current = addRoutesToAllRoutes(routes);
13641385

13651386
updatePageloadTransaction({
13661387
activeRootSpan: getActiveRootSpan(),
@@ -1380,6 +1401,10 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
13801401
[location, navigationType],
13811402
);
13821403

1404+
// Drop this `<Routes>`'s routes from the shared set when it unmounts, so they don't leak into later
1405+
// unrelated navigations (#22782).
1406+
useIsomorphicLayoutEffect(() => () => removeRoutesFromAllRoutes(addedRoutes.current), []);
1407+
13831408
// @ts-expect-error Setting more specific React Component typing for `R` generic above
13841409
// will break advanced type inference done by react router params
13851410
return <Routes {...props} />;

0 commit comments

Comments
 (0)