From d34fcf742f714ece242e0cb1f568749cc9ec8c53 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 28 Aug 2024 11:35:54 -0400
Subject: [PATCH 1/7] Fix single fetch revalidation behavior

---
 docs/guides/single-fetch.md           |   8 ++
 integration/single-fetch-test.ts      | 153 ++++++++++++++++++++++----
 packages/remix-react/single-fetch.tsx | 134 +++++++++++++++++-----
 3 files changed, 241 insertions(+), 54 deletions(-)

diff --git a/docs/guides/single-fetch.md b/docs/guides/single-fetch.md
index 22fc56c0496..e0993bb4da1 100644
--- a/docs/guides/single-fetch.md
+++ b/docs/guides/single-fetch.md
@@ -450,6 +450,14 @@ function handleBrowserRequest(
 
 ### Revalidations
 
+#### Normal Navigation Behavior
+
+for caching purposes we call `.data` and run all loaders
+the presence of a `shoulRevalidate` will opt-into granular revalidation
+and `childLoader` will also opt into granular revalidation
+
+#### Submission Revalidation Behavior
+
 Previously, Remix would always revalidate all active loaders after _any_ action submission, regardless of the result of the action. You could opt-out of revalidation on a per-route basis via [`shouldRevalidate`][should-revalidate].
 
 With Single Fetch, if an `action` returns or throws a `Response` with a `4xx/5xx` status code, Remix will _not revalidate_ loaders by default. If an `action` returns or throws anything that is not a 4xx/5xx Response, then the revalidation behavior is unchanged. The reasoning here is that in most cases, if you return a `4xx`/`5xx` Response, you didn't actually mutate any data so there is no need to reload data.
diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts
index fbcc47f4152..adf1caaa724 100644
--- a/integration/single-fetch-test.ts
+++ b/integration/single-fetch-test.ts
@@ -333,15 +333,18 @@ test.describe("single-fetch", () => {
   test("loads proper data on client side action navigation", async ({
     page,
   }) => {
-    let fixture = await createFixture({
-      config: {
-        future: {
-          unstable_singleFetch: true,
+    let fixture = await createFixture(
+      {
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
         },
+        files,
       },
-      files,
-    });
-    let appFixture = await createAppFixture(fixture);
+      ServerMode.Development
+    );
+    let appFixture = await createAppFixture(fixture, ServerMode.Development);
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickSubmitButton("/data");
@@ -1668,15 +1671,16 @@ test.describe("single-fetch", () => {
   test("allows fetcher to hit resource route and return via turbo stream", async ({
     page,
   }) => {
-    let fixture = await createFixture({
-      config: {
-        future: {
-          unstable_singleFetch: true,
+    let fixture = await createFixture(
+      {
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
         },
-      },
-      files: {
-        ...files,
-        "app/routes/_index.tsx": js`
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
           import { useFetcher } from "@remix-run/react";
 
           export default function Component() {
@@ -1691,7 +1695,7 @@ test.describe("single-fetch", () => {
             );
           }
         `,
-        "app/routes/resource.tsx": js`
+          "app/routes/resource.tsx": js`
           export function loader() {
             // Fetcher calls to resource routes will append ".data" and we'll go through
             // the turbo-stream flow.  If a user were to curl this endpoint they'd go
@@ -1702,9 +1706,11 @@ test.describe("single-fetch", () => {
             };
           }
         `,
+        },
       },
-    });
-    let appFixture = await createAppFixture(fixture);
+      ServerMode.Development
+    );
+    let appFixture = await createAppFixture(fixture, ServerMode.Development);
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickElement("#load");
@@ -1714,6 +1720,105 @@ test.describe("single-fetch", () => {
     );
   });
 
+  test("calls reused parent routes by default", async ({ page }) => {
+    let fixture = await createFixture(
+      {
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
+        },
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
+          import { Link } from '@remix-run/react';
+          export default function Component() {
+            return <Link to="/parent/a">Go to /parent/a</Link>;
+          }
+        `,
+          "app/routes/parent.tsx": js`
+          import { Link, Outlet, useLoaderData } from '@remix-run/react';
+          let count = 0;
+          export function loader({ request }) {
+            return { count: ++count };
+          }
+          export default function Component() {
+            return (
+              <>
+                <p id="parent">Parent Count: {useLoaderData().count}</p>
+                <Link to="/parent/a">Go to /parent/a</Link>
+                <Link to="/parent/b">Go to /parent/b</Link>
+                <Outlet/>
+              </>
+            );
+          }
+        `,
+          "app/routes/parent.a.tsx": js`
+          import { useLoaderData } from '@remix-run/react';
+          let count = 0;
+          export function loader({ request }) {
+            return { count: ++count };
+          }
+          export default function Component() {
+            return <p id="a">A Count: {useLoaderData().count}</p>;
+          }
+        `,
+          "app/routes/parent.b.tsx": js`
+          import { useLoaderData } from '@remix-run/react';
+          let count = 0;
+          export function loader({ request }) {
+            return { count: ++count };
+          }
+          export default function Component() {
+            return <p id="b">B Count: {useLoaderData().count}</p>;
+          }
+        `,
+        },
+      },
+      ServerMode.Development
+    );
+    let appFixture = await createAppFixture(fixture, ServerMode.Development);
+    let app = new PlaywrightFixture(appFixture, page);
+
+    let urls: string[] = [];
+    page.on("request", (req) => {
+      if (req.url().includes(".data")) {
+        urls.push(req.url());
+      }
+    });
+
+    await app.goto("/");
+
+    await app.clickLink("/parent/a");
+    await page.waitForSelector("#a");
+    expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+    expect(await app.getHtml("#a")).toContain("A Count: 1");
+    expect(urls).toEqual([expect.stringMatching(/\/parent\/a\.data$/)]);
+    urls = [];
+
+    await app.clickLink("/parent/b");
+    await page.waitForSelector("#b");
+    expect(await app.getHtml("#parent")).toContain("Parent Count: 2");
+    expect(await app.getHtml("#b")).toContain("B Count: 1");
+    expect(urls).toEqual([expect.stringMatching(/\/parent\/b\.data$/)]);
+    urls = [];
+
+    await app.clickLink("/parent/a");
+    await page.waitForSelector("#a");
+    expect(await app.getHtml("#parent")).toContain("Parent Count: 3");
+    expect(await app.getHtml("#a")).toContain("A Count: 2");
+    expect(urls).toEqual([expect.stringMatching(/\/parent\/a\.data$/)]);
+  });
+
+  // Calls reused parent route by default
+  // Does not call reused parent route if it opts out via shouldRevalidate
+  // Does not call reused parent route if it opts out via shouldRevalidate and has a clientLoader
+  // Action requests do not use ?_routes and do not run loaders on the server
+  // Fetcher loaders call singular routes
+  // Fetcher actions call .data routes without a param
+  // Fetcher loads revalidate by default
+  // Fetcher loaders can opt out of revalidation via shouldRevalidate
+
   test.describe("client loaders", () => {
     test("when no routes have client loaders", async ({ page }) => {
       let fixture = await createFixture(
@@ -1894,10 +1999,10 @@ test.describe("single-fetch", () => {
         "C server loader (C client loader)"
       );
 
-      // A/B can be loaded together, C needs it's own call due to it's clientLoader
+      // root/A/B can be loaded together, C needs it's own call due to it's clientLoader
       expect(urls.sort()).toEqual([
         expect.stringMatching(
-          /\/a\/b\/c\.data\?_routes=routes%2Fa%2Croutes%2Fa\.b$/
+          /\/a\/b\/c\.data\?_routes=root%2Croutes%2Fa%2Croutes%2Fa\.b$/
         ),
         expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b\.c$/),
       ]);
@@ -2001,10 +2106,9 @@ test.describe("single-fetch", () => {
         "C server loader (C client loader)"
       );
 
-      // B/C have client loaders so they get individual calls, which leaves A
-      // getting it's own "individual" since it's the last route standing
+      // B/C have client loaders so they get individual calls, root/A go together
       expect(urls.sort()).toEqual([
-        expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa$/),
+        expect.stringMatching(/\/a\/b\/c\.data\?_routes=root%2Croutes%2Fa$/),
         expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b$/),
         expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b\.c$/),
       ]);
@@ -2115,8 +2219,9 @@ test.describe("single-fetch", () => {
         "C server loader (C client loader)"
       );
 
-      // A/B/C all have client loaders so they get individual calls
+      // root/A/B/C all have client loaders so they get individual calls
       expect(urls.sort()).toEqual([
+        expect.stringMatching(/\/a\/b\/c.data\?_routes=root$/),
         expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa$/),
         expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa.b$/),
         expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa.b.c$/),
diff --git a/packages/remix-react/single-fetch.tsx b/packages/remix-react/single-fetch.tsx
index a49bd1e4648..8b27cf931cd 100644
--- a/packages/remix-react/single-fetch.tsx
+++ b/packages/remix-react/single-fetch.tsx
@@ -28,7 +28,7 @@ import { decode } from "turbo-stream";
 import { createRequestInit, isResponse } from "./data";
 import type { AssetsManifest, EntryContext } from "./entry";
 import { escapeHtml } from "./markup";
-import type { RouteModules } from "./routeModules";
+import { type RouteModules } from "./routeModules";
 import invariant from "./invariant";
 
 // clientLoader
@@ -143,10 +143,16 @@ export function getSingleFetchDataStrategy(
   manifest: AssetsManifest,
   routeModules: RouteModules
 ): DataStrategyFunction {
-  return async ({ request, matches }) =>
+  return async ({ request, matches, fetcherKey }) =>
     request.method !== "GET"
       ? singleFetchActionStrategy(request, matches)
-      : singleFetchLoaderStrategy(manifest, routeModules, request, matches);
+      : singleFetchLoaderStrategy(
+          manifest,
+          routeModules,
+          request,
+          matches,
+          fetcherKey
+        );
 }
 
 // Actions are simple since they're singular calls to the server
@@ -158,6 +164,9 @@ function singleFetchActionStrategy(
     matches.map(async (m) => {
       let actionStatus: number | undefined;
       let result = await m.resolve(async (handler): Promise<HandlerResult> => {
+        if (!m.shouldLoad) {
+          return { type: "data", result: undefined };
+        }
         let result = await handler(async () => {
           let url = singleFetchUrl(request.url);
           let init = await createRequestInit(request);
@@ -184,50 +193,88 @@ function singleFetchActionStrategy(
 
 // Loaders are trickier since we only want to hit the server once, so we
 // create a singular promise for all server-loader routes to latch onto.
-function singleFetchLoaderStrategy(
+async function singleFetchLoaderStrategy(
   manifest: AssetsManifest,
   routeModules: RouteModules,
   request: Request,
-  matches: DataStrategyFunctionArgs["matches"]
+  matches: DataStrategyFunctionArgs["matches"],
+  fetcherKey: string | null
 ) {
+  let optOutRoutes = new Set<string>();
+  let url = stripIndexParam(singleFetchUrl(request.url));
   let singleFetchPromise: Promise<SingleFetchResults> | undefined;
-  return Promise.all(
-    matches.map(async (m) =>
+
+  let dfds = matches.map(() => createDeferred<void>());
+  let routesLoadedPromise = Promise.all(dfds.map((d) => d.promise));
+  let init = await createRequestInit(request);
+
+  let results = await Promise.all(
+    matches.map(async (m, i) =>
       m.resolve(async (handler): Promise<HandlerResult> => {
         let result: unknown;
-        let url = stripIndexParam(singleFetchUrl(request.url));
-        let init = await createRequestInit(request);
+
+        dfds[i].resolve();
+
+        // Opt out if:
+        //   We currently have data
+        //   and we were told not to load,
+        //   and we have a loader,
+        //   and a shouldRevalidate function
+        // This implies that the user opted out via shouldRevalidate()
+        if (
+          m.data !== undefined &&
+          !m.shouldLoad &&
+          manifest.routes[m.route.id].hasLoader &&
+          routeModules[m.route.id]?.shouldRevalidate
+        ) {
+          optOutRoutes.add(m.route.id);
+          return { type: "data", result: m.data };
+        }
 
         // When a route has a client loader, it calls it's singular server loader
-        if (manifest.routes[m.route.id].hasClientLoader) {
+        if (fetcherKey || manifest.routes[m.route.id].hasClientLoader) {
+          optOutRoutes.add(m.route.id);
           result = await handler(async () => {
-            url.searchParams.set("_routes", m.route.id);
-            let { data } = await fetchAndDecode(url, init);
+            let clientLoaderUrl = new URL(url);
+            clientLoaderUrl.searchParams.set("_routes", m.route.id);
+            let { data } = await fetchAndDecode(clientLoaderUrl, init);
             return unwrapSingleFetchResults(
               data as SingleFetchResults,
               m.route.id
             );
           });
-        } else {
-          result = await handler(async () => {
-            // Otherwise we let multiple routes hook onto the same promise
-            if (!singleFetchPromise) {
-              url = addRevalidationParam(
-                manifest,
-                routeModules,
-                matches.map((m) => m.route),
-                matches.filter((m) => m.shouldLoad).map((m) => m.route),
-                url
-              );
-              singleFetchPromise = fetchAndDecode(url, init).then(
-                ({ data }) => data as SingleFetchResults
-              );
-            }
-            let results = await singleFetchPromise;
-            return unwrapSingleFetchResults(results, m.route.id);
-          });
+          return {
+            type: "data",
+            result,
+          };
+        }
+
+        // Otherwise, we can lump this route with the others on a singular
+        // `singleFetchPromise`
+
+        // Wait to let other routes fill out optOutRoutes before creating the
+        // _routes param
+        await routesLoadedPromise;
+        if (optOutRoutes.size > 0) {
+          url.searchParams.set(
+            "_routes",
+            matches
+              .filter((m) => !optOutRoutes.has(m.route.id))
+              .map((m) => m.route.id)
+              .join(",")
+          );
         }
 
+        result = await handler(async () => {
+          if (!singleFetchPromise) {
+            singleFetchPromise = fetchAndDecode(url, init).then(
+              ({ data }) => data as SingleFetchResults
+            );
+          }
+          let results = await singleFetchPromise;
+          return unwrapSingleFetchResults(results, m.route.id);
+        });
+
         return {
           type: "data",
           result,
@@ -235,6 +282,7 @@ function singleFetchLoaderStrategy(
       })
     )
   );
+  return results;
 }
 
 function stripIndexParam(url: URL) {
@@ -419,3 +467,29 @@ function unwrapSingleFetchResult(result: SingleFetchResult, routeId: string) {
     throw new Error(`No response found for routeId "${routeId}"`);
   }
 }
+
+function createDeferred<T = unknown>() {
+  let resolve: (val?: any) => Promise<void>;
+  let reject: (error?: Error) => Promise<void>;
+  let promise = new Promise<T>((res, rej) => {
+    resolve = async (val: T) => {
+      res(val);
+      try {
+        await promise;
+      } catch (e) {}
+    };
+    reject = async (error?: Error) => {
+      rej(error);
+      try {
+        await promise;
+      } catch (e) {}
+    };
+  });
+  return {
+    promise,
+    //@ts-ignore
+    resolve,
+    //@ts-ignore
+    reject,
+  };
+}

From 0332c3f707f15ead2f57ae6c1d4bd603817fb023 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 28 Aug 2024 14:02:23 -0400
Subject: [PATCH 2/7] Add e2e tests

---
 integration/single-fetch-test.ts      | 448 +++++++++++++++++++++++---
 packages/remix-server-runtime/data.ts |  25 +-
 2 files changed, 432 insertions(+), 41 deletions(-)

diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts
index adf1caaa724..655ac24aff5 100644
--- a/integration/single-fetch-test.ts
+++ b/integration/single-fetch-test.ts
@@ -1720,9 +1720,167 @@ test.describe("single-fetch", () => {
     );
   });
 
-  test("calls reused parent routes by default", async ({ page }) => {
+  test("Strips ?_routes query param from loader/action requests", async ({
+    page,
+  }) => {
     let fixture = await createFixture(
       {
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
+        },
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
+          import { Link } from '@remix-run/react';
+          export default function Component() {
+            return <Link to="/parent/a">Go to /parent/a</Link>;
+          }
+        `,
+          "app/routes/parent.tsx": js`
+          import { Link, Outlet, useLoaderData } from '@remix-run/react';
+          export function loader({ request }) {
+            return { url: request.url };
+          }
+          export default function Component() {
+            return (
+              <>
+                <p id="parent">Parent loader URL: {useLoaderData().url}</p>
+                <Outlet/>
+              </>
+            );
+          }
+        `,
+          "app/routes/parent.a.tsx": js`
+          import { useLoaderData } from '@remix-run/react';
+          export function loader({ request }) {
+            return { url: request.url };
+          }
+          export async function clientLoader({ request, serverLoader }) {
+            debugger;
+            let serverData = await serverLoader();
+            return {
+              serverUrl: serverData.url,
+              clientUrl: request.url
+            }
+          }
+          export default function Component() {
+            let data = useLoaderData();
+            return (
+              <>
+                <p id="a-server">A server loader URL: {data.serverUrl}</p>
+                <p id="a-client">A client loader URL: {data.clientUrl}</p>
+              </>
+            );
+          }
+        `,
+        },
+      },
+      ServerMode.Development
+    );
+    let appFixture = await createAppFixture(fixture, ServerMode.Development);
+    let app = new PlaywrightFixture(appFixture, page);
+
+    let urls: string[] = [];
+    page.on("request", (req) => {
+      if (req.url().includes(".data")) {
+        urls.push(req.url());
+      }
+    });
+
+    await app.goto("/");
+
+    await app.clickLink("/parent/a");
+    await page.waitForSelector("#a-server");
+
+    // HTTP Requests contained routes params
+    expect(urls.length).toBe(2);
+    expect(urls[0].endsWith("/parent/a.data?_routes=routes%2Fparent.a")).toBe(
+      true
+    );
+    expect(
+      urls[1].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent")
+    ).toBe(true);
+
+    // But loaders don't receive any routes params
+    expect(await app.getHtml("#parent")).toMatch(
+      />Parent loader URL: http:\/\/localhost:\d+\/parent\/a</
+    );
+    expect(await app.getHtml("#a-server")).toMatch(
+      />A server loader URL: http:\/\/localhost:\d+\/parent\/a</
+    );
+    expect(await app.getHtml("#a-client")).toMatch(
+      />A client loader URL: http:\/\/localhost:\d+\/parent\/a</
+    );
+  });
+
+  test("Action requests do not use _routes and do not call loaders on the server", async ({
+    page,
+  }) => {
+    let fixture = await createFixture(
+      {
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
+        },
+        files: {
+          ...files,
+          "app/routes/page.tsx": js`
+            import { Form, useActionData, useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export function action({ request }) {
+              return { message: "ACTION" };
+            }
+            export default function Component() {
+              let data = useLoaderData();
+              let actionData = useActionData();
+              return (
+                <>
+                  <p id="data">{"Count:" + data.count}</p>
+                  <Form method="post">
+                    <button type="submit">Submit</button>
+                    {actionData ? <p id="action">{actionData.message}</p> : null}
+                  </Form>
+                </>
+              )
+            }
+          `,
+        },
+      },
+      ServerMode.Development
+    );
+    let appFixture = await createAppFixture(fixture, ServerMode.Development);
+    let app = new PlaywrightFixture(appFixture, page);
+
+    let urls: string[] = [];
+    page.on("request", (req) => {
+      if (req.url().includes(".data")) {
+        urls.push(req.method() + " " + req.url());
+      }
+    });
+
+    await app.goto("/page");
+    expect(await app.getHtml("#data")).toContain("Count:1");
+
+    await app.clickSubmitButton("/page");
+    await page.waitForSelector("#action");
+    expect(await app.getHtml("#data")).toContain("Count:2");
+
+    // HTTP Requests contained routes params
+    expect(urls).toEqual([
+      expect.stringMatching(/POST .*\/page.data$/),
+      expect.stringMatching(/GET .*\/page.data$/),
+    ]);
+  });
+
+  test.describe("revalidations", () => {
+    test("calls reused parent routes by default", async ({ page }) => {
+      let fixture = await createFixture({
         config: {
           future: {
             unstable_singleFetch: true,
@@ -1774,50 +1932,255 @@ test.describe("single-fetch", () => {
           }
         `,
         },
-      },
-      ServerMode.Development
-    );
-    let appFixture = await createAppFixture(fixture, ServerMode.Development);
-    let app = new PlaywrightFixture(appFixture, page);
+      });
+      let appFixture = await createAppFixture(fixture);
+      let app = new PlaywrightFixture(appFixture, page);
 
-    let urls: string[] = [];
-    page.on("request", (req) => {
-      if (req.url().includes(".data")) {
-        urls.push(req.url());
-      }
-    });
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
 
-    await app.goto("/");
+      await app.goto("/");
 
-    await app.clickLink("/parent/a");
-    await page.waitForSelector("#a");
-    expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
-    expect(await app.getHtml("#a")).toContain("A Count: 1");
-    expect(urls).toEqual([expect.stringMatching(/\/parent\/a\.data$/)]);
-    urls = [];
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 1");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/parent/a.data")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/b");
+      await page.waitForSelector("#b");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 2");
+      expect(await app.getHtml("#b")).toContain("B Count: 1");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/parent/b.data")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 3");
+      expect(await app.getHtml("#a")).toContain("A Count: 2");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/parent/a.data")).toBe(true);
+    });
+
+    test("allows reused routes to opt out via shouldRevalidate", async ({
+      page,
+    }) => {
+      let fixture = await createFixture({
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
+        },
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
+            import { Link } from '@remix-run/react';
+            export default function Component() {
+              return <Link to="/parent/a">Go to /parent/a</Link>;
+            }
+          `,
+          "app/routes/parent.tsx": js`
+            import { Link, Outlet, useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export function shouldRevalidate() {
+              return false;
+            }
+            export default function Component() {
+              return (
+                <>
+                  <p id="parent">Parent Count: {useLoaderData().count}</p>
+                  <Link to="/parent/a">Go to /parent/a</Link>
+                  <Link to="/parent/b">Go to /parent/b</Link>
+                  <Outlet/>
+                </>
+              );
+            }
+          `,
+          "app/routes/parent.a.tsx": js`
+            import { useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <p id="a">A Count: {useLoaderData().count}</p>;
+            }
+          `,
+          "app/routes/parent.b.tsx": js`
+            import { useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <p id="b">B Count: {useLoaderData().count}</p>;
+            }
+          `,
+        },
+      });
+      let appFixture = await createAppFixture(fixture);
+      let app = new PlaywrightFixture(appFixture, page);
 
-    await app.clickLink("/parent/b");
-    await page.waitForSelector("#b");
-    expect(await app.getHtml("#parent")).toContain("Parent Count: 2");
-    expect(await app.getHtml("#b")).toContain("B Count: 1");
-    expect(urls).toEqual([expect.stringMatching(/\/parent\/b\.data$/)]);
-    urls = [];
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
 
-    await app.clickLink("/parent/a");
-    await page.waitForSelector("#a");
-    expect(await app.getHtml("#parent")).toContain("Parent Count: 3");
-    expect(await app.getHtml("#a")).toContain("A Count: 2");
-    expect(urls).toEqual([expect.stringMatching(/\/parent\/a\.data$/)]);
-  });
+      await app.goto("/");
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 1");
+      expect(urls.length).toBe(1);
+      // Not a revalidation on the first navigation so no params
+      expect(urls[0].endsWith("/parent/a.data")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/b");
+      await page.waitForSelector("#b");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#b")).toContain("B Count: 1");
+      expect(urls.length).toBe(1);
+      // Don't reload the parent route
+      expect(
+        urls[0].endsWith("/parent/b.data?_routes=root%2Croutes%2Fparent.b")
+      ).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 2");
+      expect(urls.length).toBe(1);
+      // Don't reload the parent route
+      expect(
+        urls[0].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent.a")
+      ).toBe(true);
+    });
+
+    test("allows reused routes to opt out via shouldRevalidate (w/clientLoader)", async ({
+      page,
+    }) => {
+      let fixture = await createFixture({
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
+        },
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
+            import { Link } from '@remix-run/react';
+            export default function Component() {
+              return <Link to="/parent/a">Go to /parent/a</Link>;
+            }
+          `,
+          "app/routes/parent.tsx": js`
+            import { Link, Outlet, useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export function shouldRevalidate() {
+              return false;
+            }
+            export function clientLoader({ serverLoader }) {
+              return serverLoader()
+            }
+            export default function Component() {
+              return (
+                <>
+                  <p id="parent">Parent Count: {useLoaderData().count}</p>
+                  <Link to="/parent/a">Go to /parent/a</Link>
+                  <Link to="/parent/b">Go to /parent/b</Link>
+                  <Outlet/>
+                </>
+              );
+            }
+          `,
+          "app/routes/parent.a.tsx": js`
+            import { useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <p id="a">A Count: {useLoaderData().count}</p>;
+            }
+          `,
+          "app/routes/parent.b.tsx": js`
+            import { useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <p id="b">B Count: {useLoaderData().count}</p>;
+            }
+          `,
+        },
+      });
+      let appFixture = await createAppFixture(fixture);
+      let app = new PlaywrightFixture(appFixture, page);
 
-  // Calls reused parent route by default
-  // Does not call reused parent route if it opts out via shouldRevalidate
-  // Does not call reused parent route if it opts out via shouldRevalidate and has a clientLoader
-  // Action requests do not use ?_routes and do not run loaders on the server
-  // Fetcher loaders call singular routes
-  // Fetcher actions call .data routes without a param
-  // Fetcher loads revalidate by default
-  // Fetcher loaders can opt out of revalidation via shouldRevalidate
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      await app.goto("/");
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 1");
+      expect(urls.length).toBe(2);
+      // Client loader triggers 2 requests on the first navigation
+      expect(urls[0].endsWith("/parent/a.data?_routes=routes%2Fparent")).toBe(
+        true
+      );
+      expect(
+        urls[1].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent.a")
+      ).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/b");
+      await page.waitForSelector("#b");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#b")).toContain("B Count: 1");
+      expect(urls.length).toBe(1);
+      // Don't reload the parent route
+      expect(
+        urls[0].endsWith("/parent/b.data?_routes=root%2Croutes%2Fparent.b")
+      ).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 2");
+      expect(urls.length).toBe(1);
+      // Don't reload the parent route
+      expect(
+        urls[0].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent.a")
+      ).toBe(true);
+    });
+  });
 
   test.describe("client loaders", () => {
     test("when no routes have client loaders", async ({ page }) => {
@@ -2229,6 +2592,13 @@ test.describe("single-fetch", () => {
     });
   });
 
+  test.describe("fetchers", () => {
+    // Fetcher loaders call singular routes
+    // Fetcher actions call .data routes without a param
+    // Fetcher loads revalidate by default
+    // Fetcher loaders can opt out of revalidation via shouldRevalidate
+  });
+
   test.describe("prefetching", () => {
     test("when no routes have client loaders", async ({ page }) => {
       let fixture = await createFixture(
diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts
index e4c0ffc4e6b..afdda2ac145 100644
--- a/packages/remix-server-runtime/data.ts
+++ b/packages/remix-server-runtime/data.ts
@@ -43,7 +43,9 @@ export async function callRouteAction({
   singleFetch: boolean;
 }) {
   let result = await action({
-    request: stripDataParam(stripIndexParam(request)),
+    request: singleFetch
+      ? stripRoutesParam(stripIndexParam(request))
+      : stripDataParam(stripIndexParam(request)),
     context: loadContext,
     params,
   });
@@ -79,7 +81,9 @@ export async function callRouteLoader({
   singleFetch: boolean;
 }) {
   let result = await loader({
-    request: stripDataParam(stripIndexParam(request)),
+    request: singleFetch
+      ? stripRoutesParam(stripIndexParam(request))
+      : stripDataParam(stripIndexParam(request)),
     context: loadContext,
     params,
   });
@@ -158,3 +162,20 @@ function stripDataParam(request: Request) {
 
   return new Request(url.href, init);
 }
+
+function stripRoutesParam(request: Request) {
+  let url = new URL(request.url);
+  url.searchParams.delete("_routes");
+  let init: RequestInit = {
+    method: request.method,
+    body: request.body,
+    headers: request.headers,
+    signal: request.signal,
+  };
+
+  if (init.body) {
+    (init as { duplex: "half" }).duplex = "half";
+  }
+
+  return new Request(url.href, init);
+}

From d32cc7ca7101e6630a2b85ab0615ee227c55faeb Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 28 Aug 2024 17:31:57 -0400
Subject: [PATCH 3/7] Updates and more e2e tests

---
 integration/single-fetch-test.ts      | 436 +++++++++++++++++++++++++-
 packages/remix-react/single-fetch.tsx |  90 +++++-
 2 files changed, 504 insertions(+), 22 deletions(-)

diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts
index 655ac24aff5..57758e2bfb8 100644
--- a/integration/single-fetch-test.ts
+++ b/integration/single-fetch-test.ts
@@ -629,6 +629,7 @@ test.describe("single-fetch", () => {
     await page.waitForSelector("#error");
     expect(urls).toEqual([]);
   });
+
   test("returns headers correctly for singular loader and action calls", async () => {
     let fixture = await createFixture({
       config: {
@@ -1758,7 +1759,6 @@ test.describe("single-fetch", () => {
             return { url: request.url };
           }
           export async function clientLoader({ request, serverLoader }) {
-            debugger;
             let serverData = await serverLoader();
             return {
               serverUrl: serverData.url,
@@ -1878,7 +1878,72 @@ test.describe("single-fetch", () => {
     ]);
   });
 
-  test.describe("revalidations", () => {
+  test.describe("revalidations/_routes param", () => {
+    test("does not make a server call if no loaders need to run", async ({
+      page,
+    }) => {
+      let fixture = await createFixture(
+        {
+          config: {
+            future: {
+              unstable_singleFetch: true,
+            },
+          },
+          files: {
+            "app/root.tsx": js`
+            import { Link, Links, Meta, Outlet, Scripts } from "@remix-run/react";
+
+            export default function Root() {
+              return (
+                <html lang="en">
+                  <head>
+                    <Meta />
+                    <Links />
+                  </head>
+                  <body>
+                    <Link to="/">Home</Link><br/>
+                    <Link to="/a/b">/a/b</Link><br/>
+                    <Outlet />
+                    <Scripts />
+                  </body>
+                </html>
+              );
+            }
+          `,
+            "app/routes/a.tsx": js`
+            import { Outlet } from "@remix-run/react";
+
+            export default function Root() {
+              return <Outlet />;
+            }
+          `,
+            "app/routes/a.b.tsx": js`
+            export default function Root() {
+              return <h1>B</h1>;
+            }
+          `,
+          },
+        },
+        ServerMode.Development
+      );
+
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.method() === "GET" && req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      let appFixture = await createAppFixture(fixture, ServerMode.Development);
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/");
+
+      await app.clickLink("/a/b");
+      await page.waitForSelector("h1");
+      expect(await app.getHtml("h1")).toBe("<h1>B</h1>");
+      expect(urls.length).toBe(0);
+    });
+
     test("calls reused parent routes by default", async ({ page }) => {
       let fixture = await createFixture({
         config: {
@@ -2180,6 +2245,91 @@ test.describe("single-fetch", () => {
         urls[0].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent.a")
       ).toBe(true);
     });
+
+    test("does not add a _routes param for routes without loaders", async ({
+      page,
+    }) => {
+      let fixture = await createFixture({
+        config: {
+          future: {
+            unstable_singleFetch: true,
+          },
+        },
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
+            import { Link } from '@remix-run/react';
+            export default function Component() {
+              return <Link to="/parent/a">Go to /parent/a</Link>;
+            }
+          `,
+          "app/routes/parent.tsx": js`
+            import { Link, Outlet, useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export function shouldRevalidate() {
+              return false;
+            }
+            export default function Component() {
+              return (
+                <>
+                  <p id="parent">Parent Count: {useLoaderData().count}</p>
+                  <Link to="/parent/a">Go to /parent/a</Link>
+                  <Link to="/parent/b">Go to /parent/b</Link>
+                  <Outlet/>
+                </>
+              );
+            }
+          `,
+          "app/routes/parent.a.tsx": js`
+            import { useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <p id="a">A Count: {useLoaderData().count}</p>;
+            }
+          `,
+          "app/routes/parent.b.tsx": js`
+            export default function Component() {
+              return <p id="b">B</p>;
+            }
+          `,
+        },
+      });
+      let appFixture = await createAppFixture(fixture);
+      let app = new PlaywrightFixture(appFixture, page);
+
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      await app.goto("/");
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 1");
+      expect(urls.length).toBe(1);
+      // Not a revalidation on the first navigation so no params
+      expect(urls[0].endsWith("/parent/a.data")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/b");
+      await page.waitForSelector("#b");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#b")).toContain("B");
+      expect(urls.length).toBe(1);
+      // Don't reload the parent route
+      expect(urls[0].endsWith("/parent/b.data?_routes=root")).toBe(true);
+      urls = [];
+    });
   });
 
   test.describe("client loaders", () => {
@@ -2593,10 +2743,284 @@ test.describe("single-fetch", () => {
   });
 
   test.describe("fetchers", () => {
-    // Fetcher loaders call singular routes
-    // Fetcher actions call .data routes without a param
-    // Fetcher loads revalidate by default
-    // Fetcher loaders can opt out of revalidation via shouldRevalidate
+    test("Fetcher loaders call singular routes", async ({ page }) => {
+      let fixture = await createFixture(
+        {
+          config: {
+            future: {
+              unstable_singleFetch: true,
+            },
+          },
+          files: {
+            ...files,
+            "app/routes/a.tsx": js`
+              import { Outlet } from '@remix-run/react';
+              export default function Comp() {
+                return <Outlet />;
+              }
+            `,
+            "app/routes/a.b.tsx": js`
+              import { useFetcher } from '@remix-run/react';
+
+              export function loader() {
+                return { message: 'LOADER' };
+              }
+
+              export default function Comp() {
+                let fetcher = useFetcher();
+                return (
+                  <>
+                    <button id="load" onClick={() => fetcher.load('/a/b')}>Load</button>
+                    {fetcher.data ? <p id="data">{fetcher.data.message}</p> : null}
+                  </>
+                );
+              }
+            `,
+          },
+        },
+        ServerMode.Development
+      );
+
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.method() === "GET" && req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      let appFixture = await createAppFixture(fixture, ServerMode.Development);
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/a/b");
+      await app.clickElement("#load");
+      await page.waitForSelector("#data");
+      expect(await app.getHtml("#data")).toContain("LOADER");
+
+      // No clientLoaders so we can make a single parameter-less fetch
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/a/b.data?_routes=routes%2Fa.b")).toBe(true);
+    });
+
+    test("Fetcher actions call singular routes", async ({ page }) => {
+      let fixture = await createFixture(
+        {
+          config: {
+            future: {
+              unstable_singleFetch: true,
+            },
+          },
+          files: {
+            ...files,
+            "app/routes/a.tsx": js`
+              import { Outlet } from '@remix-run/react';
+              export default function Comp() {
+                return <Outlet />;
+              }
+            `,
+            "app/routes/a.b.tsx": js`
+              import { useFetcher } from '@remix-run/react';
+
+              export function action() {
+                return { message: 'ACTION' };
+              }
+
+              export default function Comp() {
+                let fetcher = useFetcher();
+                return (
+                  <>
+                    <button id="submit" onClick={() => {
+                      fetcher.submit({}, {
+                        method: 'post',
+                        action: '/a/b'
+                      });
+                    }}>
+                      Load
+                    </button>
+                    {fetcher.data ? <p id="data">{fetcher.data.message}</p> : null}
+                  </>
+                );
+              }
+            `,
+          },
+        },
+        ServerMode.Development
+      );
+
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.method() === "GET" && req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      let appFixture = await createAppFixture(fixture, ServerMode.Development);
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/a/b");
+      await app.clickElement("#submit");
+      await page.waitForSelector("#data");
+      expect(await app.getHtml("#data")).toContain("ACTION");
+
+      // No clientLoaders so we can make a single parameter-less fetch
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/a/b.data")).toBe(true);
+    });
+
+    test("Fetcher loads do not revalidate on GET navigations by default", async ({
+      page,
+    }) => {
+      let fixture = await createFixture(
+        {
+          config: {
+            future: {
+              unstable_singleFetch: true,
+            },
+          },
+          files: {
+            ...files,
+            "app/routes/parent.tsx": js`
+            import { Link, Outlet, useFetcher } from '@remix-run/react';
+            export default function Component() {
+              let fetcher = useFetcher();
+              return (
+                <>
+                  <Link to="/parent/a">Go to /parent/a</Link>
+                  <Link to="/parent/b">Go to /parent/b</Link>
+                  <button id="load" onClick={() => fetcher.load('/fetch')}>
+                    Load Fetcher
+                  </button>
+                  {fetcher.data ? <p id="fetch">Fetch Count: {fetcher.data.count}</p> : null}
+                  <Outlet/>
+                </>
+              );
+            }
+          `,
+            "app/routes/parent.a.tsx": js`
+            export default function Component() {
+              return <p id="a">A</p>;
+            }
+          `,
+            "app/routes/parent.b.tsx": js`
+            export default function Component() {
+              return <p id="b">B</p>;
+            }
+          `,
+            "app/routes/fetch.tsx": js`
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <h1>Fetch</h1>;
+            }
+          `,
+          },
+        },
+        ServerMode.Development
+      );
+      let appFixture = await createAppFixture(fixture, ServerMode.Development);
+      let app = new PlaywrightFixture(appFixture, page);
+
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      await app.goto("/parent/a");
+      await app.clickElement("#load");
+      await page.waitForSelector("#fetch");
+      expect(await app.getHtml("#fetch")).toContain("Fetch Count: 1");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/fetch.data?_routes=routes%2Ffetch")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/b");
+      await page.waitForSelector("#b");
+      expect(await app.getHtml("#fetch")).toContain("Fetch Count: 1");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/parent/b.data")).toBe(true);
+    });
+
+    test("Fetcher loads can opt into revalidation on GET navigations", async ({
+      page,
+    }) => {
+      let fixture = await createFixture(
+        {
+          config: {
+            future: {
+              unstable_singleFetch: true,
+            },
+          },
+          files: {
+            ...files,
+            "app/routes/parent.tsx": js`
+            import { Link, Outlet, useFetcher } from '@remix-run/react';
+            export default function Component() {
+              let fetcher = useFetcher();
+              return (
+                <>
+                  <Link to="/parent/a">Go to /parent/a</Link>
+                  <Link to="/parent/b">Go to /parent/b</Link>
+                  <button id="load" onClick={() => fetcher.load('/fetch')}>
+                    Load Fetcher
+                  </button>
+                  {fetcher.data ? <p id="fetch">Fetch Count: {fetcher.data.count}</p> : null}
+                  <Outlet/>
+                </>
+              );
+            }
+          `,
+            "app/routes/parent.a.tsx": js`
+            export default function Component() {
+              return <p id="a">A</p>;
+            }
+          `,
+            "app/routes/parent.b.tsx": js`
+            export default function Component() {
+              return <p id="b">B</p>;
+            }
+          `,
+            "app/routes/fetch.tsx": js`
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export function shouldRevalidate() {
+              return true;
+            }
+            export default function Component() {
+              return <h1>Fetch</h1>;
+            }
+          `,
+          },
+        },
+        ServerMode.Development
+      );
+      let appFixture = await createAppFixture(fixture, ServerMode.Development);
+      let app = new PlaywrightFixture(appFixture, page);
+
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      await app.goto("/parent/a");
+      await app.clickElement("#load");
+      await page.waitForSelector("#fetch");
+      expect(await app.getHtml("#fetch")).toContain("Fetch Count: 1");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/fetch.data?_routes=routes%2Ffetch")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/b");
+      await page.waitForSelector("#b");
+      expect(await app.getHtml("#fetch")).toContain("Fetch Count: 2");
+      expect(urls.length).toBe(2);
+      expect(urls[0].endsWith("/fetch.data?_routes=routes%2Ffetch")).toBe(true);
+      expect(urls[1].endsWith("/parent/b.data")).toBe(true);
+    });
   });
 
   test.describe("prefetching", () => {
diff --git a/packages/remix-react/single-fetch.tsx b/packages/remix-react/single-fetch.tsx
index 8b27cf931cd..04a819be607 100644
--- a/packages/remix-react/single-fetch.tsx
+++ b/packages/remix-react/single-fetch.tsx
@@ -4,6 +4,7 @@ import type {
   LoaderFunctionArgs as RRLoaderArgs,
   unstable_DataStrategyFunction as DataStrategyFunction,
   unstable_HandlerResult as HandlerResult,
+  unstable_DataStrategyMatch,
 } from "@remix-run/router";
 import {
   UNSAFE_ErrorResponseImpl as ErrorResponseImpl,
@@ -143,19 +144,24 @@ export function getSingleFetchDataStrategy(
   manifest: AssetsManifest,
   routeModules: RouteModules
 ): DataStrategyFunction {
-  return async ({ request, matches, fetcherKey }) =>
-    request.method !== "GET"
-      ? singleFetchActionStrategy(request, matches)
-      : singleFetchLoaderStrategy(
-          manifest,
-          routeModules,
-          request,
-          matches,
-          fetcherKey
-        );
+  return async ({ request, matches, fetcherKey }) => {
+    if (request.method !== "GET") {
+      return singleFetchActionStrategy(request, matches);
+    }
+    if (fetcherKey) {
+      return singleFetchLoaderFetcherStrategy(request, matches);
+    }
+    return singleFetchLoaderNavigationStrategy(
+      manifest,
+      routeModules,
+      request,
+      matches
+    );
+  };
 }
 
-// Actions are simple since they're singular calls to the server
+// Actions are simple since they're singular calls to the server for both
+// navigations and fetchers)
 function singleFetchActionStrategy(
   request: Request,
   matches: DataStrategyFunctionArgs["matches"]
@@ -193,12 +199,11 @@ function singleFetchActionStrategy(
 
 // Loaders are trickier since we only want to hit the server once, so we
 // create a singular promise for all server-loader routes to latch onto.
-async function singleFetchLoaderStrategy(
+async function singleFetchLoaderNavigationStrategy(
   manifest: AssetsManifest,
   routeModules: RouteModules,
   request: Request,
-  matches: DataStrategyFunctionArgs["matches"],
-  fetcherKey: string | null
+  matches: DataStrategyFunctionArgs["matches"]
 ) {
   let optOutRoutes = new Set<string>();
   let url = stripIndexParam(singleFetchUrl(request.url));
@@ -232,7 +237,7 @@ async function singleFetchLoaderStrategy(
         }
 
         // When a route has a client loader, it calls it's singular server loader
-        if (fetcherKey || manifest.routes[m.route.id].hasClientLoader) {
+        if (manifest.routes[m.route.id].hasClientLoader) {
           optOutRoutes.add(m.route.id);
           result = await handler(async () => {
             let clientLoaderUrl = new URL(url);
@@ -255,11 +260,22 @@ async function singleFetchLoaderStrategy(
         // Wait to let other routes fill out optOutRoutes before creating the
         // _routes param
         await routesLoadedPromise;
+        if (optOutRoutes.size === matches.length) {
+          return { type: "data", result: m.data };
+        }
+
         if (optOutRoutes.size > 0) {
+          // When one or more routes have opted out, we add a _routes param to
+          // limit the loaders to those that have a server loader and did not
+          // opt out
           url.searchParams.set(
             "_routes",
             matches
-              .filter((m) => !optOutRoutes.has(m.route.id))
+              .filter(
+                (m) =>
+                  !optOutRoutes.has(m.route.id) &&
+                  manifest.routes[m.route.id].hasLoader
+              )
               .map((m) => m.route.id)
               .join(",")
           );
@@ -282,9 +298,51 @@ async function singleFetchLoaderStrategy(
       })
     )
   );
+
+  return results;
+}
+
+// Fetcher loader calls are much simpler than navigational loader calls
+async function singleFetchLoaderFetcherStrategy(
+  request: Request,
+  matches: DataStrategyFunctionArgs["matches"]
+) {
+  let url = stripIndexParam(singleFetchUrl(request.url));
+  let init = await createRequestInit(request);
+
+  let results = await Promise.all(
+    matches.map(async (m, i) =>
+      m.resolve(async (handler): Promise<HandlerResult> => {
+        if (!m.shouldLoad) {
+          return { type: "data", result: m.data };
+        }
+        return {
+          type: "data",
+          result: await fetchSingleLoader(handler, url, init, m.route.id),
+        };
+      })
+    )
+  );
+
   return results;
 }
 
+function fetchSingleLoader(
+  handler: Parameters<
+    NonNullable<Parameters<unstable_DataStrategyMatch["resolve"]>[0]>
+  >[0],
+  url: URL,
+  init: RequestInit,
+  routeId: string
+) {
+  return handler(async () => {
+    let singleLoaderUrl = new URL(url);
+    singleLoaderUrl.searchParams.set("_routes", routeId);
+    let { data } = await fetchAndDecode(singleLoaderUrl, init);
+    return unwrapSingleFetchResults(data as SingleFetchResults, routeId);
+  });
+}
+
 function stripIndexParam(url: URL) {
   let indexValues = url.searchParams.getAll("index");
   url.searchParams.delete("index");

From fef142696e838ee0ffacf910595783a2af29cbf5 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 4 Sep 2024 11:05:29 -0400
Subject: [PATCH 4/7] Bump router to prerelease

---
 integration/package.json                   |  2 +-
 packages/remix-dev/package.json            |  2 +-
 packages/remix-react/package.json          |  6 +--
 packages/remix-server-runtime/package.json |  2 +-
 packages/remix-testing/package.json        |  4 +-
 pnpm-lock.yaml                             | 50 +++++++++++-----------
 6 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/integration/package.json b/integration/package.json
index 0c7613c1bba..90c677e4844 100644
--- a/integration/package.json
+++ b/integration/package.json
@@ -14,7 +14,7 @@
     "@remix-run/dev": "workspace:*",
     "@remix-run/express": "workspace:*",
     "@remix-run/node": "workspace:*",
-    "@remix-run/router": "1.19.1",
+    "@remix-run/router": "1.19.2-pre.0",
     "@remix-run/server-runtime": "workspace:*",
     "@types/express": "^4.17.9",
     "@vanilla-extract/css": "^1.10.0",
diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json
index 83ce64325f6..8e3e6db0d0c 100644
--- a/packages/remix-dev/package.json
+++ b/packages/remix-dev/package.json
@@ -32,7 +32,7 @@
     "@mdx-js/mdx": "^2.3.0",
     "@npmcli/package-json": "^4.0.1",
     "@remix-run/node": "workspace:*",
-    "@remix-run/router": "1.19.1",
+    "@remix-run/router": "1.19.2-pre.0",
     "@remix-run/server-runtime": "workspace:*",
     "@types/mdx": "^2.0.5",
     "@vanilla-extract/integration": "^6.2.0",
diff --git a/packages/remix-react/package.json b/packages/remix-react/package.json
index 3cc3240cea1..de926fb1192 100644
--- a/packages/remix-react/package.json
+++ b/packages/remix-react/package.json
@@ -19,10 +19,10 @@
     "tsc": "tsc"
   },
   "dependencies": {
-    "@remix-run/router": "1.19.1",
+    "@remix-run/router": "1.19.2-pre.0",
     "@remix-run/server-runtime": "workspace:*",
-    "react-router": "6.26.1",
-    "react-router-dom": "6.26.1",
+    "react-router": "6.26.2-pre.0",
+    "react-router-dom": "6.26.2-pre.0",
     "turbo-stream": "2.3.0"
   },
   "devDependencies": {
diff --git a/packages/remix-server-runtime/package.json b/packages/remix-server-runtime/package.json
index 47fd5cbc006..4fbe6e70a5b 100644
--- a/packages/remix-server-runtime/package.json
+++ b/packages/remix-server-runtime/package.json
@@ -19,7 +19,7 @@
     "tsc": "tsc"
   },
   "dependencies": {
-    "@remix-run/router": "1.19.1",
+    "@remix-run/router": "1.19.2-pre.0",
     "@types/cookie": "^0.6.0",
     "@web3-storage/multipart-parser": "^1.0.0",
     "cookie": "^0.6.0",
diff --git a/packages/remix-testing/package.json b/packages/remix-testing/package.json
index f1c89a10a75..7929a470d07 100644
--- a/packages/remix-testing/package.json
+++ b/packages/remix-testing/package.json
@@ -21,8 +21,8 @@
   "dependencies": {
     "@remix-run/node": "workspace:*",
     "@remix-run/react": "workspace:*",
-    "@remix-run/router": "1.19.1",
-    "react-router-dom": "6.26.1"
+    "@remix-run/router": "1.19.2-pre.0",
+    "react-router-dom": "6.26.2-pre.0"
   },
   "devDependencies": {
     "@remix-run/server-runtime": "workspace:*",
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index d2ae31ecf59..80af613cc4d 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -323,8 +323,8 @@ importers:
         specifier: workspace:*
         version: link:../packages/remix-node
       '@remix-run/router':
-        specifier: 1.19.1
-        version: 1.19.1
+        specifier: 1.19.2-pre.0
+        version: 1.19.2-pre.0
       '@remix-run/server-runtime':
         specifier: workspace:*
         version: link:../packages/remix-server-runtime
@@ -871,8 +871,8 @@ importers:
         specifier: ^2.11.2
         version: link:../remix-react
       '@remix-run/router':
-        specifier: 1.19.1
-        version: 1.19.1
+        specifier: 1.19.2-pre.0
+        version: 1.19.2-pre.0
       '@remix-run/server-runtime':
         specifier: workspace:*
         version: link:../remix-server-runtime
@@ -1217,17 +1217,17 @@ importers:
   packages/remix-react:
     dependencies:
       '@remix-run/router':
-        specifier: 1.19.1
-        version: 1.19.1
+        specifier: 1.19.2-pre.0
+        version: 1.19.2-pre.0
       '@remix-run/server-runtime':
         specifier: workspace:*
         version: link:../remix-server-runtime
       react-router:
-        specifier: 6.26.1
-        version: 6.26.1(react@18.2.0)
+        specifier: 6.26.2-pre.0
+        version: 6.26.2-pre.0(react@18.2.0)
       react-router-dom:
-        specifier: 6.26.1
-        version: 6.26.1(react-dom@18.2.0)(react@18.2.0)
+        specifier: 6.26.2-pre.0
+        version: 6.26.2-pre.0(react-dom@18.2.0)(react@18.2.0)
       turbo-stream:
         specifier: 2.3.0
         version: 2.3.0
@@ -1303,8 +1303,8 @@ importers:
   packages/remix-server-runtime:
     dependencies:
       '@remix-run/router':
-        specifier: 1.19.1
-        version: 1.19.1
+        specifier: 1.19.2-pre.0
+        version: 1.19.2-pre.0
       '@types/cookie':
         specifier: ^0.6.0
         version: 0.6.0
@@ -1340,11 +1340,11 @@ importers:
         specifier: workspace:*
         version: link:../remix-react
       '@remix-run/router':
-        specifier: 1.19.1
-        version: 1.19.1
+        specifier: 1.19.2-pre.0
+        version: 1.19.2-pre.0
       react-router-dom:
-        specifier: 6.26.1
-        version: 6.26.1(react-dom@18.2.0)(react@18.2.0)
+        specifier: 6.26.2-pre.0
+        version: 6.26.2-pre.0(react-dom@18.2.0)(react@18.2.0)
     devDependencies:
       '@remix-run/server-runtime':
         specifier: workspace:*
@@ -4201,8 +4201,8 @@ packages:
       - encoding
     dev: false
 
-  /@remix-run/router@1.19.1:
-    resolution: {integrity: sha512-S45oynt/WH19bHbIXjtli6QmwNYvaz+vtnubvNpNDvUOoA/OWh6j1OikIP3G+v5GHdxyC6EXoChG3HgYGEUfcg==}
+  /@remix-run/router@1.19.2-pre.0:
+    resolution: {integrity: sha512-6hY0ygbEysBa8BWmO2uJbOjll+1kCHjJTg5qqCH2T+VVkGTjiZFd/Veruj4smF9rZP2PmZoFxSpZbWDr+xVieA==}
     engines: {node: '>=14.0.0'}
     dev: false
 
@@ -12786,26 +12786,26 @@ packages:
     engines: {node: '>=0.10.0'}
     dev: false
 
-  /react-router-dom@6.26.1(react-dom@18.2.0)(react@18.2.0):
-    resolution: {integrity: sha512-veut7m41S1fLql4pLhxeSW3jlqs+4MtjRLj0xvuCEXsxusJCbs6I8yn9BxzzDX2XDgafrccY6hwjmd/bL54tFw==}
+  /react-router-dom@6.26.2-pre.0(react-dom@18.2.0)(react@18.2.0):
+    resolution: {integrity: sha512-oHlxmc/5SufjxHI96rT1xGII8eKRlIn8juA86YK/2OUbs4HnV+8khNjakJSzTyLx86RY/kc09lDrfpbCWspSpg==}
     engines: {node: '>=14.0.0'}
     peerDependencies:
       react: '>=16.8'
       react-dom: '>=16.8'
     dependencies:
-      '@remix-run/router': 1.19.1
+      '@remix-run/router': 1.19.2-pre.0
       react: 18.2.0
       react-dom: 18.2.0(react@18.2.0)
-      react-router: 6.26.1(react@18.2.0)
+      react-router: 6.26.2-pre.0(react@18.2.0)
     dev: false
 
-  /react-router@6.26.1(react@18.2.0):
-    resolution: {integrity: sha512-kIwJveZNwp7teQRI5QmwWo39A5bXRyqpH0COKKmPnyD2vBvDwgFXSqDUYtt1h+FEyfnE8eXr7oe0MxRzVwCcvQ==}
+  /react-router@6.26.2-pre.0(react@18.2.0):
+    resolution: {integrity: sha512-nO4Kv9HNXZyaVzKxRcmgDn0tVHEs2pNN3lrTEbxYupDa/6igNHnKOe1TXlc13D9FMRUpjlpGhd/Ork/kcQ1PVw==}
     engines: {node: '>=14.0.0'}
     peerDependencies:
       react: '>=16.8'
     dependencies:
-      '@remix-run/router': 1.19.1
+      '@remix-run/router': 1.19.2-pre.0
       react: 18.2.0
     dev: false
 

From b9f9077e3a2b36e334a43bf9c66d5a8f56464d7a Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 4 Sep 2024 16:59:16 -0400
Subject: [PATCH 5/7] Adopt router dataStrategy changes to fix revalidation
 issues

---
 packages/remix-react/browser.tsx              |   3 +-
 packages/remix-react/single-fetch.tsx         | 252 ++++++++++--------
 packages/remix-server-runtime/single-fetch.ts |  28 +-
 3 files changed, 157 insertions(+), 126 deletions(-)

diff --git a/packages/remix-react/browser.tsx b/packages/remix-react/browser.tsx
index 5595fc46d80..1ee298c23f0 100644
--- a/packages/remix-react/browser.tsx
+++ b/packages/remix-react/browser.tsx
@@ -305,7 +305,8 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement {
       unstable_dataStrategy: window.__remixContext.future.unstable_singleFetch
         ? getSingleFetchDataStrategy(
             window.__remixManifest,
-            window.__remixRouteModules
+            window.__remixRouteModules,
+            () => router
           )
         : undefined,
       unstable_patchRoutesOnNavigation: getPatchRoutesOnNavigationFunction(
diff --git a/packages/remix-react/single-fetch.tsx b/packages/remix-react/single-fetch.tsx
index 04a819be607..51486a51bde 100644
--- a/packages/remix-react/single-fetch.tsx
+++ b/packages/remix-react/single-fetch.tsx
@@ -3,8 +3,9 @@ import type {
   ActionFunctionArgs as RRActionArgs,
   LoaderFunctionArgs as RRLoaderArgs,
   unstable_DataStrategyFunction as DataStrategyFunction,
-  unstable_HandlerResult as HandlerResult,
+  unstable_DataStrategyResult as DataStrategyResult,
   unstable_DataStrategyMatch,
+  Router as RemixRouter,
 } from "@remix-run/router";
 import {
   UNSAFE_ErrorResponseImpl as ErrorResponseImpl,
@@ -142,7 +143,8 @@ export function StreamTransfer({
 
 export function getSingleFetchDataStrategy(
   manifest: AssetsManifest,
-  routeModules: RouteModules
+  routeModules: RouteModules,
+  getRouter: () => RemixRouter
 ): DataStrategyFunction {
   return async ({ request, matches, fetcherKey }) => {
     if (request.method !== "GET") {
@@ -154,6 +156,7 @@ export function getSingleFetchDataStrategy(
     return singleFetchLoaderNavigationStrategy(
       manifest,
       routeModules,
+      getRouter(),
       request,
       matches
     );
@@ -162,39 +165,39 @@ export function getSingleFetchDataStrategy(
 
 // Actions are simple since they're singular calls to the server for both
 // navigations and fetchers)
-function singleFetchActionStrategy(
+async function singleFetchActionStrategy(
   request: Request,
   matches: DataStrategyFunctionArgs["matches"]
 ) {
-  return Promise.all(
-    matches.map(async (m) => {
-      let actionStatus: number | undefined;
-      let result = await m.resolve(async (handler): Promise<HandlerResult> => {
-        if (!m.shouldLoad) {
-          return { type: "data", result: undefined };
-        }
-        let result = await handler(async () => {
-          let url = singleFetchUrl(request.url);
-          let init = await createRequestInit(request);
-          let { data, status } = await fetchAndDecode(url, init);
-          actionStatus = status;
-          return unwrapSingleFetchResult(data as SingleFetchResult, m.route.id);
-        });
-        return { type: "data", result };
-      });
+  let actionMatch = matches.find((m) => m.shouldLoad);
+  invariant(actionMatch, "No action match found");
+  let actionStatus: number | undefined = undefined;
+  let result = await actionMatch.resolve(async (handler) => {
+    let result = await handler(async () => {
+      let url = singleFetchUrl(request.url);
+      let init = await createRequestInit(request);
+      let { data, status } = await fetchAndDecode(url, init);
+      actionStatus = status;
+      return unwrapSingleFetchResult(
+        data as SingleFetchResult,
+        actionMatch!.route.id
+      );
+    });
+    return result;
+  });
 
-      if (isResponse(result.result) || isRouteErrorResponse(result.result)) {
-        return result;
-      }
-
-      // For non-responses, proxy along the statusCode via unstable_data()
-      // (most notably for skipping action error revalidation)
-      return {
-        type: result.type,
-        result: unstable_data(result.result, actionStatus),
-      };
-    })
-  );
+  if (isResponse(result.result) || isRouteErrorResponse(result.result)) {
+    return { [actionMatch.route.id]: result };
+  }
+
+  // For non-responses, proxy along the statusCode via unstable_data()
+  // (most notably for skipping action error revalidation)
+  return {
+    [actionMatch.route.id]: {
+      type: result.type,
+      result: unstable_data(result.result, actionStatus),
+    },
+  };
 }
 
 // Loaders are trickier since we only want to hit the server once, so we
@@ -202,23 +205,44 @@ function singleFetchActionStrategy(
 async function singleFetchLoaderNavigationStrategy(
   manifest: AssetsManifest,
   routeModules: RouteModules,
+  router: RemixRouter,
   request: Request,
   matches: DataStrategyFunctionArgs["matches"]
 ) {
-  let optOutRoutes = new Set<string>();
-  let url = stripIndexParam(singleFetchUrl(request.url));
-  let singleFetchPromise: Promise<SingleFetchResults> | undefined;
+  // Track which routes need a server load - in case we need to tack on a
+  // `_routes` param
+  let routesParams = new Set<string>();
 
-  let dfds = matches.map(() => createDeferred<void>());
-  let routesLoadedPromise = Promise.all(dfds.map((d) => d.promise));
+  // We only add `_routes` when one or more routes opts out of a load via
+  // `shouldRevalidate` or `clientLoader`
+  let foundOptOutRoute = false;
+
+  // Deferreds for each route so we can be sure they've all loaded via
+  // `match.resolve()`, and a singular promise that can tell us all routes
+  // have been resolved
+  let routeDfds = matches.map(() => createDeferred<void>());
+  let routesLoadedPromise = Promise.all(routeDfds.map((d) => d.promise));
+
+  // Deferred that we'll use for the call to the server that each match can
+  // await and parse out it's specific result
+  let singleFetchDfd = createDeferred<SingleFetchResults>();
+
+  // Base URL and RequestInit for calls to the server
+  let url = stripIndexParam(singleFetchUrl(request.url));
   let init = await createRequestInit(request);
 
-  let results = await Promise.all(
+  // We'll build up this results object as we loop through matches
+  let results: Record<string, DataStrategyResult> = {};
+
+  let resolvePromise = Promise.all(
     matches.map(async (m, i) =>
-      m.resolve(async (handler): Promise<HandlerResult> => {
-        let result: unknown;
+      m.resolve(async (handler) => {
+        routeDfds[i].resolve();
 
-        dfds[i].resolve();
+        // On initial load we respect `shouldLoad` for `clientLoader.hydrate` instances
+        if (!router.state.initialized && !m.shouldLoad) {
+          return;
+        }
 
         // Opt out if:
         //   We currently have data
@@ -227,78 +251,100 @@ async function singleFetchLoaderNavigationStrategy(
         //   and a shouldRevalidate function
         // This implies that the user opted out via shouldRevalidate()
         if (
-          m.data !== undefined &&
+          router.state.initialized &&
+          m.route.id in router.state.loaderData &&
           !m.shouldLoad &&
           manifest.routes[m.route.id].hasLoader &&
           routeModules[m.route.id]?.shouldRevalidate
         ) {
-          optOutRoutes.add(m.route.id);
-          return { type: "data", result: m.data };
+          foundOptOutRoute = true;
+          return;
         }
 
-        // When a route has a client loader, it calls it's singular server loader
+        // When a route has a client loader, it opts out of the singular call and
+        // calls it's server loader via `serverLoader()` using a `?_routes` param
         if (manifest.routes[m.route.id].hasClientLoader) {
-          optOutRoutes.add(m.route.id);
-          result = await handler(async () => {
-            let clientLoaderUrl = new URL(url);
-            clientLoaderUrl.searchParams.set("_routes", m.route.id);
-            let { data } = await fetchAndDecode(clientLoaderUrl, init);
-            return unwrapSingleFetchResults(
-              data as SingleFetchResults,
+          if (manifest.routes[m.route.id].hasLoader) {
+            foundOptOutRoute = true;
+          }
+          try {
+            let result = await fetchSingleLoader(
+              handler,
+              url,
+              init,
               m.route.id
             );
-          });
-          return {
+            results[m.route.id] = { type: "data", result };
+          } catch (e) {
+            results[m.route.id] = { type: "error", result: e };
+          }
+          return;
+        } else if (!manifest.routes[m.route.id].hasLoader) {
+          // If we don't have a server loader, then we don't care about the HTTP
+          // call and can just send back a `null` - because we _do_ have a `loader`
+          // in the client router handling route module/styles loads
+          results[m.route.id] = {
             type: "data",
-            result,
+            result: null,
           };
+          return;
         }
 
-        // Otherwise, we can lump this route with the others on a singular
-        // `singleFetchPromise`
-
-        // Wait to let other routes fill out optOutRoutes before creating the
-        // _routes param
-        await routesLoadedPromise;
-        if (optOutRoutes.size === matches.length) {
-          return { type: "data", result: m.data };
-        }
-
-        if (optOutRoutes.size > 0) {
-          // When one or more routes have opted out, we add a _routes param to
-          // limit the loaders to those that have a server loader and did not
-          // opt out
-          url.searchParams.set(
-            "_routes",
-            matches
-              .filter(
-                (m) =>
-                  !optOutRoutes.has(m.route.id) &&
-                  manifest.routes[m.route.id].hasLoader
-              )
-              .map((m) => m.route.id)
-              .join(",")
-          );
-        }
-
-        result = await handler(async () => {
-          if (!singleFetchPromise) {
-            singleFetchPromise = fetchAndDecode(url, init).then(
-              ({ data }) => data as SingleFetchResults
-            );
+        routesParams.add(m.route.id);
+
+        // Otherwise, we can lump this route with the others on a singular promise
+        await handler(async () => {
+          try {
+            let data = await singleFetchDfd.promise;
+            results[m.route.id] = {
+              type: "data",
+              result: unwrapSingleFetchResults(data, m.route.id),
+            };
+          } catch (e) {
+            results[m.route.id] = {
+              type: "error",
+              result: e,
+            };
           }
-          let results = await singleFetchPromise;
-          return unwrapSingleFetchResults(results, m.route.id);
         });
-
-        return {
-          type: "data",
-          result,
-        };
       })
     )
   );
 
+  // Wait for all routes to resolve above before we make the HTTP call
+  await routesLoadedPromise;
+
+  if (!router.state.initialized) {
+    // Don't make any single fetch server calls on initial hydration - only
+    // clientLoaders can pass through via `clientLoader.hydrate`
+    singleFetchDfd.resolve({});
+  } else if (routesParams.size === 0) {
+    // If no routes need to run on the server, then there's nothing to fetch
+    singleFetchDfd.resolve({});
+  } else {
+    if (routesParams.size > 0 && foundOptOutRoute) {
+      // When one or more routes have opted out, we add a _routes param to
+      // limit the loaders to those that have a server loader and did not
+      // opt out
+      url.searchParams.set(
+        "_routes",
+        matches
+          .filter((m) => routesParams.has(m.route.id))
+          .map((m) => m.route.id)
+          .join(",")
+      );
+    }
+
+    try {
+      let data = await fetchAndDecode(url, init);
+      singleFetchDfd.resolve(data.data as SingleFetchResults);
+    } catch (e) {
+      singleFetchDfd.reject(e as Error);
+    }
+  }
+
+  await resolvePromise;
+
   return results;
 }
 
@@ -309,22 +355,12 @@ async function singleFetchLoaderFetcherStrategy(
 ) {
   let url = stripIndexParam(singleFetchUrl(request.url));
   let init = await createRequestInit(request);
-
-  let results = await Promise.all(
-    matches.map(async (m, i) =>
-      m.resolve(async (handler): Promise<HandlerResult> => {
-        if (!m.shouldLoad) {
-          return { type: "data", result: m.data };
-        }
-        return {
-          type: "data",
-          result: await fetchSingleLoader(handler, url, init, m.route.id),
-        };
-      })
-    )
+  let fetcherMatch = matches.find((m) => m.shouldLoad);
+  invariant(fetcherMatch, "No fetcher match found");
+  let result = await fetcherMatch.resolve(async (handler) =>
+    fetchSingleLoader(handler, url, init, fetcherMatch!.route.id)
   );
-
-  return results;
+  return { [fetcherMatch.route.id]: result };
 }
 
 function fetchSingleLoader(
diff --git a/packages/remix-server-runtime/single-fetch.ts b/packages/remix-server-runtime/single-fetch.ts
index b1c888f033a..a014a8f61cf 100644
--- a/packages/remix-server-runtime/single-fetch.ts
+++ b/packages/remix-server-runtime/single-fetch.ts
@@ -58,27 +58,21 @@ export function getSingleFetchDataStrategy({
   return async ({ request, matches }: DataStrategyFunctionArgs) => {
     // Don't call loaders on action data requests
     if (isActionDataRequest && request.method === "GET") {
-      return await Promise.all(
-        matches.map((m) =>
-          m.resolve(async () => ({ type: "data", result: null }))
-        )
-      );
+      return {};
     }
 
+    // Only run opt-in loaders when fine-grained revalidation is enabled
+    let matchesToLoad = loadRouteIds
+      ? matches.filter((m) => loadRouteIds.includes(m.route.id))
+      : matches;
     let results = await Promise.all(
-      matches.map(async (match) => {
-        let result = await match.resolve(async (handler) => {
-          // Only run opt-in loaders when fine-grained revalidation is enabled
-          let data =
-            loadRouteIds && !loadRouteIds.includes(match.route.id)
-              ? null
-              : await handler();
-          return { type: "data", result: data };
-        });
-        return result;
-      })
+      matchesToLoad.map((match) => match.resolve())
+    );
+    return results.reduce(
+      (acc, result, i) =>
+        Object.assign(acc, { [matchesToLoad[i].route.id]: result }),
+      {}
     );
-    return results;
   };
 }
 

From 6715b40c7ee10564ab05eb64c073b3796f118b5d Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 4 Sep 2024 18:03:02 -0400
Subject: [PATCH 6/7] Fix test, update docs and chanbgeset

---
 .changeset/flat-wasps-heal.md         | 14 ++++++++++++
 docs/guides/single-fetch.md           | 32 +++++++++++++++++++++++----
 integration/error-boundary-v2-test.ts |  2 +-
 3 files changed, 43 insertions(+), 5 deletions(-)
 create mode 100644 .changeset/flat-wasps-heal.md

diff --git a/.changeset/flat-wasps-heal.md b/.changeset/flat-wasps-heal.md
new file mode 100644
index 00000000000..1fc73acc63d
--- /dev/null
+++ b/.changeset/flat-wasps-heal.md
@@ -0,0 +1,14 @@
+---
+"@remix-run/react": patch
+"@remix-run/server-runtime": patch
+---
+
+Single Fetch - fix revalidation behavior bugs
+
+  - With Single Fetch, existing routes revalidate by default
+  - This means requests do not need special query params for granular route revalidations out of the box - i.e., `GET /a/b/c.data`
+  - There are two conditions that will trigger granular revalidation:
+    - If a route opts out of revalidation via `shouldRevalidate`, it will be excluded from the single fetch call
+    - If a route defines a `clientLoader` then it will be excluded from the single fetch call and if you call `serverLoader()` from your `clientLoader`, that will make a separarte HTTP call for just that route loader - i.e., `GET /a/b/c.data?_routes=routes/a` for a `clientLoader` in `routes/a.tsx`
+  - When one or more routes are excluded from the single fetch call, the remaining routes that have loaders are included as query params:
+    - For example, if A was excluded, and the `root` route and `routes/b` had a `loader` but `routes/c` did not, the single fetch request would be `GET /a/b/c.data?_routes=root,routes/a`
diff --git a/docs/guides/single-fetch.md b/docs/guides/single-fetch.md
index cb6bfca8523..d52d7648e07 100644
--- a/docs/guides/single-fetch.md
+++ b/docs/guides/single-fetch.md
@@ -86,6 +86,7 @@ There are a handful of breaking changes introduced with Single Fetch - some of w
   - Add `@remix-run/react/future/single-fetch.d.ts` to the end of your `tsconfig.json`'s `compilerOptions.types` array
   - Begin using `unstable_defineLoader`/`unstable_defineAction` in your routes
     - This can be done incrementally - you should have _mostly_ accurate type inference in your current state
+- [**Default revalidation behavior changes to opt-out on GET navigations**][revalidation]: Default revalidation behavior on normal navigations changes from opt-in to opt-out and your server loaders will re-run by default
 - [**Opt-in `action` revalidation**][action-revalidation]: Revalidation after an `action` `4xx`/`5xx` `Response` is now opt-in, versus opt-out
 
 ## Adding a New Route with Single Fetch
@@ -429,9 +430,27 @@ function handleBrowserRequest(
 
 #### Normal Navigation Behavior
 
-for caching purposes we call `.data` and run all loaders
-the presence of a `shoulRevalidate` will opt-into granular revalidation
-and `childLoader` will also opt into granular revalidation
+In addition to the simpler mental model and the alignment of document and data requests, another benefit of Single Fetch is simpler (and hopefully better) caching behavior. Generally, Single Fetch will make fewer HTTP requests and hopefully cache those results more frequently compared to the previous multiple-fetch behavior.
+
+To reduce cache fragmentation, Single Fetch changes the default revalidation behavior on GET navigations. Previously, Remix would not re-run loaders for reused ancestor routes unless you opted-in via `shouldRevalidate`. Now, Remix _will_ re-run those by default in the simple case for a Single Fetch request like `GET /a/b/c.data`. If you do not have any `shouldRevalidate` or `clientLoader` functions, this will be the behavior for your app.
+
+Adding either a `shouldRevalidate` or a `clientLoader` to any of the active routes will trigger granular Single Fetch calls that include a `_routes` parameter specifying the subset of routes to run.
+
+If a `clientLoader` calls `serverLoader()` internally, that will trigger a separate HTTP call for that specific route, akin to the old behavior.
+
+For example, if you are on `/a/b` and you navigate to `/a/b/c`:
+
+- When no `shouldRevalidate` or `clientLoader` functions exist: `GET /a/b/c.data`
+- If all routes have loaders but `routes/a` opts out via `shouldRevalidate`:
+  - `GET /a/b/c.data?_routes=root,routes/b,routes/c`
+- If all routes have loaders but `routes/b` has a `clientLoader`:
+  - `GET /a/b/c.data?_routes=root,routes/a,routes/c`
+  - And then if B's `clientLoader` calls `serverLoader()`:
+    - `GET /a/b/c.data?_routes=routes/b`
+
+If this new behavior is sub-optimal for your application, you should be able to opt-back into the old behavior of not-revalidating by adding a `shouldRevalidate` that returns `false` in the desired scenarios to your parent routes.
+
+Another option is to leverage a server-side cache for expensive parent loader calculations.
 
 #### Submission Revalidation Behavior
 
@@ -466,9 +485,14 @@ Revalidation is handled via a `?_routes` query string parameter on the single fe
 [merging-remix-and-rr]: https://remix.run/blog/merging-remix-and-react-router
 [migration-guide]: #migrating-a-route-with-single-fetch
 [breaking-changes]: #breaking-changes
-[action-revalidation]: #streaming-data-format
+[revalidation]: #normal-navigation-behavior
+[action-revalidation]: #submission-revalidation-behavior
 [start]: #enabling-single-fetch
 [type-inference-section]: #type-inference
 [compatibility-flag]: https://developers.cloudflare.com/workers/configuration/compatibility-dates
 [data-utility]: ../utils/data
 [augment]: https://www.typescriptlang.org/docs/handbook/declaration-merging.html#module-augmentation
+
+```
+
+```
diff --git a/integration/error-boundary-v2-test.ts b/integration/error-boundary-v2-test.ts
index 3b4bab581c0..8d06573154e 100644
--- a/integration/error-boundary-v2-test.ts
+++ b/integration/error-boundary-v2-test.ts
@@ -416,7 +416,7 @@ test.describe("single fetch", () => {
         await waitForAndAssert(
           page,
           app,
-          "#child-error",
+          "#parent-error",
           "Unable to decode turbo-stream response from URL"
         );
       });

From 99a0e5d0d0e9c8cd0fc6c4f76b1c8786c81568e1 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Thu, 5 Sep 2024 09:32:37 -0400
Subject: [PATCH 7/7] polish

---
 packages/remix-react/single-fetch.tsx | 85 ++++++++++++++-------------
 1 file changed, 44 insertions(+), 41 deletions(-)

diff --git a/packages/remix-react/single-fetch.tsx b/packages/remix-react/single-fetch.tsx
index 5338714c3b4..e10da5f199f 100644
--- a/packages/remix-react/single-fetch.tsx
+++ b/packages/remix-react/single-fetch.tsx
@@ -125,12 +125,17 @@ export function getSingleFetchDataStrategy(
   getRouter: () => RemixRouter
 ): DataStrategyFunction {
   return async ({ request, matches, fetcherKey }) => {
+    // Actions are simple and behave the same for navigations and fetchers
     if (request.method !== "GET") {
       return singleFetchActionStrategy(request, matches);
     }
+
+    // Fetcher loads are singular calls to one loader
     if (fetcherKey) {
       return singleFetchLoaderFetcherStrategy(request, matches);
     }
+
+    // Navigational loads are more complex...
     return singleFetchLoaderNavigationStrategy(
       manifest,
       routeModules,
@@ -217,26 +222,25 @@ async function singleFetchLoaderNavigationStrategy(
       m.resolve(async (handler) => {
         routeDfds[i].resolve();
 
-        // On initial load we respect `shouldLoad` for `clientLoader.hydrate` instances
-        if (!router.state.initialized && !m.shouldLoad) {
-          return;
-        }
+        if (!m.shouldLoad) {
+          // If we're not yet initialized and this is the initial load, respect
+          // `shouldLoad` because we're only dealing with `clientLoader.hydrate`
+          // routes which will fall into the `clientLoader` section below.
+          if (!router.state.initialized) {
+            return;
+          }
 
-        // Opt out if:
-        //   We currently have data
-        //   and we were told not to load,
-        //   and we have a loader,
-        //   and a shouldRevalidate function
-        // This implies that the user opted out via shouldRevalidate()
-        if (
-          router.state.initialized &&
-          m.route.id in router.state.loaderData &&
-          !m.shouldLoad &&
-          manifest.routes[m.route.id].hasLoader &&
-          routeModules[m.route.id]?.shouldRevalidate
-        ) {
-          foundOptOutRoute = true;
-          return;
+          // Otherwise, we opt out if we currently have data, a `loader`, and a
+          // `shouldRevalidate` function.  This implies that the user opted out
+          // via `shouldRevalidate`
+          if (
+            m.route.id in router.state.loaderData &&
+            manifest.routes[m.route.id].hasLoader &&
+            routeModules[m.route.id]?.shouldRevalidate
+          ) {
+            foundOptOutRoute = true;
+            return;
+          }
         }
 
         // When a route has a client loader, it opts out of the singular call and
@@ -268,9 +272,10 @@ async function singleFetchLoaderNavigationStrategy(
           return;
         }
 
+        // Otherwise, we want to load this route on the server and can lump this
+        // it in with the others on a singular promise
         routesParams.add(m.route.id);
 
-        // Otherwise, we can lump this route with the others on a singular promise
         await handler(async () => {
           try {
             let data = await singleFetchDfd.promise;
@@ -292,28 +297,26 @@ async function singleFetchLoaderNavigationStrategy(
   // Wait for all routes to resolve above before we make the HTTP call
   await routesLoadedPromise;
 
-  if (!router.state.initialized) {
-    // Don't make any single fetch server calls on initial hydration - only
-    // clientLoaders can pass through via `clientLoader.hydrate`
-    singleFetchDfd.resolve({});
-  } else if (routesParams.size === 0) {
-    // If no routes need to run on the server, then there's nothing to fetch
+  // Don't make any single fetch server calls:
+  // - On initial hydration - only clientLoaders can pass through via `clientLoader.hydrate`
+  // - If there are no routes to fetch from the server
+  if (!router.state.initialized || routesParams.size === 0) {
     singleFetchDfd.resolve({});
   } else {
-    if (routesParams.size > 0 && foundOptOutRoute) {
+    try {
       // When one or more routes have opted out, we add a _routes param to
       // limit the loaders to those that have a server loader and did not
       // opt out
-      url.searchParams.set(
-        "_routes",
-        matches
-          .filter((m) => routesParams.has(m.route.id))
-          .map((m) => m.route.id)
-          .join(",")
-      );
-    }
+      if (foundOptOutRoute && routesParams.size > 0) {
+        url.searchParams.set(
+          "_routes",
+          matches
+            .filter((m) => routesParams.has(m.route.id))
+            .map((m) => m.route.id)
+            .join(",")
+        );
+      }
 
-    try {
       let data = await fetchAndDecode(url, init);
       singleFetchDfd.resolve(data.data as SingleFetchResults);
     } catch (e) {
@@ -331,13 +334,13 @@ async function singleFetchLoaderFetcherStrategy(
   request: Request,
   matches: DataStrategyFunctionArgs["matches"]
 ) {
-  let url = stripIndexParam(singleFetchUrl(request.url));
-  let init = await createRequestInit(request);
   let fetcherMatch = matches.find((m) => m.shouldLoad);
   invariant(fetcherMatch, "No fetcher match found");
-  let result = await fetcherMatch.resolve(async (handler) =>
-    fetchSingleLoader(handler, url, init, fetcherMatch!.route.id)
-  );
+  let result = await fetcherMatch.resolve(async (handler) => {
+    let url = stripIndexParam(singleFetchUrl(request.url));
+    let init = await createRequestInit(request);
+    return fetchSingleLoader(handler, url, init, fetcherMatch!.route.id);
+  });
   return { [fetcherMatch.route.id]: result };
 }