From 4f907c12675af8219018c699e68fd7fa6a407d2d Mon Sep 17 00:00:00 2001
From: Javier Castro <javier.alejandro.castro@gmail.com>
Date: Mon, 6 Dec 2021 16:34:45 -0300
Subject: [PATCH 01/11] chore(tests): add test on gists app (#927)

---
 contributors.yml                             |  1 +
 fixtures/gists-app/tests/nested-form-test.ts | 27 ++++++++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 fixtures/gists-app/tests/nested-form-test.ts

diff --git a/contributors.yml b/contributors.yml
index afbba12c8ea..a571650ad99 100644
--- a/contributors.yml
+++ b/contributors.yml
@@ -14,6 +14,7 @@
 - goncy
 - graham42
 - ianduvall
+- jacargentina
 - jacob-ebey
 - jesseflorig
 - juhanakristian
diff --git a/fixtures/gists-app/tests/nested-form-test.ts b/fixtures/gists-app/tests/nested-form-test.ts
new file mode 100644
index 00000000000..829ac08993b
--- /dev/null
+++ b/fixtures/gists-app/tests/nested-form-test.ts
@@ -0,0 +1,27 @@
+import type { Browser, Page } from "puppeteer";
+import puppeteer from "puppeteer";
+
+import { reactIsHydrated } from "./utils";
+
+const testPort = 3000;
+const testServer = `http://localhost:${testPort}`;
+
+describe("nested form", () => {
+  let browser: Browser;
+  let page: Page;
+  beforeEach(async () => {
+    browser = await puppeteer.launch();
+    page = await browser.newPage();
+  });
+
+  afterEach(() => browser.close());
+
+  it("keep query string on post", async () => {
+    await page.goto(`${testServer}/nested-forms?q=1`);
+    await reactIsHydrated(page);
+
+    const originalUrl = page.url();
+    await page.click("button[type=submit]");
+    expect(page.url()).toEqual(originalUrl);
+  });
+});

From 291f3cc8d31438a9f4309942d890443560773e31 Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Sun, 10 Jul 2022 22:06:49 -0700
Subject: [PATCH 02/11] update tests

---
 integration/form-test.ts        | 120 ++++++++++++++++++++++++++++++++
 integration/nested-form-test.ts |  27 -------
 2 files changed, 120 insertions(+), 27 deletions(-)
 delete mode 100644 integration/nested-form-test.ts

diff --git a/integration/form-test.ts b/integration/form-test.ts
index ab7cfd5de58..f6e48012b40 100644
--- a/integration/form-test.ts
+++ b/integration/form-test.ts
@@ -20,22 +20,27 @@ test.describe("Forms", () => {
   let ACTION = "action";
   let EAT = "EAT";
 
+  let STATIC_ROUTE_NO_ACTION = "static-route-none";
   let STATIC_ROUTE_ABSOLUTE_ACTION = "static-route-abs";
   let STATIC_ROUTE_CURRENT_ACTION = "static-route-cur";
   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_ABSOLUTE_ACTION = "index-route-abs";
   let INDEX_ROUTE_CURRENT_ACTION = "index-route-cur";
   let INDEX_ROUTE_PARENT_ACTION = "index-route-parent";
   let INDEX_ROUTE_TOO_MANY_DOTS_ACTION = "index-route-too-many-dots";
+  let DYNAMIC_ROUTE_NO_ACTION = "dynamic-route-none";
   let DYNAMIC_ROUTE_ABSOLUTE_ACTION = "dynamic-route-abs";
   let DYNAMIC_ROUTE_CURRENT_ACTION = "dynamic-route-cur";
   let DYNAMIC_ROUTE_PARENT_ACTION = "dynamic-route-parent";
   let DYNAMIC_ROUTE_TOO_MANY_DOTS_ACTION = "dynamic-route-too-many-dots";
+  let LAYOUT_ROUTE_NO_ACTION = "layout-route-none";
   let LAYOUT_ROUTE_ABSOLUTE_ACTION = "layout-route-abs";
   let LAYOUT_ROUTE_CURRENT_ACTION = "layout-route-cur";
   let LAYOUT_ROUTE_PARENT_ACTION = "layout-route-parent";
   let LAYOUT_ROUTE_TOO_MANY_DOTS_ACTION = "layout-route-too-many-dots";
+  let SPLAT_ROUTE_NO_ACTION = "splat-route-none";
   let SPLAT_ROUTE_ABSOLUTE_ACTION = "splat-route-abs";
   let SPLAT_ROUTE_CURRENT_ACTION = "splat-route-cur";
   let SPLAT_ROUTE_PARENT_ACTION = "splat-route-parent";
@@ -131,6 +136,9 @@ test.describe("Forms", () => {
           export default function() {
             return (
               <>
+                <Form id="${STATIC_ROUTE_NO_ACTION}">
+                  <button>Submit</button>
+                </Form>
                 <Form id="${STATIC_ROUTE_ABSOLUTE_ACTION}" action="/about">
                   <button>Submit</button>
                 </Form>
@@ -154,6 +162,9 @@ test.describe("Forms", () => {
             return (
               <>
                 <h1>Blog</h1>
+                <Form id="${LAYOUT_ROUTE_NO_ACTION}">
+                  <button>Submit</button>
+                </Form>
                 <Form id="${LAYOUT_ROUTE_ABSOLUTE_ACTION}" action="/about">
                   <button>Submit</button>
                 </Form>
@@ -177,6 +188,9 @@ test.describe("Forms", () => {
           export default function() {
             return (
               <>
+                <Form id="${INDEX_ROUTE_NO_ACTION}">
+                  <button>Submit</button>
+                </Form>
                 <Form id="${INDEX_ROUTE_ABSOLUTE_ACTION}" action="/about">
                   <button>Submit</button>
                 </Form>
@@ -199,6 +213,9 @@ test.describe("Forms", () => {
           export default function() {
             return (
               <>
+                <Form id="${DYNAMIC_ROUTE_NO_ACTION}">
+                  <button>Submit</button>
+                </Form>
                 <Form id="${DYNAMIC_ROUTE_ABSOLUTE_ACTION}" action="/about">
                   <button>Submit</button>
                 </Form>
@@ -239,6 +256,9 @@ test.describe("Forms", () => {
           export default function() {
             return (
               <>
+                <Form id="${SPLAT_ROUTE_NO_ACTION}">
+                  <button>Submit</button>
+                </Form>
                 <Form id="${SPLAT_ROUTE_ABSOLUTE_ACTION}" action="/about">
                   <button>Submit</button>
                 </Form>
@@ -366,6 +386,26 @@ test.describe("Forms", () => {
 
   test.describe("<Form> action", () => {
     test.describe("in a static route", () => {
+      test("no action resolves relative to the current route", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/inbox");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${STATIC_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/inbox");
+      });
+
+      test("no action resolves relative to the current route w/ search params", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/inbox?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${STATIC_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/inbox?foo=bar");
+      });
+
       test("absolute action resolves relative to the root route", async ({
         page,
       }) => {
@@ -408,6 +448,26 @@ test.describe("Forms", () => {
     });
 
     test.describe("in a dynamic route", () => {
+      test("no action resolves relative to the current route", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog/abc");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${DYNAMIC_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog/abc");
+      });
+
+      test("no action resolves relative to the current route w/ search params", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog/abc?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${DYNAMIC_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog/abc?foo=bar");
+      });
+
       test("absolute action resolves relative to the root route", async ({
         page,
       }) => {
@@ -450,6 +510,26 @@ test.describe("Forms", () => {
     });
 
     test.describe("in an index route", () => {
+      test("no action resolves relative to the current route", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${INDEX_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog");
+      });
+
+      test("no action resolves relative to the current route w/ search params", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${INDEX_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog?index&foo=bar");
+      });
+
       test("absolute action resolves relative to the root route", async ({
         page,
       }) => {
@@ -492,6 +572,26 @@ test.describe("Forms", () => {
     });
 
     test.describe("in a layout route", () => {
+      test("no action resolves relative to the current route", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${LAYOUT_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog");
+      });
+
+      test("no action resolves relative to the current route w/ search params", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${LAYOUT_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog?foo=bar");
+      });
+
       test("absolute action resolves relative to the root route", async ({
         page,
       }) => {
@@ -534,6 +634,26 @@ test.describe("Forms", () => {
     });
 
     test.describe("in a splat route", () => {
+      test("no action resolves relative to the current route", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/projects/blarg");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${SPLAT_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/projects");
+      });
+
+      test("no action resolves relative to the current route w/ search params", async ({
+        page,
+      }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/projects/blarg?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${SPLAT_ROUTE_NO_ACTION}`);
+        expect(el.attr("action")).toMatch("/projects?foo=bar");
+      });
+
       test("absolute action resolves relative to the root route", async ({
         page,
       }) => {
diff --git a/integration/nested-form-test.ts b/integration/nested-form-test.ts
deleted file mode 100644
index 829ac08993b..00000000000
--- a/integration/nested-form-test.ts
+++ /dev/null
@@ -1,27 +0,0 @@
-import type { Browser, Page } from "puppeteer";
-import puppeteer from "puppeteer";
-
-import { reactIsHydrated } from "./utils";
-
-const testPort = 3000;
-const testServer = `http://localhost:${testPort}`;
-
-describe("nested form", () => {
-  let browser: Browser;
-  let page: Page;
-  beforeEach(async () => {
-    browser = await puppeteer.launch();
-    page = await browser.newPage();
-  });
-
-  afterEach(() => browser.close());
-
-  it("keep query string on post", async () => {
-    await page.goto(`${testServer}/nested-forms?q=1`);
-    await reactIsHydrated(page);
-
-    const originalUrl = page.url();
-    await page.click("button[type=submit]");
-    expect(page.url()).toEqual(originalUrl);
-  });
-});

From 74b057dee00fd05a4620b5fab902559d00c183a8 Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Sun, 10 Jul 2022 22:07:27 -0700
Subject: [PATCH 03/11] fix: preserve search params w/ empty form action

---
 packages/remix-react/components.tsx | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 57e4918407d..e587239e03c 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -933,7 +933,7 @@ let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
       reloadDocument = false,
       replace = false,
       method = "get",
-      action = ".",
+      action,
       encType = "application/x-www-form-urlencoded",
       fetchKey,
       onSubmit,
@@ -988,16 +988,37 @@ type HTMLFormSubmitter = HTMLButtonElement | HTMLInputElement;
  * @see https://remix.run/api/remix#useformaction
  */
 export function useFormAction(
-  action = ".",
+  action?: string,
   // TODO: Remove method param in v2 as it's no longer needed and is a breaking change
   method: FormMethod = "get"
 ): string {
   let { id } = useRemixRouteContext();
-  let path = useResolvedPath(action);
+
+  // The documented behavior of a form without an explicit action prop is:
+  // -  Forms without an action prop (<Form method="post">) will automatically
+  //    post to the same route within which they are rendered.
+  //
+  // The problem with setting the default action to `.` is that
+  // `useResolvedPath(".")` excludes search params. This is the intended
+  // behavior of useResolvedPath, but it's unexpected for forms without an
+  // action prop since the search params are an important part of the current
+  // location, and the action function should receive those in the request URL.
+  //
+  // If the action prop is omitted, we'll append the current route's search
+  // string to the current location if it exists. If the action is explicitly
+  // set to `.` it will resolve without the search params to match the intended
+  // behavior of useResolvedPath, resolving the same URL as `<Link to="." />`
+  // https://github.com/remix-run/remix/issues/927
+  let { search: currentSearch } = useLocation();
+  let actionIsCurrentRoute = action === undefined || action === ".";
+  let path = useResolvedPath(
+    action === undefined ? { pathname: ".", search: currentSearch } : action
+  );
+
   let search = path.search;
   let isIndexRoute = id.endsWith("/index");
 
-  if (action === "." && isIndexRoute) {
+  if (actionIsCurrentRoute && isIndexRoute) {
     search = search ? search.replace(/^\?/, "?index&") : "?index";
   }
 

From 6d2cd40f96eeff43308045b9f9d38b0eede671df Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Sun, 10 Jul 2022 22:16:27 -0700
Subject: [PATCH 04/11] add changeset

---
 .changeset/curvy-drinks-flash.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/curvy-drinks-flash.md

diff --git a/.changeset/curvy-drinks-flash.md b/.changeset/curvy-drinks-flash.md
new file mode 100644
index 00000000000..f51c4970c48
--- /dev/null
+++ b/.changeset/curvy-drinks-flash.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/react": patch
+---
+
+When an action is omitted from`<Form>`or`useFormAction`, the resolved action would be the current location but the search string was omitted. This was a bug, and the search params will now be included in the form's action attribute.

From 0be6f721dfd0137235226ccd58c214342d1625d9 Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Wed, 3 Aug 2022 11:35:16 -0700
Subject: [PATCH 05/11] update tests

---
 integration/form-test.ts | 70 +++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 15 deletions(-)

diff --git a/integration/form-test.ts b/integration/form-test.ts
index f6e48012b40..d67fb10132e 100644
--- a/integration/form-test.ts
+++ b/integration/form-test.ts
@@ -386,7 +386,7 @@ test.describe("Forms", () => {
 
   test.describe("<Form> action", () => {
     test.describe("in a static route", () => {
-      test("no action resolves relative to the current route", async ({
+      test("no action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -396,7 +396,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/inbox");
       });
 
-      test("no action resolves relative to the current route w/ search params", async ({
+      test("no action resolves to action w/ search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -416,7 +416,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/about");
       });
 
-      test("'.' action resolves relative to the current route", async ({
+      test("'.' action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -426,6 +426,14 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/inbox");
       });
 
+      test("'.' excludes search params", async ({ page }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/inbox?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${STATIC_ROUTE_CURRENT_ACTION}`);
+        expect(el.attr("action")).toMatch("/inbox");
+      });
+
       test("'..' action resolves relative to the parent route", async ({
         page,
       }) => {
@@ -448,7 +456,7 @@ test.describe("Forms", () => {
     });
 
     test.describe("in a dynamic route", () => {
-      test("no action resolves relative to the current route", async ({
+      test("no action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -458,7 +466,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog/abc");
       });
 
-      test("no action resolves relative to the current route w/ search params", async ({
+      test("no action resolves to action w/ search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -478,7 +486,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/about");
       });
 
-      test("'.' action resolves relative to the current route", async ({
+      test("'.' action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -488,6 +496,14 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog/abc");
       });
 
+      test("'.' excludes search params", async ({ page }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog/abc?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${DYNAMIC_ROUTE_CURRENT_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog/abc");
+      });
+
       test("'..' action resolves relative to the parent route", async ({
         page,
       }) => {
@@ -510,7 +526,7 @@ test.describe("Forms", () => {
     });
 
     test.describe("in an index route", () => {
-      test("no action resolves relative to the current route", async ({
+      test("no action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -520,7 +536,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog");
       });
 
-      test("no action resolves relative to the current route w/ search params", async ({
+      test("no action resolves to action w/ search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -540,7 +556,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/about");
       });
 
-      test("'.' action resolves relative to the current route", async ({
+      test("'.' action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -550,6 +566,14 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog");
       });
 
+      test("'.' excludes search params", async ({ page }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${INDEX_ROUTE_CURRENT_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog");
+      });
+
       test("'..' action resolves relative to the parent route", async ({
         page,
       }) => {
@@ -572,7 +596,7 @@ test.describe("Forms", () => {
     });
 
     test.describe("in a layout route", () => {
-      test("no action resolves relative to the current route", async ({
+      test("no action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -582,7 +606,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog");
       });
 
-      test("no action resolves relative to the current route w/ search params", async ({
+      test("no action resolves to action w/ search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -602,7 +626,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/about");
       });
 
-      test("'.' action resolves relative to the current route", async ({
+      test("'.' action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -612,6 +636,14 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog");
       });
 
+      test("'.' excludes search params", async ({ page }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/blog?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${LAYOUT_ROUTE_CURRENT_ACTION}`);
+        expect(el.attr("action")).toMatch("/blog");
+      });
+
       test("'..' action resolves relative to the parent route", async ({
         page,
       }) => {
@@ -634,7 +666,7 @@ test.describe("Forms", () => {
     });
 
     test.describe("in a splat route", () => {
-      test("no action resolves relative to the current route", async ({
+      test("no action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -644,7 +676,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/projects");
       });
 
-      test("no action resolves relative to the current route w/ search params", async ({
+      test("no action resolves to action w/ search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -664,7 +696,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/about");
       });
 
-      test("'.' action resolves relative to the current route", async ({
+      test("'.' action resolves relative to the closest route", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -674,6 +706,14 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/projects");
       });
 
+      test("'.' excludes search params", async ({ page }) => {
+        let app = new PlaywrightFixture(appFixture, page);
+        await app.goto("/projects/blarg?foo=bar");
+        let html = await app.getHtml();
+        let el = getElement(html, `#${SPLAT_ROUTE_CURRENT_ACTION}`);
+        expect(el.attr("action")).toMatch("/projects");
+      });
+
       test("'..' action resolves relative to the parent route", async ({
         page,
       }) => {

From 783c43573f37bb89e1cf25be5ec87b72aee5186f Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Wed, 3 Aug 2022 15:15:39 -0700
Subject: [PATCH 06/11] differentiate between no action and '.'

Co-authored-by: Ionut Botizan <ionut.botizan@haufe-lexware.com>
---
 packages/remix-react/components.tsx | 36 +++++++++++------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 8d120f5962a..7fcb0b03e43 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -994,36 +994,26 @@ export function useFormAction(
   method: FormMethod = "get"
 ): string {
   let { id } = useRemixRouteContext();
+  let resolvedPath = useResolvedPath(action ?? ".");
 
-  // The documented behavior of a form without an explicit action prop is:
-  // -  Forms without an action prop (<Form method="post">) will automatically
-  //    post to the same route within which they are rendered.
-  //
-  // The problem with setting the default action to `.` is that
-  // `useResolvedPath(".")` excludes search params. This is the intended
-  // behavior of useResolvedPath, but it's unexpected for forms without an
-  // action prop since the search params are an important part of the current
-  // location, and the action function should receive those in the request URL.
-  //
-  // If the action prop is omitted, we'll append the current route's search
-  // string to the current location if it exists. If the action is explicitly
-  // set to `.` it will resolve without the search params to match the intended
-  // behavior of useResolvedPath, resolving the same URL as `<Link to="." />`
+  // Previously we set the default action to ".". The problem with this is that
+  // `useResolvedPath(".")` excludes search params and the hash of the resolved
+  // URL. This is the intended behavior of when "." is specifically provided as
+  // the form action, but inconsistent w/ browsers when the action is omitted.
   // https://github.com/remix-run/remix/issues/927
-  let { search: currentSearch } = useLocation();
-  let actionIsCurrentRoute = action === undefined || action === ".";
-  let path = useResolvedPath(
-    action === undefined ? { pathname: ".", search: currentSearch } : action
-  );
+  let location = useLocation();
+  let { search, hash } = resolvedPath;
+  if (action === undefined) {
+    search = location.search;
+    hash = location.hash;
+  }
 
-  let search = path.search;
   let isIndexRoute = id.endsWith("/index");
-
-  if (actionIsCurrentRoute && isIndexRoute) {
+  if ((action === undefined || action === ".") && isIndexRoute) {
     search = search ? search.replace(/^\?/, "?index&") : "?index";
   }
 
-  return path.pathname + search;
+  return resolvedPath.pathname + search + hash;
 }
 
 export interface SubmitOptions {

From e12927bd382c926ecca439c375906f68041ab4e2 Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Wed, 3 Aug 2022 15:19:54 -0700
Subject: [PATCH 07/11] update changeset

---
 .changeset/curvy-drinks-flash.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.changeset/curvy-drinks-flash.md b/.changeset/curvy-drinks-flash.md
index f51c4970c48..24b78d03bbb 100644
--- a/.changeset/curvy-drinks-flash.md
+++ b/.changeset/curvy-drinks-flash.md
@@ -2,4 +2,4 @@
 "@remix-run/react": patch
 ---
 
-When an action is omitted from`<Form>`or`useFormAction`, the resolved action would be the current location but the search string was omitted. This was a bug, and the search params will now be included in the form's action attribute.
+Previously, if an `action` was omitted from `<Form>` or `useFormAction`, the action value would default to `"."`. This is incorrect, as `"."` should resolve based on the current _path_, but an empty action resolves relative to the current _URL_ (including the search and hash values). We've fixed this to differentiate between the two.

From b9414ab9094e35556c4b90c28978183cb1846155 Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Wed, 3 Aug 2022 15:22:22 -0700
Subject: [PATCH 08/11] update test assertions

---
 integration/form-test.ts | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/integration/form-test.ts b/integration/form-test.ts
index d67fb10132e..b83e731e28b 100644
--- a/integration/form-test.ts
+++ b/integration/form-test.ts
@@ -396,7 +396,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/inbox");
       });
 
-      test("no action resolves to action w/ search params", async ({
+      test("no action resolves to URL including search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -466,7 +466,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog/abc");
       });
 
-      test("no action resolves to action w/ search params", async ({
+      test("no action resolves to URL including search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -536,7 +536,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog");
       });
 
-      test("no action resolves to action w/ search params", async ({
+      test("no action resolves to URL including search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -606,7 +606,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/blog");
       });
 
-      test("no action resolves to action w/ search params", async ({
+      test("no action resolves to URL including search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);
@@ -676,7 +676,7 @@ test.describe("Forms", () => {
         expect(el.attr("action")).toMatch("/projects");
       });
 
-      test("no action resolves to action w/ search params", async ({
+      test("no action resolves to URL including search params", async ({
         page,
       }) => {
         let app = new PlaywrightFixture(appFixture, page);

From f6ddd780c80910dbc2393bda6880565c7fb45ac9 Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Thu, 4 Aug 2022 08:50:12 -0700
Subject: [PATCH 09/11] use `createPath` instead of string concat

---
 packages/remix-react/components.tsx | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 7fcb0b03e43..f3ea5dcf9bf 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -21,6 +21,7 @@ import {
 } from "react-router-dom";
 import type { LinkProps, NavLinkProps } from "react-router-dom";
 import type { Merge } from "type-fest";
+import { createPath } from "history";
 
 import type { AppData, FormEncType, FormMethod } from "./data";
 import type { EntryContext, AssetsManifest } from "./entry";
@@ -994,7 +995,7 @@ export function useFormAction(
   method: FormMethod = "get"
 ): string {
   let { id } = useRemixRouteContext();
-  let resolvedPath = useResolvedPath(action ?? ".");
+  let resolvedPath = useResolvedPath(action || ".");
 
   // Previously we set the default action to ".". The problem with this is that
   // `useResolvedPath(".")` excludes search params and the hash of the resolved
@@ -1013,7 +1014,7 @@ export function useFormAction(
     search = search ? search.replace(/^\?/, "?index&") : "?index";
   }
 
-  return resolvedPath.pathname + search + hash;
+  return createPath({ pathname: resolvedPath.pathname, search, hash });
 }
 
 export interface SubmitOptions {

From bf9e435d53be50ef755fe1306083d8f57ad23b0d Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Wed, 10 Aug 2022 12:55:52 -0700
Subject: [PATCH 10/11] check explicitly for nullish action

---
 packages/remix-react/components.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 63ad105057e..45e0245c12d 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -1005,7 +1005,7 @@ export function useFormAction(
   method: FormMethod = "get"
 ): string {
   let { id } = useRemixRouteContext();
-  let resolvedPath = useResolvedPath(action || ".");
+  let resolvedPath = useResolvedPath(action ?? ".");
 
   // Previously we set the default action to ".". The problem with this is that
   // `useResolvedPath(".")` excludes search params and the hash of the resolved

From 9147639294aafd339b7e8cb96b389b9b80b829dd Mon Sep 17 00:00:00 2001
From: Chance Strickland <hi@chance.dev>
Date: Wed, 10 Aug 2022 13:19:57 -0700
Subject: [PATCH 11/11] use nullish check instead of explicit undefined check

---
 packages/remix-react/components.tsx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 45e0245c12d..7b89d3d2332 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -1014,13 +1014,13 @@ export function useFormAction(
   // https://github.com/remix-run/remix/issues/927
   let location = useLocation();
   let { search, hash } = resolvedPath;
-  if (action === undefined) {
+  if (action == null) {
     search = location.search;
     hash = location.hash;
   }
 
   let isIndexRoute = id.endsWith("/index");
-  if ((action === undefined || action === ".") && isIndexRoute) {
+  if ((action == null || action === ".") && isIndexRoute) {
     search = search ? search.replace(/^\?/, "?index&") : "?index";
   }