Fix issues with useFormAction/useResolvedPath for dot paths in param/…

…splat routes (#10983)
remix-run · Nov 3, 2023 · fe066bd · fe066bd
1 parent 4f6c454
commit fe066bd
Show file tree

Hide file tree

Showing 6 changed files with 322 additions and 10 deletions.
diff --git a/.changeset/fix-resolve-path.md b/.changeset/fix-resolve-path.md
@@ -0,0 +1,7 @@
+---
+"react-router": patch
+---
+
+Fix bug in `useResolvedPath` that would cause `useResolvedPath(".")` in a splat route to lose the splat portion of the URL path.
+
+- ⚠️ This fixes a quite long-standing bug specifically for `"."` paths inside a splat route which incorrectly dropped the splat portion of the URL. If you are relative routing via `"."` inside a splat route in your application you should double check that your logic is not relying on this buggy behavior and update accordingly.
diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx
@@ -2653,6 +2653,46 @@ function testDomRouter(
             "/foo/bar"
           );
         });
+
+        it("does not include dynamic parameters from a parent layout route", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route path="/">
+                <Route path="foo" element={<ActionEmptyComponent />}>
+                  <Route path=":param" element={<h1>Param</h1>} />
+                </Route>
+              </Route>
+            ),
+            {
+              window: getWindow("/foo/bar"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          expect(container.querySelector("form")?.getAttribute("action")).toBe(
+            "/foo"
+          );
+        });
+
+        it("does not include splat parameters from a parent layout route", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route path="/">
+                <Route path="foo" element={<ActionEmptyComponent />}>
+                  <Route path="*" element={<h1>Splat</h1>} />
+                </Route>
+              </Route>
+            ),
+            {
+              window: getWindow("/foo/bar/baz/qux"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          expect(container.querySelector("form")?.getAttribute("action")).toBe(
+            "/foo"
+          );
+        });
       });
 
       describe("index routes", () => {
@@ -2876,6 +2916,44 @@ function testDomRouter(
             "/foo/bar"
           );
         });
+
+        it("includes param portion of path when no action is specified (inline splat)", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route path="/">
+                <Route path="foo">
+                  <Route path=":param" element={<NoActionComponent />} />
+                </Route>
+              </Route>
+            ),
+            {
+              window: getWindow("/foo/bar"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          expect(container.querySelector("form")?.getAttribute("action")).toBe(
+            "/foo/bar"
+          );
+        });
+
+        it("includes splat portion of path when no action is specified (nested splat)", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route path="/">
+                <Route path="foo/:param" element={<NoActionComponent />} />
+              </Route>
+            ),
+            {
+              window: getWindow("/foo/bar"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          expect(container.querySelector("form")?.getAttribute("action")).toBe(
+            "/foo/bar"
+          );
+        });
       });
 
       describe("splat routes", () => {
@@ -2895,7 +2973,7 @@ function testDomRouter(
           let { container } = render(<RouterProvider router={router} />);
 
           expect(container.querySelector("form")?.getAttribute("action")).toBe(
-            "/foo?a=1"
+            "/foo/bar?a=1"
           );
         });
 
@@ -2915,7 +2993,7 @@ function testDomRouter(
           let { container } = render(<RouterProvider router={router} />);
 
           expect(container.querySelector("form")?.getAttribute("action")).toBe(
-            "/foo"
+            "/foo/bar"
           );
         });
 
@@ -2935,7 +3013,25 @@ function testDomRouter(
           let { container } = render(<RouterProvider router={router} />);
 
           expect(container.querySelector("form")?.getAttribute("action")).toBe(
-            "/foo"
+            "/foo/bar"
+          );
+        });
+
+        it("includes splat portion of path when no action is specified (inline splat)", async () => {
+          let router = createTestRouter(
+            createRoutesFromElements(
+              <Route path="/">
+                <Route path="foo/*" element={<NoActionComponent />} />
+              </Route>
+            ),
+            {
+              window: getWindow("/foo/bar/baz"),
+            }
+          );
+          let { container } = render(<RouterProvider router={router} />);
+
+          expect(container.querySelector("form")?.getAttribute("action")).toBe(
+            "/foo/bar/baz"
           );
         });
       });

diff --git a/packages/react-router-dom/__tests__/link-href-test.tsx b/packages/react-router-dom/__tests__/link-href-test.tsx
@@ -530,7 +530,7 @@ describe("<Link> href", () => {
       });
 
       expect(renderer.root.findByType("a").props.href).toEqual(
-        "/inbox/messages"
+        "/inbox/messages/abc"
       );
     });
 

diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx
@@ -1478,10 +1478,8 @@ export function useFormAction(
   // object referenced by useMemo inside useResolvedPath
   let path = { ...useResolvedPath(action ? action : ".", { relative }) };
 
-  // Previously we set the default action to ".". The problem with this is that
-  // `useResolvedPath(".")` excludes search params 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.
+  // If no action was specified, browsers will persist current search params
+  // when determining the path, so match that behavior
   // https://github.com/remix-run/remix/issues/927
   let location = useLocation();
   if (action == null) {

diff --git a/packages/react-router/__tests__/useResolvedPath-test.tsx b/packages/react-router/__tests__/useResolvedPath-test.tsx
@@ -85,7 +85,7 @@ describe("useResolvedPath", () => {
   });
 
   describe("in a splat route", () => {
-    it("resolves . to the route path", () => {
+    it("resolves . to the route path (nested splat)", () => {
       let renderer: TestRenderer.ReactTestRenderer;
       TestRenderer.act(() => {
         renderer = TestRenderer.create(
@@ -99,6 +99,213 @@ describe("useResolvedPath", () => {
         );
       });
 
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users/mj","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves .. to the parent route path (nested splat)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/mj"]}>
+            <Routes>
+              <Route path="/users">
+                <Route path="*" element={<ShowResolvedPath path=".." />} />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves . to the route path (inline splat)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/name/mj"]}>
+            <Routes>
+              <Route path="/users">
+                <Route path="name/*" element={<ShowResolvedPath path="." />} />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users/name/mj","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves .. to the parent route path (inline splat)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/name/mj"]}>
+            <Routes>
+              <Route path="/users">
+                <Route path="name/*" element={<ShowResolvedPath path=".." />} />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves . to the route path (descendant route)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/mj"]}>
+            <Routes>
+              <Route
+                path="/users/*"
+                element={
+                  <Routes>
+                    <Route path="mj" element={<ShowResolvedPath path="." />} />
+                  </Routes>
+                }
+              />
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users/mj","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves .. to the parent route path (descendant route)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/mj"]}>
+            <Routes>
+              <Route
+                path="/users/*"
+                element={
+                  <Routes>
+                    <Route path="mj" element={<ShowResolvedPath path=".." />} />
+                  </Routes>
+                }
+              />
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users","search":"","hash":""}
+        </pre>
+      `);
+    });
+  });
+
+  describe("in a param route", () => {
+    it("resolves . to the route path (nested param)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/mj"]}>
+            <Routes>
+              <Route path="/users">
+                <Route path=":name" element={<ShowResolvedPath path="." />} />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users/mj","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves .. to the parent route (nested param)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/mj"]}>
+            <Routes>
+              <Route path="/users">
+                <Route path=":name" element={<ShowResolvedPath path=".." />} />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves . to the route path (inline param)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/name/mj"]}>
+            <Routes>
+              <Route path="/users">
+                <Route
+                  path="name/:name"
+                  element={<ShowResolvedPath path="." />}
+                />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.toJSON()).toMatchInlineSnapshot(`
+        <pre>
+          {"pathname":"/users/name/mj","search":"","hash":""}
+        </pre>
+      `);
+    });
+
+    it("resolves .. to the parent route (inline param)", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/users/name/mj"]}>
+            <Routes>
+              <Route path="/users">
+                <Route
+                  path="name/:name"
+                  element={<ShowResolvedPath path=".." />}
+                />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
       expect(renderer.toJSON()).toMatchInlineSnapshot(`
         <pre>
           {"pathname":"/users","search":"","hash":""}