fix(react): Remove routes from shared set on <Routes> unmount (#22792)

Removes a `<Routes>`'s routes from the module-level `allRoutes` set when
it unmounts, so they can't be matched against a later, unrelated
navigation.

The set accumulated every route ever mounted and never removed any. Once
two independent routers had each been mounted, `matchRoutes` ran over
the union of both, so a navigation into one could pick up a param name
from the other and produce a hybrid transaction name like `/bar/:fooId`
instead of `/bar/:barId`.

I've taken out the route adding logic from the effect we had and added
another one so that we can isolate the add/remove to the same effect to
avoid relying on refs that can be fidgity in dev/prod and strict modes.

closes #22782

Will backport to v10

Author: logaretm

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

@@ -103,13 +103,31 @@ const DeepTeamRoutes = () => (
   </SentryRoutes>
 );
 
+// Two independent descendant <SentryRoutes> trees that each contribute a single-segment param leaf
+// (`:fooId` / `:barId`). Because `allRoutes` is a shared module-level set, once both have mounted a
+// navigation into one can pick up the param name from the other, yielding a hybrid name like
+// `/bar/:fooId` instead of `/bar/:barId` (see issue #22782).
+const FooRoutes = () => (
+  <SentryRoutes>
+    <Route path=":fooId" element={<div id="foo">Foo</div>} />
+  </SentryRoutes>
+);
+
+const BarRoutes = () => (
+  <SentryRoutes>
+    <Route path=":barId" element={<div id="bar">Bar</div>} />
+  </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="foo/*" element={<FooRoutes />} />
+      <Route path="bar/*" element={<BarRoutes />} />
       <Route path="/*" element={<ProjectsRoutes />} />
     </SentryRoutes>
   </BrowserRouter>,

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

@@ -16,6 +16,12 @@ const Index = () => {
       <Link to="/workspace/team/u123" id="deep-member-navigation">
         navigate deep member
       </Link>
+      <Link to="/foo/123" id="foo-navigation">
+        navigate foo
+      </Link>
+      <Link to="/bar/456" id="bar-navigation">
+        navigate bar
+      </Link>
     </>
   );
 };

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

@@ -293,6 +293,66 @@ test('resolves deep wildcard chain with three levels of nesting - pageload', asy
   });
 });
 
+test('does not mix param names across independent descendant routers', async ({ page }) => {
+  const pageloadTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
+    return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
+  });
+
+  const fooNavigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
+    return (
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.contexts?.trace?.data?.['url.path'] === '/foo/123'
+    );
+  });
+
+  const barNavigationTxnPromise = waitForTransaction('react-router-6-descendant-routes', async transactionEvent => {
+    return (
+      transactionEvent.contexts?.trace?.op === 'navigation' &&
+      transactionEvent.contexts?.trace?.data?.['url.path'] === '/bar/456'
+    );
+  });
+
+  await page.goto(`/`);
+  await pageloadTxnPromise;
+
+  // Mount the first descendant router (`foo/*` -> `:fooId`), which populates the shared `allRoutes` set.
+  const [, fooNavigationTxn] = await Promise.all([page.locator('id=foo-navigation').click(), fooNavigationTxnPromise]);
+
+  expect((await page.innerHTML('#root')).includes('Foo')).toBe(true);
+  expect(fooNavigationTxn).toMatchObject({
+    transaction: '/foo/:fooId',
+    transaction_info: { source: 'route' },
+  });
+
+  // Return to the index so we can navigate into the second, unrelated descendant router client-side.
+  // A fresh page load would reset the module-level `allRoutes` and hide the bug.
+  await page.goBack();
+  await page.locator('id=bar-navigation').waitFor();
+
+  // Now mount the second descendant router (`bar/*` -> `:barId`). With the accumulation bug, the name
+  // comes out as the hybrid `/bar/:fooId`.
+  const [, barNavigationTxn] = await Promise.all([page.locator('id=bar-navigation').click(), barNavigationTxnPromise]);
+
+  expect((await page.innerHTML('#root')).includes('Bar')).toBe(true);
+  expect(barNavigationTxn).toMatchObject({
+    contexts: {
+      trace: {
+        op: 'navigation',
+        origin: 'auto.navigation.react.reactrouter_v6',
+        data: {
+          'sentry.source': 'route',
+          'url.template': '/bar/:barId',
+          'url.path': '/bar/456',
+        },
+      },
+    },
+    transaction: '/bar/:barId',
+    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';

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

@@ -766,13 +766,20 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio
     const stableLocationParam =
       typeof locationArg === 'string' || locationArg?.pathname ? (locationArg as { pathname: string }) : location;
 
+    // Register this `<Routes>`'s routes in the shared set for as long as it is mounted, removing them on
+    // unmount so they don't leak into later unrelated navigations (#22782). Tying add and remove to the
+    // same effect lifecycle keeps it correct under StrictMode's mount/unmount/remount.
+    useIsomorphicLayoutEffect(() => {
+      const added = addRoutesToAllRoutes(routes);
+
+      return () => removeRoutesFromAllRoutes(added);
+    });
+
     useIsomorphicLayoutEffect(() => {
       const normalizedLocation =
         typeof stableLocationParam === 'string' ? { pathname: stableLocationParam } : stableLocationParam;
 
       if (isMountRenderPass.current) {
-        addRoutesToAllRoutes(routes);
-
         updatePageloadTransaction({
           activeRootSpan: getActiveRootSpan(),
           location: normalizedLocation,
@@ -1049,14 +1056,29 @@ export function handleNavigation(opts: {
 }
 
 /* Only exported for testing purposes */
-export function addRoutesToAllRoutes(routes: RouteObject[]): void {
+export function addRoutesToAllRoutes(routes: RouteObject[]): RouteObject[] {
+  const added: RouteObject[] = [];
   routes.forEach(route => {
     const extractedChildRoutes = getChildRoutesRecursively(route);
 
     extractedChildRoutes.forEach(r => {
       allRoutes.add(r);
+      added.push(r);
     });
   });
+
+  return added;
+}
+
+/**
+ * Removes routes previously added via `addRoutesToAllRoutes` from the shared set. Called when a
+ * `<Routes>` unmounts so its routes don't linger and get matched against later, unrelated navigations
+ * (which produced hybrid names like `/bar/:fooId` across independent routers - see #22782).
+ */
+function removeRoutesFromAllRoutes(routes: RouteObject[]): void {
+  routes.forEach(route => {
+    allRoutes.delete(route);
+  });
 }
 
 function getChildRoutesRecursively(route: RouteObject, allRoutes: Set<RouteObject> = new Set()): Set<RouteObject> {
@@ -1355,13 +1377,20 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
     const location = _useLocation();
     const navigationType = _useNavigationType();
 
+    const routes = _createRoutesFromChildren(props.children) as RouteObject[];
+
+    // Register this `<Routes>`'s routes in the shared set for as long as it is mounted, removing them on
+    // unmount so they don't leak into later unrelated navigations (#22782). Tying add and remove to the
+    // same effect lifecycle keeps it correct under StrictMode's mount/unmount/remount.
+    useIsomorphicLayoutEffect(() => {
+      const added = addRoutesToAllRoutes(routes);
+
+      return () => removeRoutesFromAllRoutes(added);
+    });
+
     useIsomorphicLayoutEffect(
       () => {
-        const routes = _createRoutesFromChildren(props.children) as RouteObject[];
-
         if (isMountRenderPass.current) {
-          addRoutesToAllRoutes(routes);
-
           updatePageloadTransaction({
             activeRootSpan: getActiveRootSpan(),
             location,