Trigger a new router.routes identity during fog of war route patching

.changeset/lovely-apples-help.md

@@ -0,0 +1,19 @@
+---
+"@remix-run/router": patch
+---
+
+Trigger a new `router.routes` identity/reflow during fog of war route patching
+
+- This also adds a new batched API for `router.patchRoutes` so you can perform multiple patches but only a single reflow at the end:
+
+  ```js
+  // Apply one patch and reflow
+  router.patchRoutes(parentId, children);
+
+  // Apply multiples patches and a single reflow
+  router.patchRoutes((patch) => {
+    patch(parentId, children);
+    patch(parentId2, children2);
+    patch(parentId3, children3);
+  });
+  ```

packages/router/__tests__/lazy-discovery-test.ts

@@ -835,6 +835,137 @@ describe("Lazy Route Discovery (Fog of War)", () => {
     ]);
   });
 
+  it("creates a new router.routes identity when patching routes", async () => {
+    let childrenDfd = createDeferred<AgnosticDataRouteObject[]>();
+
+    router = createRouter({
+      history: createMemoryHistory(),
+      routes: [
+        {
+          path: "/",
+        },
+        {
+          id: "parent",
+          path: "parent",
+        },
+      ],
+      async unstable_patchRoutesOnMiss({ patch }) {
+        let children = await childrenDfd.promise;
+        patch("parent", children);
+      },
+    });
+    let originalRoutes = router.routes;
+
+    router.navigate("/parent/child");
+    childrenDfd.resolve([
+      {
+        id: "child",
+        path: "child",
+      },
+    ]);
+    await tick();
+
+    expect(router.state.location.pathname).toBe("/parent/child");
+    expect(router.state.matches.map((m) => m.route.id)).toEqual([
+      "parent",
+      "child",
+    ]);
+
+    expect(router.routes).not.toBe(originalRoutes);
+  });
+
+  it("allows patching externally/eagerly and triggers a reflow", async () => {
+    router = createRouter({
+      history: createMemoryHistory(),
+      routes: [
+        {
+          path: "/",
+        },
+        {
+          id: "parent",
+          path: "parent",
+        },
+      ],
+    });
+    let spy = jest.fn();
+    let unsubscribe = router.subscribe(spy);
+    let originalRoutes = router.routes;
+    router.patchRoutes("parent", [
+      {
+        id: "child",
+        path: "child",
+      },
+    ]);
+    router.patchRoutes("parent", [
+      {
+        id: "child2",
+        path: "child2",
+      },
+    ]);
+    expect(spy).toHaveBeenCalledTimes(2);
+    expect(router.routes).not.toBe(originalRoutes);
+
+    await router.navigate("/parent/child");
+    expect(router.state.location.pathname).toBe("/parent/child");
+    expect(router.state.matches.map((m) => m.route.id)).toEqual([
+      "parent",
+      "child",
+    ]);
+
+    unsubscribe();
+  });
+
+  it("allows patching externally/eagerly and triggers a reflow (batch API)", async () => {
+    router = createRouter({
+      history: createMemoryHistory(),
+      routes: [
+        {
+          path: "/",
+        },
+        {
+          id: "parent",
+          path: "parent",
+        },
+      ],
+    });
+    let spy = jest.fn();
+    let unsubscribe = router.subscribe(spy);
+    let originalRoutes = router.routes;
+    router.patchRoutes((patch) => {
+      patch("parent", [
+        {
+          id: "child",
+          path: "child",
+        },
+      ]);
+      patch("parent", [
+        {
+          id: "child2",
+          path: "child2",
+        },
+      ]);
+    });
+
+    expect(spy).toHaveBeenCalledTimes(1);
+    expect(router.routes).not.toBe(originalRoutes);
+
+    await router.navigate("/parent/child");
+    expect(router.state.location.pathname).toBe("/parent/child");
+    expect(router.state.matches.map((m) => m.route.id)).toEqual([
+      "parent",
+      "child",
+    ]);
+
+    await router.navigate("/parent/child2");
+    expect(router.state.location.pathname).toBe("/parent/child2");
+    expect(router.state.matches.map((m) => m.route.id)).toEqual([
+      "parent",
+      "child2",
+    ]);
+
+    unsubscribe();
+  });
+
   describe("errors", () => {
     it("lazy 404s (GET navigation)", async () => {
       let childrenDfd = createDeferred<AgnosticDataRouteObject[]>();

packages/router/router.ts

@@ -249,10 +249,16 @@ export interface Router {
    * PRIVATE DO NOT USE
    *
    * Patch additional children routes into an existing parent route
-   * @param routeId The parent route id
+   * @param routeId The parent route id or a callback function accepting `patch`
+   *                to perform batch patching
    * @param children The additional children routes
    */
   patchRoutes(routeId: string | null, children: AgnosticRouteObject[]): void;
+  patchRoutes(
+    cb: (
+      patch: (r: string | null, children: AgnosticRouteObject[]) => void
+    ) => void
+  ): void;
 
   /**
    * @internal
@@ -1222,6 +1228,7 @@ export function createRouter(init: RouterInit): Router {
         isMutationMethod(state.navigation.formMethod) &&
         location.state?._isRedirect !== true);
 
+    // Commit any in-flight routes at the end of the HMR revalidation "navigation"
     if (inFlightDataRoutes) {
       dataRoutes = inFlightDataRoutes;
       inFlightDataRoutes = undefined;
@@ -3204,26 +3211,37 @@ export function createRouter(init: RouterInit): Router {
         ? partialMatches[partialMatches.length - 1].route
         : null;
     while (true) {
+      let isNonHMR = inFlightDataRoutes == null;
+      let routesToUse = inFlightDataRoutes || dataRoutes;
       try {
         await loadLazyRouteChildren(
           patchRoutesOnMissImpl!,
           pathname,
           partialMatches,
-          dataRoutes || inFlightDataRoutes,
+          routesToUse,
           manifest,
           mapRouteProperties,
           pendingPatchRoutes,
           signal
         );
       } catch (e) {
         return { type: "error", error: e, partialMatches };
+      } finally {
+        // If we are not in the middle of an HMR revalidation and we changed the
+        // routes, provide a new identity so when we `updateState` at the end of
+        // this navigation/fetch `router.routes` will be a new identity and
+        // trigger a re-run of memoized `router.routes` dependencies.
+        // HMR will already update the identity and reflow when it lands
+        // `inFlightDataRoutes` in `completeNavigation`
+        if (isNonHMR) {
+          dataRoutes = [...dataRoutes];
+        }
       }
 
       if (signal.aborted) {
         return { type: "aborted" };
       }
 
-      let routesToUse = inFlightDataRoutes || dataRoutes;
       let newMatches = matchRoutes(routesToUse, pathname, basename);
       let matchedSplat = false;
       if (newMatches) {
@@ -3284,6 +3302,51 @@ export function createRouter(init: RouterInit): Router {
     );
   }
 
+  // single-patch implementation:
+  //   router.patchRoutes(parentId, children);
+  //
+  // batch-patch implementation:
+  //   router.patchRoutes((patch) => {
+  //     patch(parentId1, children1);
+  //     patch(parentId2, children2);
+  //     patch(parentId3, children3);
+  //   });
+  function patchRoutes(
+    routeIdOrCb:
+      | string
+      | null
+      | ((
+          patch: (r: string | null, children: AgnosticRouteObject[]) => void
+        ) => void),
+    children?: AgnosticRouteObject[]
+  ): void {
+    let isNonHMR = inFlightDataRoutes == null;
+    let routesToUse = inFlightDataRoutes || dataRoutes;
+    if (typeof routeIdOrCb === "function") {
+      routeIdOrCb((r, c) =>
+        patchRoutesImpl(r, c, routesToUse, manifest, mapRouteProperties)
+      );
+    } else if (typeof routeIdOrCb === "string" && children != null) {
+      patchRoutesImpl(
+        routeIdOrCb,
+        children,
+        routesToUse,
+        manifest,
+        mapRouteProperties
+      );
+    }
+
+    // If we are not in the middle of an HMR revalidation and we changed the
+    // routes, provide a new identity and trigger a reflow via `updateState`
+    // to re-run memoized `router.routes` dependencies.
+    // HMR will already update the identity and reflow when it lands
+    // `inFlightDataRoutes` in `completeNavigation`
+    if (isNonHMR) {
+      dataRoutes = [...dataRoutes];
+      updateState({});
+    }
+  }
+
   router = {
     get basename() {
       return basename;
@@ -3315,15 +3378,7 @@ export function createRouter(init: RouterInit): Router {
     dispose,
     getBlocker,
     deleteBlocker,
-    patchRoutes(routeId, children) {
-      return patchRoutes(
-        routeId,
-        children,
-        dataRoutes || inFlightDataRoutes,
-        manifest,
-        mapRouteProperties
-      );
-    },
+    patchRoutes,
     _internalFetchControllers: fetchControllers,
     _internalActiveDeferreds: activeDeferreds,
     // TODO: Remove setRoutes, it's temporary to avoid dealing with
@@ -4488,7 +4543,7 @@ function shouldRevalidateLoader(
 }
 
 /**
- * Idempotent utility to execute route.children() method to lazily load route
+ * Idempotent utility to execute patchRoutesOnMiss() to lazily load route
  * definitions and update the routes/routeManifest
  */
 async function loadLazyRouteChildren(
@@ -4510,7 +4565,7 @@ async function loadLazyRouteChildren(
         matches,
         patch: (routeId, children) => {
           if (!signal.aborted) {
-            patchRoutes(
+            patchRoutesImpl(
               routeId,
               children,
               routes,
@@ -4531,10 +4586,10 @@ async function loadLazyRouteChildren(
   }
 }
 
-function patchRoutes(
+function patchRoutesImpl(
   routeId: string | null,
   children: AgnosticRouteObject[],
-  routes: AgnosticDataRouteObject[],
+  routesToUse: AgnosticDataRouteObject[],
   manifest: RouteManifest,
   mapRouteProperties: MapRoutePropertiesFunction
 ) {
@@ -4559,10 +4614,10 @@ function patchRoutes(
     let dataChildren = convertRoutesToDataRoutes(
       children,
       mapRouteProperties,
-      ["patch", String(routes.length || "0")],
+      ["patch", String(routesToUse.length || "0")],
       manifest
     );
-    routes.push(...dataChildren);
+    routesToUse.push(...dataChildren);
   }
 }