Fix shoulRevalidate behavior in clientLoader-only routes (#13221)

Author: brophdawg11

.changeset/three-oranges-sort.md

@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+Fix `shouldRevalidate` behavior for `clientLoader`-only routes in `ssr:true` apps

integration/single-fetch-test.ts

@@ -2200,6 +2200,97 @@ test.describe("single-fetch", () => {
       ).toBe(true);
     });
 
+    test("allows reused routes to opt out via shouldRevalidate (w/only clientLoader)", async ({
+      page,
+    }) => {
+      let fixture = await createFixture({
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
+            import { Link } from "react-router";
+            export default function Component() {
+              return <Link to="/parent/a">Go to /parent/a</Link>;
+            }
+          `,
+          "app/routes/parent.tsx": js`
+            import { Link, Outlet, useLoaderData } from "react-router";
+            let count = 0;
+            export function clientLoader({ 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 "react-router";
+            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 "react-router";
+            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);
+
+      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);
+      // Client loader triggers 2 requests on the first navigation
+      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);
+      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: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 2");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/parent/a.data")).toBe(true);
+    });
+
     test("provides the proper defaultShouldRevalidate value", async ({
       page,
     }) => {

packages/react-router/lib/dom/ssr/single-fetch.tsx

@@ -305,21 +305,6 @@ async function nonSsrStrategy(
   return results;
 }
 
-function isOptedOut(
-  manifestRoute: EntryRoute | undefined,
-  routeModule: RouteModule | undefined,
-  match: DataStrategyMatch,
-  router: DataRouter
-) {
-  return (
-    match.route.id in router.state.loaderData &&
-    manifestRoute &&
-    manifestRoute.hasLoader &&
-    routeModule &&
-    routeModule.shouldRevalidate
-  );
-}
-
 // 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 singleFetchLoaderNavigationStrategy(
@@ -374,11 +359,19 @@ async function singleFetchLoaderNavigationStrategy(
             return;
           }
 
-          // Otherwise, we opt out if we currently have data, a `loader`, and a
+          // Otherwise, we opt out if we currently have data and a
           // `shouldRevalidate` function.  This implies that the user opted out
           // via `shouldRevalidate`
-          if (isOptedOut(manifestRoute, routeModules[m.route.id], m, router)) {
-            foundOptOutRoute = true;
+          if (
+            m.route.id in router.state.loaderData &&
+            manifestRoute &&
+            m.route.shouldRevalidate
+          ) {
+            if (manifestRoute.hasLoader) {
+              // If we have a server loader, make sure we don't include it in the
+              // single fetch .data request
+              foundOptOutRoute = true;
+            }
             return;
           }
         }