diff --git a/packages/react-router/__tests__/router/lazy-discovery-test.ts b/packages/react-router/__tests__/router/lazy-discovery-test.ts index c78ab8d3e0..91d56d40b7 100644 --- a/packages/react-router/__tests__/router/lazy-discovery-test.ts +++ b/packages/react-router/__tests__/router/lazy-discovery-test.ts @@ -1273,6 +1273,196 @@ describe("Lazy Route Discovery (Fog of War)", () => { `); }); + it("distinguishes sibling pathless layout routes in idempotent patch check (via id)", async () => { + let count = 0; + router = createRouter({ + history: createMemoryHistory(), + routes: [ + { + id: "root", + path: "/", + children: [ + { + id: "a-layout", + children: [ + { + id: "a", + path: "a", + }, + ], + }, + ], + }, + ], + async patchRoutesOnNavigation({ patch, path }) { + count++; + if (path === "/b") { + patch("root", [ + { + id: "b-layout", + children: [ + { + id: "b", + path: "b", + }, + ], + }, + ]); + } + await tick(); + }, + }); + + await router.navigate("/a"); + expect(router.state.location.pathname).toBe("/a"); + expect(router.state.matches.map((m) => m.route.id)).toEqual([ + "root", + "a-layout", + "a", + ]); + expect(router.state.errors).toBeNull(); + expect(count).toBe(0); + + await router.navigate("/b"); + expect(router.state.location.pathname).toBe("/b"); + expect(router.state.matches.map((m) => m.route.id)).toEqual([ + "root", + "b-layout", + "b", + ]); + expect(router.state.errors).toBeNull(); + expect(count).toBe(1); + + expect(router.routes).toMatchInlineSnapshot(` + [ + { + "children": [ + { + "children": [ + { + "children": undefined, + "hasErrorBoundary": false, + "id": "a", + "path": "a", + }, + ], + "hasErrorBoundary": false, + "id": "a-layout", + }, + { + "children": [ + { + "children": undefined, + "hasErrorBoundary": false, + "id": "b", + "path": "b", + }, + ], + "hasErrorBoundary": false, + "id": "b-layout", + }, + ], + "hasErrorBoundary": false, + "id": "root", + "path": "/", + }, + ] + `); + }); + + it("distinguishes sibling pathless layout routes in idempotent patch check (via children)", async () => { + let count = 0; + router = createRouter({ + history: createMemoryHistory(), + routes: [ + { + id: "root", + path: "/", + children: [ + { + children: [ + { + path: "a", + }, + ], + }, + ], + }, + ], + async patchRoutesOnNavigation({ patch, path }) { + count++; + if (path === "/b") { + patch("root", [ + { + children: [ + { + path: "b", + }, + ], + }, + ]); + } + await tick(); + }, + }); + + await router.navigate("/a"); + expect(router.state.location.pathname).toBe("/a"); + expect(router.state.matches.map((m) => m.route.id)).toEqual([ + "root", + "0-0", + "0-0-0", + ]); + expect(router.state.errors).toBeNull(); + expect(count).toBe(0); + + await router.navigate("/b"); + expect(router.state.location.pathname).toBe("/b"); + expect(router.state.matches.map((m) => m.route.id)).toEqual([ + "root", + "root-patch-1-0", + "root-patch-1-0-0", + ]); + expect(router.state.errors).toBeNull(); + expect(count).toBe(1); + + expect(router.routes).toMatchInlineSnapshot(` + [ + { + "children": [ + { + "children": [ + { + "children": undefined, + "hasErrorBoundary": false, + "id": "0-0-0", + "path": "a", + }, + ], + "hasErrorBoundary": false, + "id": "0-0", + }, + { + "children": [ + { + "children": undefined, + "hasErrorBoundary": false, + "id": "root-patch-1-0-0", + "path": "b", + }, + ], + "hasErrorBoundary": false, + "id": "root-patch-1-0", + }, + ], + "hasErrorBoundary": false, + "id": "root", + "path": "/", + }, + ] + `); + }); + describe("errors", () => { it("lazy 404s (GET navigation)", async () => { let childrenDfd = createDeferred(); diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index d66bdb13cc..f0bd699c6c 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -4514,12 +4514,9 @@ function patchRoutesImpl( // to simplify user-land code. This is useful because we re-call the // `patchRoutesOnNavigation` function for matched routes with params. let uniqueChildren = children.filter( - (a) => - !childrenToPatch.some( - (b) => - a.index === b.index && - a.path === b.path && - a.caseSensitive === b.caseSensitive + (newRoute) => + !childrenToPatch.some((existingRoute) => + isSameRoute(newRoute, existingRoute) ) ); @@ -4533,6 +4530,46 @@ function patchRoutesImpl( childrenToPatch.push(...newRoutes); } +function isSameRoute( + newRoute: AgnosticRouteObject, + existingRoute: AgnosticRouteObject +): boolean { + // Most optimal check is by id + if ( + "id" in newRoute && + "id" in existingRoute && + newRoute.id === existingRoute.id + ) { + return true; + } + + // Second is by pathing differences + if ( + !( + newRoute.index === existingRoute.index && + newRoute.path === existingRoute.path && + newRoute.caseSensitive === existingRoute.caseSensitive + ) + ) { + return false; + } + + // Pathless layout routes are trickier since we need to check children. + // If they have no children then they're the same as far as we can tell + if ( + (!newRoute.children || newRoute.children.length === 0) && + (!existingRoute.children || existingRoute.children.length === 0) + ) { + return true; + } + + // Otherwise, we look to see if every child in the new route is already + // represented in the existing route's children + return newRoute.children!.every((aChild, i) => + existingRoute.children?.some((bChild) => isSameRoute(aChild, bChild)) + ); +} + /** * Execute route.lazy() methods to lazily load route modules (loader, action, * shouldRevalidate) and update the routeManifest in place which shares objects