fix(remix-react): Preserve search string in form action when action prop is omitted

.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.

contributors.yml

@@ -153,6 +153,7 @@
 - isaacrmoreno
 - ishan-me
 - IshanKBG
+- jacargentina
 - jacob-ebey
 - JacobParis
 - jakewtaylor

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,
       }) => {

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";
   }