Fix issues with contextual routing and index params (#12003)

remix-run · Sep 17, 2024 · 0389eba · 0389eba
1 parent f941ddf
commit 0389eba
Show file tree

Hide file tree

Showing 4 changed files with 308 additions and 15 deletions.
diff --git a/.changeset/calm-wombats-design.md b/.changeset/calm-wombats-design.md
@@ -0,0 +1,8 @@
+---
+"react-router-dom": patch
+"react-router": patch
+"@remix-run/router": patch
+---
+
+- Fix bug when submitting to the current contextual route (parent route with an index child) when an `?index` param already exists from a prior submission
+- Fix `useFormAction` bug - when removing `?index` param it would not keep other non-Remix `index` params
diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx
@@ -3000,6 +3000,280 @@ function testDomRouter(
         });
       });
 
+      describe("submitting to self from parent/index when ?index param exists", () => {
+        it("useSubmit", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route
+                id="parent"
+                path="/parent"
+                element={<Parent />}
+                action={({ request }) => "PARENT ACTION: " + request.url}
+              >
+                <Route
+                  id="index"
+                  index
+                  element={<Index />}
+                  action={({ request }) => "INDEX ACTION: " + request.url}
+                />
+              </Route>
+            ),
+            {
+              window: getWindow("/parent?index&index=keep"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          function Parent() {
+            let actionData = useActionData();
+            let submit = useSubmit();
+            return (
+              <>
+                <p id="parent">{actionData}</p>
+                <button onClick={() => submit({}, { method: "post" })}>
+                  Submit from parent
+                </button>
+                <Outlet />
+              </>
+            );
+          }
+
+          function Index() {
+            let actionData = useActionData();
+            let submit = useSubmit();
+            return (
+              <>
+                <p id="index">{actionData}</p>
+                <button onClick={() => submit({}, { method: "post" })}>
+                  Submit from index
+                </button>
+              </>
+            );
+          }
+
+          fireEvent.click(screen.getByText("Submit from parent"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("PARENT ACTION")));
+          expect(getHtml(container.querySelector("#parent")!)).toContain(
+            "PARENT ACTION: http://localhost/parent?index=keep"
+          );
+
+          fireEvent.click(screen.getByText("Submit from index"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("INDEX ACTION")));
+          expect(getHtml(container.querySelector("#index")!)).toContain(
+            "INDEX ACTION: http://localhost/parent?index&index=keep"
+          );
+        });
+
+        it("Form", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route
+                id="parent"
+                path="/parent"
+                element={<Parent />}
+                action={({ request }) => "PARENT ACTION: " + request.url}
+              >
+                <Route
+                  id="index"
+                  index
+                  element={<Index />}
+                  action={({ request }) => "INDEX ACTION: " + request.url}
+                />
+              </Route>
+            ),
+            {
+              window: getWindow("/parent?index&index=keep"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          function Parent() {
+            let actionData = useActionData();
+            return (
+              <>
+                <p id="parent">{actionData}</p>
+                <Form method="post" id="parent-form">
+                  <button type="submit">Submit from parent</button>
+                </Form>
+                <Outlet />
+              </>
+            );
+          }
+
+          function Index() {
+            let actionData = useActionData();
+            return (
+              <>
+                <p id="index">{actionData}</p>
+                <Form method="post" id="index-form">
+                  <button type="submit">Submit from index</button>
+                </Form>
+              </>
+            );
+          }
+
+          expect(
+            container.querySelector("#parent-form")?.getAttribute("action")
+          ).toBe("/parent?index=keep");
+          expect(
+            container.querySelector("#index-form")?.getAttribute("action")
+          ).toBe("/parent?index&index=keep");
+
+          fireEvent.click(screen.getByText("Submit from parent"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("PARENT ACTION")));
+          expect(getHtml(container.querySelector("#parent")!)).toContain(
+            "PARENT ACTION: http://localhost/parent?index=keep"
+          );
+
+          fireEvent.click(screen.getByText("Submit from index"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("INDEX ACTION")));
+          expect(getHtml(container.querySelector("#index")!)).toContain(
+            "INDEX ACTION: http://localhost/parent?index&index=keep"
+          );
+        });
+
+        it("fetcher.submit", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route
+                id="parent"
+                path="/parent"
+                element={<Parent />}
+                action={({ request }) => "PARENT ACTION: " + request.url}
+              >
+                <Route
+                  id="index"
+                  index
+                  element={<Index />}
+                  action={({ request }) => "INDEX ACTION: " + request.url}
+                />
+              </Route>
+            ),
+            {
+              window: getWindow("/parent?index&index=keep"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          function Parent() {
+            let fetcher = useFetcher();
+
+            return (
+              <>
+                <p id="parent">{fetcher.data}</p>
+                <button onClick={() => fetcher.submit({}, { method: "post" })}>
+                  Submit from parent
+                </button>
+                <Outlet />
+              </>
+            );
+          }
+
+          function Index() {
+            let fetcher = useFetcher();
+
+            return (
+              <>
+                <p id="index">{fetcher.data}</p>
+                <button onClick={() => fetcher.submit({}, { method: "post" })}>
+                  Submit from index
+                </button>
+              </>
+            );
+          }
+
+          fireEvent.click(screen.getByText("Submit from parent"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("PARENT ACTION")));
+          expect(getHtml(container.querySelector("#parent")!)).toContain(
+            "PARENT ACTION: http://localhost/parent?index=keep"
+          );
+
+          fireEvent.click(screen.getByText("Submit from index"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("INDEX ACTION")));
+          expect(getHtml(container.querySelector("#index")!)).toContain(
+            "INDEX ACTION: http://localhost/parent?index&index=keep"
+          );
+        });
+
+        it("fetcher.Form", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route
+                id="parent"
+                path="/parent"
+                element={<Parent />}
+                action={({ request }) => "PARENT ACTION: " + request.url}
+              >
+                <Route
+                  id="index"
+                  index
+                  element={<Index />}
+                  action={({ request }) => "INDEX ACTION: " + request.url}
+                />
+              </Route>
+            ),
+            {
+              window: getWindow("/parent?index&index=keep"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          function Parent() {
+            let fetcher = useFetcher();
+
+            return (
+              <>
+                <p id="parent">{fetcher.data}</p>
+                <fetcher.Form method="post" id="parent-form">
+                  <button type="submit">Submit from parent</button>
+                </fetcher.Form>
+                <Outlet />
+              </>
+            );
+          }
+
+          function Index() {
+            let fetcher = useFetcher();
+
+            return (
+              <>
+                <p id="index">{fetcher.data}</p>
+                <fetcher.Form method="post" id="index-form">
+                  <button type="submit">Submit from index</button>
+                </fetcher.Form>
+              </>
+            );
+          }
+
+          expect(
+            container.querySelector("#parent-form")?.getAttribute("action")
+          ).toBe("/parent?index=keep");
+          expect(
+            container.querySelector("#index-form")?.getAttribute("action")
+          ).toBe("/parent?index&index=keep");
+
+          fireEvent.click(screen.getByText("Submit from parent"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("PARENT ACTION")));
+          expect(getHtml(container.querySelector("#parent")!)).toContain(
+            "PARENT ACTION: http://localhost/parent?index=keep"
+          );
+
+          fireEvent.click(screen.getByText("Submit from index"));
+          await tick();
+          await waitFor(() => screen.getByText(new RegExp("INDEX ACTION")));
+          expect(getHtml(container.querySelector("#index")!)).toContain(
+            "INDEX ACTION: http://localhost/parent?index&index=keep"
+          );
+        });
+      });
+
       it("allows user to specify search params and hash", async () => {
         let router = createTestRouter(
           createRoutesFromElements(
@@ -5370,9 +5644,9 @@ function testDomRouter(
           let { container } = render(<RouterProvider router={router} />);
           expect(container.innerHTML).not.toMatch(/my-key/);
           await waitFor(() =>
-            // React `useId()` results in either `:r2a:` or `:rp:` depending on
-            // `DataBrowserRouter`/`DataHashRouter`
-            expect(container.innerHTML).toMatch(/(:r2a:|:rp:),my-key/)
+            // React `useId()` results in something such as `:r2a:`, `:r2i:`,
+            // `:rt:`, or `:rp:` depending on `DataBrowserRouter`/`DataHashRouter`
+            expect(container.innerHTML).toMatch(/(:r[0-9]?[a-z]:),my-key/)
           );
         });
       });

diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx
@@ -1635,9 +1635,13 @@ export function useFormAction(
     // since it might not apply to our contextual route.  We add it back based
     // on match.route.index below
     let params = new URLSearchParams(path.search);
-    if (params.has("index") && params.get("index") === "") {
+    let indexValues = params.getAll("index");
+    let hasNakedIndexParam = indexValues.some((v) => v === "");
+    if (hasNakedIndexParam) {
       params.delete("index");
-      path.search = params.toString() ? `?${params.toString()}` : "";
+      indexValues.filter((v) => v).forEach((v) => params.append("index", v));
+      let qs = params.toString();
+      path.search = qs ? `?${qs}` : "";
     }
   }
 

diff --git a/packages/router/router.ts b/packages/router/router.ts
@@ -4169,16 +4169,23 @@ function normalizeTo(
     path.hash = location.hash;
   }
 
-  // Add an ?index param for matched index routes if we don't already have one
-  if (
-    (to == null || to === "" || to === ".") &&
-    activeRouteMatch &&
-    activeRouteMatch.route.index &&
-    !hasNakedIndexQuery(path.search)
-  ) {
-    path.search = path.search
-      ? path.search.replace(/^\?/, "?index&")
-      : "?index";
+  // Account for `?index` params when routing to the current location
+  if ((to == null || to === "" || to === ".") && activeRouteMatch) {
+    let nakedIndex = hasNakedIndexQuery(path.search);
+    if (activeRouteMatch.route.index && !nakedIndex) {
+      // Add one when we're targeting an index route
+      path.search = path.search
+        ? path.search.replace(/^\?/, "?index&")
+        : "?index";
+    } else if (!activeRouteMatch.route.index && nakedIndex) {
+      // Remove existing ones when we're not
+      let params = new URLSearchParams(path.search);
+      let indexValues = params.getAll("index");
+      params.delete("index");
+      indexValues.filter((v) => v).forEach((v) => params.append("index", v));
+      let qs = params.toString();
+      path.search = qs ? `?${qs}` : "";
+    }
   }
 
   // If we're operating within a basename, prepend it to the pathname.  If