Remove internal cache from patchRoutesOnNavigation (#12055)

remix-run · Oct 10, 2024 · 4d56751 · 4d56751
1 parent 38dae6f
commit 4d56751
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 64 deletions.
diff --git a/packages/react-router/__tests__/router/lazy-discovery-test.ts b/packages/react-router/__tests__/router/lazy-discovery-test.ts
@@ -1,7 +1,7 @@
 import type { Router } from "../../lib/router/router";
 import type { AgnosticDataRouteObject } from "../../lib/router/utils";
 import { createMemoryHistory } from "../../lib/router/history";
-import { createRouter } from "../../lib/router/router";
+import { IDLE_NAVIGATION, createRouter } from "../../lib/router/router";
 import { ErrorResponseImpl } from "../../lib/router/utils";
 import { getFetcherData } from "./utils/data-router-setup";
 import { createDeferred, createFormData, tick } from "./utils/utils";
@@ -272,7 +272,7 @@ describe("Lazy Route Discovery (Fog of War)", () => {
     ]);
   });
 
-  it("reuses promises", async () => {
+  it("does not reuse former calls to patchRoutes on interruptions", async () => {
     let aDfd = createDeferred<AgnosticDataRouteObject[]>();
     let calls: string[][] = [];
     router = createRouter({
@@ -308,8 +308,10 @@ describe("Lazy Route Discovery (Fog of War)", () => {
     expect(router.state).toMatchObject({
       navigation: { state: "submitting", location: { pathname: "/a/b" } },
     });
-    // Didn't call again for the same path
-    expect(calls).toEqual([["/a/b", "a"]]);
+    expect(calls).toEqual([
+      ["/a/b", "a"],
+      ["/a/b", "a"],
+    ]);
 
     aDfd.resolve([
       {
@@ -324,10 +326,71 @@ describe("Lazy Route Discovery (Fog of War)", () => {
       navigation: { state: "idle" },
       location: { pathname: "/a/b" },
     });
-    expect(calls).toEqual([["/a/b", "a"]]);
+    expect(calls).toEqual([
+      ["/a/b", "a"],
+      ["/a/b", "a"],
+    ]);
+  });
+
+  it("handles interruptions when navigating to the same route", async () => {
+    let dfd1 = createDeferred<AgnosticDataRouteObject[]>();
+    let dfd2 = createDeferred<AgnosticDataRouteObject[]>();
+    let called = false;
+    router = createRouter({
+      history: createMemoryHistory(),
+      routes: [
+        {
+          path: "/",
+        },
+      ],
+      async patchRoutesOnNavigation({ patch }) {
+        if (!called) {
+          called = true;
+          patch(null, await dfd1.promise);
+        } else {
+          patch(null, await dfd2.promise);
+        }
+      },
+    });
+
+    router.navigate("/a");
+    await tick();
+    expect(router.state).toMatchObject({
+      navigation: { state: "loading", location: { pathname: "/a" } },
+    });
+
+    router.navigate("/a");
+    await tick();
+    expect(router.state).toMatchObject({
+      navigation: { state: "loading", location: { pathname: "/a" } },
+    });
+
+    dfd1.resolve([
+      {
+        id: "a1",
+        path: "/a",
+      },
+    ]);
+    await tick();
+    expect(router.state).toMatchObject({
+      navigation: { state: "loading", location: { pathname: "/a" } },
+    });
+
+    dfd2.resolve([
+      {
+        id: "a2",
+        path: "/a",
+      },
+    ]);
+    await tick();
+    expect(router.state).toMatchObject({
+      location: { pathname: "/a" },
+      navigation: IDLE_NAVIGATION,
+      matches: [{ route: { id: "a2" } }],
+    });
   });
 
-  it("handles interruptions", async () => {
+  it("handles interruptions when navigating to a new route", async () => {
     let aDfd = createDeferred<AgnosticDataRouteObject[]>();
     let bDfd = createDeferred<AgnosticDataRouteObject[]>();
     router = createRouter({

diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts
@@ -3179,21 +3179,30 @@ export function createRouter(init: RouterInit): Router {
     pathname: string,
     signal: AbortSignal
   ): Promise<DiscoverRoutesResult> {
+    if (!patchRoutesOnNavigationImpl) {
+      return { type: "success", matches };
+    }
+
     let partialMatches: AgnosticDataRouteMatch[] | null = matches;
     while (true) {
       let isNonHMR = inFlightDataRoutes == null;
       let routesToUse = inFlightDataRoutes || dataRoutes;
+      let localManifest = manifest;
       try {
-        await loadLazyRouteChildren(
-          patchRoutesOnNavigationImpl!,
-          pathname,
-          partialMatches,
-          routesToUse,
-          manifest,
-          mapRouteProperties,
-          pendingPatchRoutes,
-          signal
-        );
+        await patchRoutesOnNavigationImpl({
+          path: pathname,
+          matches: partialMatches,
+          patch: (routeId, children) => {
+            if (signal.aborted) return;
+            patchRoutesImpl(
+              routeId,
+              children,
+              routesToUse,
+              localManifest,
+              mapRouteProperties
+            );
+          },
+        });
       } catch (e) {
         return { type: "error", error: e, partialMatches };
       } finally {
@@ -3203,7 +3212,7 @@ export function createRouter(init: RouterInit): Router {
         // 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) {
+        if (isNonHMR && !signal.aborted) {
           dataRoutes = [...dataRoutes];
         }
       }
@@ -4420,53 +4429,6 @@ function shouldRevalidateLoader(
   return arg.defaultShouldRevalidate;
 }
 
-/**
- * Idempotent utility to execute patchRoutesOnNavigation() to lazily load route
- * definitions and update the routes/routeManifest
- */
-async function loadLazyRouteChildren(
-  patchRoutesOnNavigationImpl: AgnosticPatchRoutesOnNavigationFunction,
-  path: string,
-  matches: AgnosticDataRouteMatch[],
-  routes: AgnosticDataRouteObject[],
-  manifest: RouteManifest,
-  mapRouteProperties: MapRoutePropertiesFunction,
-  pendingRouteChildren: Map<
-    string,
-    ReturnType<typeof patchRoutesOnNavigationImpl>
-  >,
-  signal: AbortSignal
-) {
-  let key = [path, ...matches.map((m) => m.route.id)].join("-");
-  try {
-    let pending = pendingRouteChildren.get(key);
-    if (!pending) {
-      pending = patchRoutesOnNavigationImpl({
-        path,
-        matches,
-        patch: (routeId, children) => {
-          if (!signal.aborted) {
-            patchRoutesImpl(
-              routeId,
-              children,
-              routes,
-              manifest,
-              mapRouteProperties
-            );
-          }
-        },
-      });
-      pendingRouteChildren.set(key, pending);
-    }
-
-    if (pending && isPromise<AgnosticRouteObject[]>(pending)) {
-      await pending;
-    }
-  } finally {
-    pendingRouteChildren.delete(key);
-  }
-}
-
 function patchRoutesImpl(
   routeId: string | null,
   children: AgnosticRouteObject[],