fix: do not append URL params on get submissions

remix-run · Aug 26, 2022 · 1e84760 · 1e84760
1 parent b7dbd2a
commit 1e84760
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 9 deletions.
diff --git a/integration/form-test.ts b/integration/form-test.ts
@@ -26,6 +26,7 @@ test.describe("Forms", () => {
   let STATIC_ROUTE_PARENT_ACTION = "static-route-parent";
   let STATIC_ROUTE_TOO_MANY_DOTS_ACTION = "static-route-too-many-dots";
   let INDEX_ROUTE_NO_ACTION = "index-route-none";
+  let INDEX_ROUTE_NO_ACTION_POST = "index-route-none-post";
   let INDEX_ROUTE_ABSOLUTE_ACTION = "index-route-abs";
   let INDEX_ROUTE_CURRENT_ACTION = "index-route-cur";
   let INDEX_ROUTE_PARENT_ACTION = "index-route-parent";
@@ -192,6 +193,7 @@ test.describe("Forms", () => {
             return (
               <>
                 <Form id="${INDEX_ROUTE_NO_ACTION}">
+                  <input type="hidden" name="foo" defaultValue="1" />
                   <button>Submit</button>
                 </Form>
                 <Form id="${INDEX_ROUTE_ABSOLUTE_ACTION}" action="/about">
@@ -206,6 +208,11 @@ test.describe("Forms", () => {
                 <Form id="${INDEX_ROUTE_TOO_MANY_DOTS_ACTION}" action="../../../../about">
                   <button>Submit</button>
                 </Form>
+
+                <Form method="post" id="${INDEX_ROUTE_NO_ACTION_POST}">
+                  <input type="hidden" name="bar" defaultValue="2" />
+                  <button>Submit</button>
+                </Form>
               </>
             )
           }
@@ -620,25 +627,53 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/");
       });
 
-      test("does not repeatedly add index params on submissions", async ({
+      test("handles search params correctly on GET submissions", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
-        await app.goto("/blog");
+
+        // Start with a query param
+        await app.goto("/blog?junk=1");
         let html = await app.getHtml();
         let el = getElement(html, `#${INDEX_ROUTE_NO_ACTION}`);
-        expect(el.attr("action")).toBe("/blog?index");
-        expect(app.page.url()).toMatch(/\/blog$/);
+        expect(el.attr("action")).toBe("/blog?index&junk=1");
+        expect(app.page.url()).toMatch(/\/blog\?junk=1$/);
 
+        // On submission, we replace existing parameters (reflected in the
+        // form action) with the values from the form data.  We also do not
+        // need to preserve the index param in the URL on GET submissions
         await app.clickElement(`#${INDEX_ROUTE_NO_ACTION} button`);
+        html = await app.getHtml();
         el = getElement(html, `#${INDEX_ROUTE_NO_ACTION}`);
-        expect(el.attr("action")).toBe("/blog?index");
-        expect(app.page.url()).toMatch(/\/blog\?index$/);
+        expect(el.attr("action")).toBe("/blog?index&foo=1");
+        expect(app.page.url()).toMatch(/\/blog\?foo=1$/);
 
+        // Does not append duplicate params on re-submissions
         await app.clickElement(`#${INDEX_ROUTE_NO_ACTION} button`);
+        html = await app.getHtml();
         el = getElement(html, `#${INDEX_ROUTE_NO_ACTION}`);
-        expect(el.attr("action")).toBe("/blog?index");
-        expect(app.page.url()).toMatch(/\/blog\?index$/);
+        expect(el.attr("action")).toBe("/blog?index&foo=1");
+        expect(app.page.url()).toMatch(/\/blog\?foo=1$/);
+      });
+
+      test("handles search params correctly on POST submissions", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+
+        // Start with a query param
+        await app.goto("/blog?junk=1");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${INDEX_ROUTE_NO_ACTION_POST}`);
+        expect(el.attr("action")).toBe("/blog?index&junk=1");
+        expect(app.page.url()).toMatch(/\/blog\?junk=1$/);
+
+        // Form action reflects the current params and change them on submission
+        await app.clickElement(`#${INDEX_ROUTE_NO_ACTION_POST} button`);
+        html = await app.getHtml();
+        el = getElement(html, `#${INDEX_ROUTE_NO_ACTION_POST}`);
+        expect(el.attr("action")).toBe("/blog?index&junk=1");
+        expect(app.page.url()).toMatch(/\/blog\?index&junk=1$/);
       });
     });
 

diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
@@ -1221,13 +1221,19 @@ export function useSubmitImpl(key?: string): SubmitFunction {
       let url = new URL(action, `${protocol}//${host}`);
 
       if (method.toLowerCase() === "get") {
+        // Start with a fresh set of params and wipe out the old params to
+        // match default browser behavior
+        let params = new URLSearchParams();
+        let hasParams = false;
         for (let [name, value] of formData) {
           if (typeof value === "string") {
-            url.searchParams.append(name, value);
+            hasParams = true;
+            params.append(name, value);
           } else {
             throw new Error(`Cannot submit binary form data using GET`);
           }
         }
+        url.search = hasParams ? `?${params.toString()}` : "";
       }
 
       let submission: Submission = {