From bfd1f88de007ba79d1dca85391f1a3b7c58d3677 Mon Sep 17 00:00:00 2001
From: Nicholas Rakoto <nico@nrako.com>
Date: Thu, 30 Jun 2022 00:32:22 +0200
Subject: [PATCH] fix(remix-react): submit `<Form>` w/method set by submitter
 `formmethod`

Fix `<Form>` component by not passing the form's `method` into the
`options` when calling the `submit(target, options)` function (created
with the `useSubmit()` hook). This ensures that the option does not take
precedence on any `formethod` set on the submitter which otherwise break
the `formmethod` functionality.

Since #3094 and #3094 the `<Form>` passes the "submitter" (if any) as
the `target` to the `submit(target, options)` (`useSubmitImpl`) which
will attempt to read the `formmethod`, `formaction` and `formenctype`
attributes on the target. Therefore, the responsability to infer which
method should be used should solely be on `useSubmitImpl` for the given
`target`.

Co-authored-by: Maxime Doury <maxime.doury@docavenue.com>
---
 integration/bug-report-test.ts      | 58 ++++++++++++++---------------
 integration/form-test.ts            | 43 +++++++++++++++++++++
 packages/remix-react/components.tsx |  2 +-
 3 files changed, 73 insertions(+), 30 deletions(-)

diff --git a/integration/bug-report-test.ts b/integration/bug-report-test.ts
index c341db00f2a..b393ddc3cb0 100644
--- a/integration/bug-report-test.ts
+++ b/integration/bug-report-test.ts
@@ -48,34 +48,29 @@ test.beforeAll(async () => {
     ////////////////////////////////////////////////////////////////////////////
     files: {
       "app/routes/index.jsx": js`
-        import { useActionData, useLoaderData, Form } from "@remix-run/react";
-        import { json } from '@remix-run/server-runtime'
+        import { json } from "@remix-run/node";
+        import { useLoaderData, Link } from "@remix-run/react";
 
-        export function action({ request }) {
-          return json(request.method)
-        }
-
-        export function loader({ request }) {
-          return json(request.method)
+        export function loader() {
+          return json("pizza");
         }
 
         export default function Index() {
-          let actionData = useActionData();
-          let loaderData = useLoaderData();
+          let data = useLoaderData();
           return (
-            <>
-              <Form method="post">
-                <button type="submit" formMethod="get">Submit with GET</button>
-              </Form>
-              <form method="get">
-                <button type="submit" formMethod="post">Submit with POST</button>
-              </form>
-
-              <pre>{loaderData || actionData}</pre>
-            </>
+            <div>
+              {data}
+              <Link to="/burgers">Other Route</Link>
+            </div>
           )
         }
       `,
+
+      "app/routes/burgers.jsx": js`
+        export default function Index() {
+          return <div>cheeseburger</div>;
+        }
+      `,
     },
   });
 
@@ -90,17 +85,22 @@ test.afterAll(async () => appFixture.close());
 // add a good description for what you expect Remix to do 👇🏽
 ////////////////////////////////////////////////////////////////////////////////
 
-test("`<Form>` should submit with the method set via the `formmethod` attribute set on the submitter (button)", async ({
-  page,
-}) => {
+test("[description of what you expect it to do]", async ({ page }) => {
   let app = new PlaywrightFixture(appFixture, page);
+  // You can test any request your app might get using `fixture`.
+  let response = await fixture.requestDocument("/");
+  expect(await response.text()).toMatch("pizza");
+
+  // If you need to test interactivity use the `app`
   await app.goto("/");
-  await app.clickElement("text=Submit with GET");
-  await page.waitForLoadState("load");
-  expect(await app.getHtml("pre")).toBe(`<pre>GET</pre>`);
-  await page.waitForLoadState("load");
-  await app.clickElement("text=Submit with POST");
-  expect(await app.getHtml("pre")).toBe(`<pre>POST</pre>`);
+  await app.clickLink("/burgers");
+  expect(await app.getHtml()).toMatch("cheeseburger");
+
+  // If you're not sure what's going on, you can "poke" the app, it'll
+  // automatically open up in your browser for 20 seconds, so be quick!
+  // await app.poke(20);
+
+  // Go check out the other tests to see what else you can do.
 });
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/integration/form-test.ts b/integration/form-test.ts
index ab7cfd5de58..d5d385cea65 100644
--- a/integration/form-test.ts
+++ b/integration/form-test.ts
@@ -277,6 +277,36 @@ test.describe("Forms", () => {
             )
           }
         `,
+
+        "app/routes/submitter-formmethod.jsx": js`
+          import { useActionData, useLoaderData, Form } from "@remix-run/react";
+          import { json } from '@remix-run/server-runtime'
+
+          export function action({ request }) {
+            return json(request.method)
+          }
+
+          export function loader({ request }) {
+            return json(request.method)
+          }
+
+          export default function Index() {
+            let actionData = useActionData();
+            let loaderData = useLoaderData();
+            return (
+              <>
+                <Form method="post">
+                  <button type="submit" formMethod="get">Submit with GET</button>
+                </Form>
+                <Form method="get">
+                  <button type="submit" formMethod="post">Submit with POST</button>
+                </Form>
+
+                <pre>{loaderData || actionData}</pre>
+              </>
+            )
+          }
+        `,
       },
     });
 
@@ -574,5 +604,18 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/");
       });
     });
+
+    test("submits with the method set by `formmethod` attribute when set on the submitter (submit button)", async ({
+      page,
+    }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/submitter-formmethod");
+      await app.clickElement("text=Submit with GET");
+      await page.waitForLoadState("load");
+      expect(await app.getHtml("pre")).toBe(`<pre>GET</pre>`);
+      await page.waitForLoadState("load");
+      await app.clickElement("text=Submit with POST");
+      expect(await app.getHtml("pre")).toBe(`<pre>POST</pre>`);
+    });
   });
 });
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 721e5a49ae3..cff561b92e2 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -963,7 +963,7 @@ let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
                 let submitter = (event as unknown as HTMLSubmitEvent)
                   .nativeEvent.submitter as HTMLFormSubmitter | null;
 
-                submit(submitter || event.currentTarget, { method, replace });
+                submit(submitter || event.currentTarget, { replace });
               }
         }
         {...props}