Fix clientLoader calls to serverLoader for prerendered routes (#13047)

Author: brophdawg11

.changeset/orange-balloons-bathe.md

@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+[REMOVE] Fix prerender calls to serverLoader from clientLoader

integration/vite-prerender-test.ts

@@ -1,6 +1,7 @@
 import fs from "node:fs";
 import path from "node:path";
 import { PassThrough } from "node:stream";
+import type { Page } from "@playwright/test";
 import { test, expect } from "@playwright/test";
 
 import {
@@ -784,6 +785,20 @@ test.describe("Prerendering", () => {
   });
 
   test.describe("ssr: false", () => {
+    function captureRequests(page: Page) {
+      let requests: string[] = [];
+      page.on("request", (request) => {
+        let url = new URL(request.url());
+        if (
+          url.pathname.endsWith(".data") ||
+          url.pathname.endsWith("__manifest")
+        ) {
+          requests.push(url.pathname + url.search);
+        }
+      });
+      return requests;
+    }
+
     test("Errors on headers/action functions in any route", async () => {
       let cwd = await createProject({
         "react-router.config.ts": reactRouterConfig({
@@ -979,14 +994,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/");
       await page.waitForSelector("[data-mounted]");
@@ -1048,7 +1056,7 @@ test.describe("Prerendering", () => {
       );
     });
 
-    test("Properly navigates across SPA/prerender pages when starting from a SPA page", async ({
+    test("Navigates across SPA/prerender pages when starting from a SPA page", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -1138,14 +1146,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/", true);
       await page.waitForSelector('a[href="/page"]');
@@ -1194,7 +1195,7 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual(["/page.data", "/page.data"]);
     });
 
-    test("Properly navigates across SPA/prerender pages when starting from a prerendered page", async ({
+    test("Navigates across SPA/prerender pages when starting from a prerendered page", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -1284,14 +1285,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/", true);
       await page.waitForSelector('a[href="/page"]');
@@ -1340,7 +1334,7 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual(["/page.data", "/page.data"]);
     });
 
-    test("Properly navigates across SPA/prerender pages when starting from a SPA page and a root loader exists", async ({
+    test("Navigates across SPA/prerender pages when starting from a SPA page and a root loader exists", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -1439,14 +1433,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/", true);
       await page.waitForSelector("[data-root]");
@@ -1498,7 +1485,7 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual(["/page.data", "/page.data"]);
     });
 
-    test("Properly navigates across SPA/prerender pages when starting from a prerendered page and a root loader exists", async ({
+    test("Navigates across SPA/prerender pages when starting from a prerendered page and a root loader exists", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -1597,14 +1584,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/", true);
       await page.waitForSelector("[data-root]");
@@ -1656,7 +1636,7 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual(["/page.data", "/page.data"]);
     });
 
-    test("Properly navigates between prerendered parent and child SPA route", async ({
+    test("Navigates between prerendered parent and child SPA route", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -1743,14 +1723,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/parent", true);
       await expect(page.getByText("PARENT DATA")).toBeVisible();
@@ -1796,7 +1769,7 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual([]);
     });
 
-    test("Properly navigates between SPA parent and prerendered child route", async ({
+    test("Navigates between SPA parent and prerendered child route", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -1880,14 +1853,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/parent", true);
       await expect(page.getByText("PARENT DATA")).toBeVisible();
@@ -1911,7 +1877,7 @@ test.describe("Prerendering", () => {
 
       // Initial navigation and submission from /parent
       expect(requests).toEqual(["/parent/child.data", "/parent/child.data"]);
-      requests = [];
+      while (requests.length) requests.pop();
 
       await app.goto("/parent/child", true);
       await expect(page.getByText("PARENT DATA")).toBeVisible();
@@ -1935,7 +1901,7 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual(["/parent/child.data"]);
     });
 
-    test("Properly navigates between prerendered parent and child SPA route (with a root loader)", async ({
+    test("Navigates between prerendered parent and child SPA route (with a root loader)", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -2035,14 +2001,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/parent", true);
       await expect(page.getByText("ROOT DATA")).toBeVisible();
@@ -2090,7 +2049,7 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual([]);
     });
 
-    test("Properly navigates between SPA parent and prerendered child route (with a root loader)", async ({
+    test("Navigates between SPA parent and prerendered child route (with a root loader)", async ({
       page,
     }) => {
       fixture = await createFixture({
@@ -2183,14 +2142,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data") || pathname.endsWith("__manifest")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/parent", true);
       await expect(page.getByText("ROOT DATA")).toBeVisible();
@@ -2215,7 +2167,7 @@ test.describe("Prerendering", () => {
 
       // Initial navigation and submission from /parent
       expect(requests).toEqual(["/parent/child.data", "/parent/child.data"]);
-      requests = [];
+      while (requests.length) requests.pop();
 
       await app.goto("/parent/child", true);
       await expect(page.getByText("PARENT DATA")).toBeVisible();
@@ -2239,6 +2191,77 @@ test.describe("Prerendering", () => {
       expect(requests).toEqual(["/parent/child.data"]);
     });
 
+    test("Navigates to prerendered parent with clientLoader calling loader", async ({
+      page,
+    }) => {
+      fixture = await createFixture({
+        prerender: true,
+        files: {
+          "react-router.config.ts": reactRouterConfig({
+            ssr: false,
+            prerender: ["/", "/parent"],
+          }),
+          "vite.config.ts": files["vite.config.ts"],
+          "app/root.tsx": js`
+            import * as React from "react";
+            import { Link, Outlet, Scripts } from "react-router";
+
+            export function Layout({ children }) {
+              return (
+                <html lang="en">
+                  <head />
+                  <body>
+                    {children}
+                    <Scripts />
+                  </body>
+                </html>
+              );
+            }
+
+            export default function Root({ loaderData }) {
+              return (
+                <>
+                  <Link to="/parent">Go to parent</Link>
+                  <Outlet/>
+                </>
+              );
+            }
+
+            export function HydrateFallback() {
+              return <p>Loading...</p>;
+            }
+          `,
+          "app/routes/parent.tsx": js`
+            import { Link, Form, Outlet } from 'react-router';
+            export async function loader() {
+              return "PARENT DATA"
+            }
+            export async function clientLoader({ serverLoader }) {
+              let str = await serverLoader();
+              return str + " - CLIENT"
+            }
+            export function clientAction() {
+              return "PARENT ACTION"
+            }
+            export default function Parent({ loaderData, actionData }) {
+              return <p data-parent>{loaderData}</p>;
+            }
+          `,
+        },
+      });
+      appFixture = await createAppFixture(fixture);
+
+      let requests = captureRequests(page);
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/", true);
+      await expect(page.getByText("Go to parent")).toBeVisible();
+
+      await app.clickLink("/parent");
+      await expect(page.getByText("PARENT DATA - CLIENT")).toBeVisible();
+
+      expect(requests).toEqual(["/parent.data?_routes=routes%2Fparent"]);
+    });
+
     test("Handles 404s on data requests", async ({ page }) => {
       fixture = await createFixture({
         prerender: true,
@@ -2266,14 +2289,7 @@ test.describe("Prerendering", () => {
       });
       appFixture = await createAppFixture(fixture);
 
-      let requests: string[] = [];
-      page.on("request", (request) => {
-        let pathname = new URL(request.url()).pathname;
-        if (pathname.endsWith(".data")) {
-          requests.push(pathname);
-        }
-      });
-
+      let requests = captureRequests(page);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/");
       await page.waitForSelector("[data-mounted]");

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

@@ -186,12 +186,27 @@ export function getSingleFetchDataStrategy(
         // Skip single fetch and just call the loaders in parallel when this is
         // a SPA mode navigation
         let matchesToLoad = matches.filter((m) => m.shouldLoad);
-        let results = await Promise.all(matchesToLoad.map((m) => m.resolve()));
-        return results.reduce(
-          (acc, result, i) =>
-            Object.assign(acc, { [matchesToLoad[i].route.id]: result }),
-          {}
+        let url = stripIndexParam(singleFetchUrl(request.url));
+        let init = await createRequestInit(request);
+        let results: Record<string, DataStrategyResult> = {};
+        await Promise.all(
+          matchesToLoad.map((m) =>
+            m.resolve(async (handler) => {
+              try {
+                // Need to pass through a `singleFetch` override handler so
+                // clientLoader's can still call server loaders through `.data`
+                // requests
+                let result = manifest.routes[m.route.id]?.hasClientLoader
+                  ? await fetchSingleLoader(handler, url, init, m.route.id)
+                  : await handler();
+                results[m.route.id] = { type: "data", result };
+              } catch (e) {
+                results[m.route.id] = { type: "error", result: e };
+              }
+            })
+          )
         );
+        return results;
       }
     }