From 79097ceaedbf5074e9c45df4859a5ebfa4b0c83a Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Fri, 8 Dec 2023 14:42:24 -0500
Subject: [PATCH 1/4] fix thrownc errors if no server handlerr exists

---
 integration/client-data-test.ts | 183 ++++++++++++++++++++++++++++++++
 packages/remix-react/routes.tsx |  28 +++--
 2 files changed, 204 insertions(+), 7 deletions(-)

diff --git a/integration/client-data-test.ts b/integration/client-data-test.ts
index 5e4bb543abd..a115fbde72f 100644
--- a/integration/client-data-test.ts
+++ b/integration/client-data-test.ts
@@ -545,6 +545,50 @@ test.describe("Client Data", () => {
       html = await app.getHtml("main");
       expect(html).toMatch("Loader Data (clientLoader only)");
     });
+
+    test("throws a 400 if you call serverLoader without a server loader", async ({
+      page,
+    }) => {
+      appFixture = await createAppFixture(
+        await createFixture({
+          files: {
+            ...getFiles({
+              parentClientLoader: false,
+              parentClientLoaderHydrate: false,
+              childClientLoader: false,
+              childClientLoaderHydrate: false,
+            }),
+            // Blow away parent.child.tsx with our own version without a server loader
+            "app/routes/parent.child.tsx": js`
+              import * as React from 'react';
+              import { useLoaderData, useRouteError } from '@remix-run/react';
+              export async function clientLoader({ serverLoader }) {
+                return await serverLoader();
+              }
+              export default function Component() {
+                return <p>Child</p>;
+              }
+              export function HydrateFallback() {
+                return <p>Loading...</p>;
+              }
+              export function ErrorBoundary() {
+                let error = useRouteError();
+                return <p id="child-error">{error.status} {error.data}</p>;
+              }
+            `,
+          },
+        })
+      );
+      let app = new PlaywrightFixture(appFixture, page);
+
+      await app.goto("/parent/child");
+      await page.waitForSelector("#child-error");
+      let html = await app.getHtml("#child-error");
+      expect(html.replace(/\n/g, " ").replace(/ +/g, " ")).toMatch(
+        "400 Error: You are trying to call serverLoader() on a route that does " +
+          'not have a server loader (routeId: "routes/parent.child")'
+      );
+    });
   });
 
   test.describe("clientLoader - lazy route module", () => {
@@ -632,6 +676,51 @@ test.describe("Client Data", () => {
       expect(html).toMatch("Parent Server Loader (mutated by client)");
       expect(html).toMatch("Child Server Loader (mutated by client");
     });
+
+    test("throws a 400 if you call serverLoader without a server loader", async ({
+      page,
+    }) => {
+      appFixture = await createAppFixture(
+        await createFixture({
+          files: {
+            ...getFiles({
+              parentClientLoader: false,
+              parentClientLoaderHydrate: false,
+              childClientLoader: false,
+              childClientLoaderHydrate: false,
+            }),
+            // Blow away parent.child.tsx with our own version without a server loader
+            "app/routes/parent.child.tsx": js`
+              import * as React from 'react';
+              import { useLoaderData, useRouteError } from '@remix-run/react';
+              export async function clientLoader({ serverLoader }) {
+                return await serverLoader();
+              }
+              export default function Component() {
+                return <p>Child</p>;
+              }
+              export function HydrateFallback() {
+                return <p>Loading...</p>;
+              }
+              export function ErrorBoundary() {
+                let error = useRouteError();
+                return <p id="child-error">{error.status} {error.data}</p>;
+              }
+            `,
+          },
+        })
+      );
+      let app = new PlaywrightFixture(appFixture, page);
+
+      await app.goto("/");
+      await app.clickLink("/parent/child");
+      await page.waitForSelector("#child-error");
+      let html = await app.getHtml("#child-error");
+      expect(html.replace(/\n/g, " ").replace(/ +/g, " ")).toMatch(
+        "400 Error: You are trying to call serverLoader() on a route that does " +
+          'not have a server loader (routeId: "routes/parent.child")'
+      );
+    });
   });
 
   test.describe("clientAction - critical route module", () => {
@@ -796,6 +885,52 @@ test.describe("Client Data", () => {
       expect(html).toMatch("Child Server Loader (mutated by client)");
       expect(html).toMatch("Child Server Action (mutated by client)");
     });
+
+    test("throws a 400 if you call serverAction without a server action", async ({
+      page,
+    }) => {
+      appFixture = await createAppFixture(
+        await createFixture({
+          files: {
+            ...getFiles({
+              parentClientLoader: false,
+              parentClientLoaderHydrate: false,
+              childClientLoader: false,
+              childClientLoaderHydrate: false,
+            }),
+            // Blow away parent.child.tsx with our own version without a server loader
+            "app/routes/parent.child.tsx": js`
+              import * as React from 'react';
+              import { json } from '@remix-run/node';
+              import { Form, useRouteError } from '@remix-run/react';
+              export async function clientAction({ serverAction }) {
+                return await serverAction();
+              }
+              export default function Component() {
+                return (
+                  <Form method="post">
+                    <button type="submit">Submit</button>
+                  </Form>
+                );
+              }
+              export function ErrorBoundary() {
+                let error = useRouteError();
+                return <p id="child-error">{error.status} {error.data}</p>;
+              }
+            `,
+          },
+        })
+      );
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/parent/child");
+      app.clickSubmitButton("/parent/child");
+      await page.waitForSelector("#child-error");
+      let html = await app.getHtml("#child-error");
+      expect(html.replace(/\n/g, " ").replace(/ +/g, " ")).toMatch(
+        "400 Error: You are trying to call serverAction() on a route that does " +
+          'not have a server action (routeId: "routes/parent.child")'
+      );
+    });
   });
 
   test.describe("clientAction - lazy route module", () => {
@@ -968,5 +1103,53 @@ test.describe("Client Data", () => {
       expect(html).toMatch("Child Server Loader (mutated by client)");
       expect(html).toMatch("Child Server Action (mutated by client)");
     });
+
+    test("throws a 400 if you call serverAction without a server action", async ({
+      page,
+    }) => {
+      appFixture = await createAppFixture(
+        await createFixture({
+          files: {
+            ...getFiles({
+              parentClientLoader: false,
+              parentClientLoaderHydrate: false,
+              childClientLoader: false,
+              childClientLoaderHydrate: false,
+            }),
+            // Blow away parent.child.tsx with our own version without a server loader
+            "app/routes/parent.child.tsx": js`
+              import * as React from 'react';
+              import { json } from '@remix-run/node';
+              import { Form, useRouteError } from '@remix-run/react';
+              export async function clientAction({ serverAction }) {
+                return await serverAction();
+              }
+              export default function Component() {
+                return (
+                  <Form method="post">
+                    <button type="submit">Submit</button>
+                  </Form>
+                );
+              }
+              export function ErrorBoundary() {
+                let error = useRouteError();
+                return <p id="child-error">{error.status} {error.data}</p>;
+              }
+            `,
+          },
+        })
+      );
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/");
+      await app.goto("/parent/child");
+      await page.waitForSelector("form");
+      app.clickSubmitButton("/parent/child");
+      await page.waitForSelector("#child-error");
+      let html = await app.getHtml("#child-error");
+      expect(html.replace(/\n/g, " ").replace(/ +/g, " ")).toMatch(
+        "400 Error: You are trying to call serverAction() on a route that does " +
+          'not have a server action (routeId: "routes/parent.child")'
+      );
+    });
   });
 });
diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx
index edcb2824e4a..80a5d44efda 100644
--- a/packages/remix-react/routes.tsx
+++ b/packages/remix-react/routes.tsx
@@ -135,6 +135,15 @@ export function createClientRoutesWithHMRRevalidationOptOut(
   );
 }
 
+function getNoServerHandlerError(type: "action" | "loader", routeId: string) {
+  let fn = type === "action" ? "serverAction()" : "serverLoader()";
+  let msg =
+    `You are trying to call ${fn} on a route that does not have a server ` +
+    `${type} (routeId: "${routeId}")`;
+  console.error(msg);
+  throw new ErrorResponse(400, "Bad Request", new Error(msg), true);
+}
+
 export function createClientRoutes(
   manifest: RouteManifest<EntryRoute>,
   routeModulesCache: RouteModules,
@@ -226,7 +235,7 @@ export function createClientRoutes(
       let isHydrationRequest =
         needsRevalidation == null &&
         routeModule.clientLoader != null &&
-        routeModule.clientLoader.hydrate === true;
+        (routeModule.clientLoader.hydrate === true || !route.hasLoader);
 
       dataRoute.loader = ({ request, params }: LoaderFunctionArgs) => {
         return prefetchStylesAndCallHandler(async () => {
@@ -242,12 +251,8 @@ export function createClientRoutes(
               if (isHydrationRequest) {
                 isHydrationRequest = false;
 
-                // Throw an error if a clientLoader tries to call a serverLoader that doesn't exist
-                if (initialData === undefined) {
-                  throw new Error(
-                    `You are trying to call serverLoader() on a route that does " +
-                      "not have a server loader (routeId: "${route.id}")`
-                  );
+                if (!route.hasLoader) {
+                  throw getNoServerHandlerError("loader", route.id);
                 }
 
                 // Otherwise, resolve the hydration clientLoader with the pre-loaded server data
@@ -276,6 +281,9 @@ export function createClientRoutes(
             request,
             params,
             async serverAction() {
+              if (!route.hasAction) {
+                throw getNoServerHandlerError("action", route.id);
+              }
               let result = await fetchServerAction(request);
               let unwrapped = await unwrapServerResponse(result);
               return unwrapped;
@@ -310,6 +318,9 @@ export function createClientRoutes(
             clientLoader({
               ...args,
               async serverLoader() {
+                if (!route.hasLoader) {
+                  throw getNoServerHandlerError("loader", route.id);
+                }
                 let response = await fetchServerLoader(args.request);
                 let result = await unwrapServerResponse(response);
                 return result;
@@ -323,6 +334,9 @@ export function createClientRoutes(
             clientAction({
               ...args,
               async serverAction() {
+                if (!route.hasAction) {
+                  throw getNoServerHandlerError("action", route.id);
+                }
                 let response = await fetchServerAction(args.request);
                 let result = await unwrapServerResponse(response);
                 return result;

From 6d91ca70d4e29838498a1263300dd56dd3895591 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Fri, 8 Dec 2023 15:05:49 -0500
Subject: [PATCH 2/4] Fix bug with isHydrationRequest check in clientLoader

---
 integration/client-data-test.ts | 142 ++++++++++++++++++++++++++++++++
 packages/remix-react/routes.tsx |  51 ++++++------
 2 files changed, 170 insertions(+), 23 deletions(-)

diff --git a/integration/client-data-test.ts b/integration/client-data-test.ts
index a115fbde72f..ae3a3f6f753 100644
--- a/integration/client-data-test.ts
+++ b/integration/client-data-test.ts
@@ -589,6 +589,148 @@ test.describe("Client Data", () => {
           'not have a server loader (routeId: "routes/parent.child")'
       );
     });
+
+    test("initial hydration data check functions properly", async ({
+      page,
+    }) => {
+      appFixture = await createAppFixture(
+        await createFixture({
+          files: {
+            ...getFiles({
+              parentClientLoader: false,
+              parentClientLoaderHydrate: false,
+              childClientLoader: false,
+              childClientLoaderHydrate: false,
+            }),
+            // Blow away parent.child.tsx with our own version without a server loader
+            "app/routes/parent.child.tsx": js`
+              import * as React from 'react';
+              import { json } from '@remix-run/node';
+              import { useLoaderData, useRevalidator } from '@remix-run/react';
+              let isFirstCall = true;
+              export async function loader({ serverLoader }) {
+                if (isFirstCall) {
+                  isFirstCall = false
+                  return json({
+                    message: "Child Server Loader Data (1)",
+                  });
+                }
+                return json({
+                  message: "Child Server Loader Data (2+)",
+                });
+              }
+              export async function clientLoader({ serverLoader }) {
+                await new Promise(r => setTimeout(r, 100));
+                let serverData = await serverLoader();
+                return {
+                  message: serverData.message + " (mutated by client)",
+                };
+              }
+              clientLoader.hydrate=true;
+              export default function Component() {
+                let data = useLoaderData();
+                let revalidator = useRevalidator();
+                return (
+                  <>
+                    <p id="child-data">{data.message}</p>
+                    <button onClick={() => revalidator.revalidate()}>Revalidate</button>
+                  </>
+                );
+              }
+              export function HydrateFallback() {
+                return <p>Loading...</p>
+              }
+            `,
+          },
+        })
+      );
+      let app = new PlaywrightFixture(appFixture, page);
+
+      await app.goto("/parent/child");
+      await page.waitForSelector("#child-data");
+      let html = await app.getHtml();
+      expect(html).toMatch("Child Server Loader Data (1) (mutated by client)");
+      app.clickElement("button");
+      await page.waitForSelector(':has-text("Child Server Loader Data (2+)")');
+      html = await app.getHtml("main");
+      expect(html).toMatch("Child Server Loader Data (2+) (mutated by client)");
+    });
+
+    test("initial hydration data check functions properly even if serverLoader isn't called on hydration", async ({
+      page,
+    }) => {
+      appFixture = await createAppFixture(
+        await createFixture({
+          files: {
+            ...getFiles({
+              parentClientLoader: false,
+              parentClientLoaderHydrate: false,
+              childClientLoader: false,
+              childClientLoaderHydrate: false,
+            }),
+            // Blow away parent.child.tsx with our own version without a server loader
+            "app/routes/parent.child.tsx": js`
+              import * as React from 'react';
+              import { json } from '@remix-run/node';
+              import { useLoaderData, useRevalidator } from '@remix-run/react';
+              let isFirstCall = true;
+              export async function loader({ serverLoader }) {
+                if (isFirstCall) {
+                  isFirstCall = false
+                  return json({
+                    message: "Child Server Loader Data (1)",
+                  });
+                }
+                return json({
+                  message: "Child Server Loader Data (2+)",
+                });
+              }
+              let isFirstClientCall = true;
+              export async function clientLoader({ serverLoader }) {
+                await new Promise(r => setTimeout(r, 100));
+                if (isFirstClientCall) {
+                  isFirstClientCall = false;
+                  // First time through - don't even call serverLoader
+                  return {
+                    message: "Child Client Loader Data",
+                  };
+                }
+                // Only call the serverLoader on subsequent calls and this
+                // should *not* return us the initialData any longer
+                let serverData = await serverLoader();
+                return {
+                  message: serverData.message + " (mutated by client)",
+                };
+              }
+              clientLoader.hydrate=true;
+              export default function Component() {
+                let data = useLoaderData();
+                let revalidator = useRevalidator();
+                return (
+                  <>
+                    <p id="child-data">{data.message}</p>
+                    <button onClick={() => revalidator.revalidate()}>Revalidate</button>
+                  </>
+                );
+              }
+              export function HydrateFallback() {
+                return <p>Loading...</p>
+              }
+            `,
+          },
+        })
+      );
+      let app = new PlaywrightFixture(appFixture, page);
+
+      await app.goto("/parent/child");
+      await page.waitForSelector("#child-data");
+      let html = await app.getHtml();
+      expect(html).toMatch("Child Client Loader Data");
+      app.clickElement("button");
+      await page.waitForSelector(':has-text("Child Server Loader Data (2+)")');
+      html = await app.getHtml("main");
+      expect(html).toMatch("Child Server Loader Data (2+) (mutated by client)");
+    });
   });
 
   test.describe("clientLoader - lazy route module", () => {
diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx
index 80a5d44efda..fef11b8fc5c 100644
--- a/packages/remix-react/routes.tsx
+++ b/packages/remix-react/routes.tsx
@@ -237,35 +237,40 @@ export function createClientRoutes(
         routeModule.clientLoader != null &&
         (routeModule.clientLoader.hydrate === true || !route.hasLoader);
 
-      dataRoute.loader = ({ request, params }: LoaderFunctionArgs) => {
-        return prefetchStylesAndCallHandler(async () => {
-          if (!routeModule.clientLoader) {
-            // Call the server when no client loader exists
-            return fetchServerLoader(request);
-          }
-
-          return routeModule.clientLoader({
-            request,
-            params,
-            async serverLoader() {
-              if (isHydrationRequest) {
-                isHydrationRequest = false;
-
+      dataRoute.loader = async ({ request, params }: LoaderFunctionArgs) => {
+        try {
+          let result = await prefetchStylesAndCallHandler(async () => {
+            if (!routeModule.clientLoader) {
+              // Call the server when no client loader exists
+              return fetchServerLoader(request);
+            }
+
+            return routeModule.clientLoader({
+              request,
+              params,
+              async serverLoader() {
                 if (!route.hasLoader) {
                   throw getNoServerHandlerError("loader", route.id);
                 }
 
-                // Otherwise, resolve the hydration clientLoader with the pre-loaded server data
-                return initialData;
-              }
+                // On the first call, resolve with the pre-loaded server data
+                if (isHydrationRequest) {
+                  return initialData;
+                }
 
-              // Call the server loader for client-side navigations
-              let result = await fetchServerLoader(request);
-              let unwrapped = await unwrapServerResponse(result);
-              return unwrapped;
-            },
+                // Call the server loader for client-side navigations
+                let result = await fetchServerLoader(request);
+                let unwrapped = await unwrapServerResponse(result);
+                return unwrapped;
+              },
+            });
           });
-        });
+          return result;
+        } finally {
+          // Whether or not the user calls `serverLoader`, we only let this
+          // stick around as true for one loader call
+          isHydrationRequest = false;
+        }
       };
 
       // Let React Router know whether to run this on hydration

From be8c5c6fa3da3a03a18b96237e51adfa019da602 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Fri, 8 Dec 2023 15:06:38 -0500
Subject: [PATCH 3/4] Add changeset

---
 .changeset/mean-falcons-love.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/mean-falcons-love.md

diff --git a/.changeset/mean-falcons-love.md b/.changeset/mean-falcons-love.md
new file mode 100644
index 00000000000..43b1742b591
--- /dev/null
+++ b/.changeset/mean-falcons-love.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/react": patch
+---
+
+[REMOVE] Fix a few edge cases with client data (throw if server handler doesnt exist, and only return initialData on first loader call)

From 9d449b5027b0ad919ad0a27853ece6a038b0c84d Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Fri, 8 Dec 2023 15:10:45 -0500
Subject: [PATCH 4/4] Remove duped comments

---
 integration/client-data-test.ts | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/integration/client-data-test.ts b/integration/client-data-test.ts
index ae3a3f6f753..a4b26c0d97f 100644
--- a/integration/client-data-test.ts
+++ b/integration/client-data-test.ts
@@ -315,7 +315,6 @@ test.describe("Client Data", () => {
             childClientLoader: false,
             childClientLoaderHydrate: false,
           }),
-          // Blow away parent.child.tsx with our own deferred version
           "app/routes/parent.child.tsx": js`
             import * as React from 'react';
             import { defer, json } from '@remix-run/node'
@@ -415,7 +414,6 @@ test.describe("Client Data", () => {
             childClientLoader: false,
             childClientLoaderHydrate: false,
           }),
-          // Blow away parent.child.tsx with our own version
           "app/routes/parent.child.tsx": js`
             import * as React from 'react';
             import { json } from '@remix-run/node';
@@ -470,7 +468,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { useLoaderData } from '@remix-run/react';
@@ -514,7 +511,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { useLoaderData } from '@remix-run/react';
@@ -558,7 +554,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { useLoaderData, useRouteError } from '@remix-run/react';
@@ -602,7 +597,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { json } from '@remix-run/node';
@@ -668,7 +662,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { json } from '@remix-run/node';
@@ -831,7 +824,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { useLoaderData, useRouteError } from '@remix-run/react';
@@ -1040,7 +1032,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { json } from '@remix-run/node';
@@ -1258,7 +1249,6 @@ test.describe("Client Data", () => {
               childClientLoader: false,
               childClientLoaderHydrate: false,
             }),
-            // Blow away parent.child.tsx with our own version without a server loader
             "app/routes/parent.child.tsx": js`
               import * as React from 'react';
               import { json } from '@remix-run/node';