fix(remix-react): prevent form data loss by appending submitter's val…

…ue in `useSubmit` (`<Form>`) (#3611)

* test: failing test with submitter value not appended to form data

Currently, remix-react's `<Form>` through `useSubmit` (`useSubmitImpl`)
implementation, includes the submitter value by setting it on the
`formData`. Unfortunetly, this may override any existing inputs having
the same name than the submitter; resulting in data loss.

Instead, the submitter's value should be appended to the `formData`.

* fix(remix-react): prevent form data loss by appending submitter's value

When submitting a form the submitter's value must be appended to the
`formData` and not have it's value "set" or pre-existing data under the
same name may be overidden resulting in data loss.

This commit fixes the `useSubmit() hook (`useSubmitImpl`) and
consequently it's indirect usage through the `<Form>` component.

contributors.yml

@@ -303,6 +303,7 @@
 - niwsa
 - nobeeakon
 - nordiauwu
+- nrako
 - nurul3101
 - nvh95
 - nwalters512

integration/form-test.ts

@@ -277,6 +277,29 @@ test.describe("Forms", () => {
             )
           }
         `,
+
+        "app/routes/submitter.jsx": js`
+          import { useLoaderData, Form } from "@remix-run/react";
+
+          export function loader({ request }) {
+            let url = new URL(request.url);
+            return url.searchParams.toString()
+          }
+
+          export default function() {
+            let data = useLoaderData();
+            return (
+              <Form>
+                <input type="text" name="tasks" defaultValue="first" />
+                <input type="text" name="tasks" defaultValue="second" />
+                <button type="submit" name="tasks" value="">
+                  Add Task
+                </button>
+                <pre>{data}</pre>
+              </Form>
+            )
+          }
+        `,
       },
     });
 
@@ -575,4 +598,16 @@ test.describe("Forms", () => {
       });
     });
   });
+
+  test("<Form> submits the submitter's value appended to the form data", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/submitter");
+    await app.clickElement("text=Add Task");
+    await page.waitForLoadState("load");
+    expect(await app.getHtml("pre")).toBe(
+      `<pre>tasks=first&amp;tasks=second&amp;tasks=</pre>`
+    );
+  });
 });

integration/hook-useSubmit-test.ts

@@ -0,0 +1,64 @@
+import { test, expect } from "@playwright/test";
+
+import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
+import type { Fixture, AppFixture } from "./helpers/create-fixture";
+import { PlaywrightFixture } from "./helpers/playwright-fixture";
+
+test.describe("`useSubmit()` returned function", () => {
+  let fixture: Fixture;
+  let appFixture: AppFixture;
+
+  test.beforeAll(async () => {
+    fixture = await createFixture({
+      files: {
+        "app/routes/index.jsx": js`
+        import { useLoaderData, useSubmit } from "@remix-run/react";
+
+        export function loader({ request }) {
+          let url = new URL(request.url);
+          return url.searchParams.toString()
+        }
+
+        export default function Index() {
+          let submit = useSubmit();
+          let handleClick = event => {
+            event.preventDefault()
+            submit(event.nativeEvent.submitter || event.currentTarget)
+          }
+          let data = useLoaderData();
+          return (
+            <form>
+              <input type="text" name="tasks" defaultValue="first" />
+              <input type="text" name="tasks" defaultValue="second" />
+
+              <button onClick={handleClick} name="tasks" value="third">
+                Prepare Third Task
+              </button>
+
+              <pre>{data}</pre>
+            </form>
+          )
+        }
+      `,
+      },
+    });
+
+    appFixture = await createAppFixture(fixture);
+  });
+
+  test.afterAll(async () => {
+    await appFixture.close();
+  });
+
+  test("submits the submitter's value appended to the form data", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/");
+    await app.clickElement("text=Prepare Third Task");
+    await page.waitForLoadState("load");
+    expect(await app.getHtml("pre")).toBe(
+      `<pre>tasks=first&amp;tasks=second&amp;tasks=third</pre>`
+    );
+  });
+});

packages/remix-react/components.tsx

@@ -1149,7 +1149,7 @@ export function useSubmitImpl(key?: string): SubmitFunction {
 
         // Include name + value from a <button>
         if (target.name) {
-          formData.set(target.name, target.value);
+          formData.append(target.name, target.value);
         }
       } else {
         if (isHtmlElement(target)) {