Fix single fetch revalidation bug on reused routes

.changeset/fair-weeks-beam.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Do not short circuit navigation logic if no matches are found requiring revalidation if a user-provided `dataStrategy` exists since it may want to change the revalidation behavior

packages/router/__tests__/data-strategy-test.ts

@@ -569,6 +569,92 @@ describe("router dataStrategy", () => {
         child: "CHILD",
       });
     });
+
+    it("does not short circuit when there are no matchesToLoad", async () => {
+      let dataStrategy = mockDataStrategy(async ({ matches }) => {
+        let results = await Promise.all(
+          matches.map((m) => m.resolve((handler) => handler()))
+        );
+        // Don't use keyedResults since it checks for shouldLoad and this test
+        // is always loading
+        return results.reduce(
+          (acc, r, i) => Object.assign(acc, { [matches[i].route.id]: r }),
+          {}
+        );
+      });
+      let t = setup({
+        routes: [
+          {
+            path: "/",
+          },
+          {
+            id: "parent",
+            path: "/parent",
+            loader: true,
+            children: [
+              {
+                id: "child",
+                path: "child",
+                loader: true,
+              },
+            ],
+          },
+        ],
+        dataStrategy,
+      });
+
+      let A = await t.navigate("/parent");
+      await A.loaders.parent.resolve("PARENT1");
+      expect(A.loaders.parent.stub).toHaveBeenCalled();
+      expect(t.router.state.loaderData).toEqual({
+        parent: "PARENT1",
+      });
+      expect(dataStrategy.mock.calls[0][0].matches).toEqual([
+        expect.objectContaining({
+          route: expect.objectContaining({
+            id: "parent",
+          }),
+        }),
+      ]);
+
+      let B = await t.navigate("/parent/child");
+      await B.loaders.parent.resolve("PARENT2");
+      await B.loaders.child.resolve("CHILD");
+      expect(B.loaders.parent.stub).toHaveBeenCalled();
+      expect(B.loaders.child.stub).toHaveBeenCalled();
+      expect(t.router.state.loaderData).toEqual({
+        parent: "PARENT2",
+        child: "CHILD",
+      });
+      expect(dataStrategy.mock.calls[1][0].matches).toEqual([
+        expect.objectContaining({
+          route: expect.objectContaining({
+            id: "parent",
+          }),
+        }),
+        expect.objectContaining({
+          route: expect.objectContaining({
+            id: "child",
+          }),
+        }),
+      ]);
+
+      let C = await t.navigate("/parent");
+      await C.loaders.parent.resolve("PARENT3");
+      expect(C.loaders.parent.stub).toHaveBeenCalled();
+      expect(t.router.state.loaderData).toEqual({
+        parent: "PARENT3",
+      });
+      expect(dataStrategy.mock.calls[2][0].matches).toEqual([
+        expect.objectContaining({
+          route: expect.objectContaining({
+            id: "parent",
+          }),
+        }),
+      ]);
+
+      expect(dataStrategy).toHaveBeenCalledTimes(3);
+    });
   });
 
   describe("actions", () => {

packages/router/router.ts

@@ -1943,8 +1943,13 @@ export function createRouter(init: RouterInit): Router {
 
     pendingNavigationLoadId = ++incrementingLoadId;
 
-    // Short circuit if we have no loaders to run
-    if (matchesToLoad.length === 0 && revalidatingFetchers.length === 0) {
+    // Short circuit if we have no loaders to run, unless there's a custom dataStrategy
+    // since they may have different revalidation rules (i.e., single fetch)
+    if (
+      !init.dataStrategy &&
+      matchesToLoad.length === 0 &&
+      revalidatingFetchers.length === 0
+    ) {
       let updatedFetchers = markFetchRedirectsDone();
       completeNavigation(
         location,
@@ -3944,7 +3949,7 @@ export function createStaticHandler(
     );
 
     // Short circuit if we have no loaders to run (query())
-    if (matchesToLoad.length === 0) {
+    if (!dataStrategy && matchesToLoad.length === 0) {
       return {
         matches,
         // Add a null for all matched routes for proper revalidation on the client