From 2aef4a2711113be55fc248ac7bf1a9580b779cef Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 2 Aug 2023 14:54:31 -0400 Subject: [PATCH 1/2] Do not include hash in useFormAction() for unspecified actions --- .changeset/form-action-hash.md | 5 ++ .../__tests__/data-browser-router-test.tsx | 60 ++++++++++++------- packages/react-router-dom/index.tsx | 8 +-- 3 files changed, 47 insertions(+), 26 deletions(-) create mode 100644 .changeset/form-action-hash.md diff --git a/.changeset/form-action-hash.md b/.changeset/form-action-hash.md new file mode 100644 index 0000000000..36fe2e2907 --- /dev/null +++ b/.changeset/form-action-hash.md @@ -0,0 +1,5 @@ +--- +"react-router-dom": patch +--- + +Do not include hash in `useFormAction()` for unspecified actions since it cannot be determined on the server and causes hydration issues diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx index 83828d986e..0c33573c3d 100644 --- a/packages/react-router-dom/__tests__/data-browser-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-browser-router-test.tsx @@ -2529,7 +2529,7 @@ function testDomRouter( } describe("static routes", () => { - it("includes search params + hash when no action is specified", async () => { + it("includes search params when no action is specified", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2545,11 +2545,11 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar?a=1#hash" + "/foo/bar?a=1" ); }); - it("does not include search params + hash when action='.'", async () => { + it("does not include search params when action='.'", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2567,7 +2567,7 @@ function testDomRouter( ); }); - it("does not include search params + hash when action=''", async () => { + it("does not include search params when action=''", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2587,7 +2587,7 @@ function testDomRouter( }); describe("layout routes", () => { - it("includes search params + hash when no action is specified", async () => { + it("includes search params when no action is specified", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2605,11 +2605,11 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar?a=1#hash" + "/foo/bar?a=1" ); }); - it("does not include search params + hash when action='.'", async () => { + it("does not include search params when action='.'", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2631,7 +2631,7 @@ function testDomRouter( ); }); - it("does not include search params + hash when action=''", async () => { + it("does not include search params when action=''", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2655,7 +2655,7 @@ function testDomRouter( }); describe("index routes", () => { - it("includes search params + hash when no action is specified", async () => { + it("includes search params when no action is specified", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2673,11 +2673,11 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar?index&a=1#hash" + "/foo/bar?index&a=1" ); }); - it("does not include search params + hash action='.'", async () => { + it("does not include search params action='.'", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2699,7 +2699,7 @@ function testDomRouter( ); }); - it("does not include search params + hash action=''", async () => { + it("does not include search params action=''", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2777,7 +2777,7 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar?index&a=1#hash" + "/foo/bar?index&a=1" ); }); @@ -2816,7 +2816,7 @@ function testDomRouter( }); describe("dynamic routes", () => { - it("includes search params + hash when no action is specified", async () => { + it("includes search params when no action is specified", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2832,11 +2832,11 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar?a=1#hash" + "/foo/bar?a=1" ); }); - it("does not include search params + hash action='.'", async () => { + it("does not include search params action='.'", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2856,7 +2856,7 @@ function testDomRouter( ); }); - it("does not include search params + hash when action=''", async () => { + it("does not include search params when action=''", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2878,7 +2878,7 @@ function testDomRouter( }); describe("splat routes", () => { - it("includes search params + hash when no action is specified", async () => { + it("includes search params when no action is specified", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2894,11 +2894,11 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo?a=1#hash" + "/foo?a=1" ); }); - it("does not include search params + hash when action='.'", async () => { + it("does not include search params when action='.'", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2918,7 +2918,7 @@ function testDomRouter( ); }); - it("does not include search params + hash when action=''", async () => { + it("does not include search params when action=''", async () => { let router = createTestRouter( createRoutesFromElements( @@ -2938,6 +2938,24 @@ function testDomRouter( ); }); }); + + it("allows uer to specify search params and hash", async () => { + let router = createTestRouter( + createRoutesFromElements( + + + } /> + + + ), + { window: getWindow("/foo/bar?a=1#hash") } + ); + let { container } = render(); + + expect(container.querySelector("form")?.getAttribute("action")).toBe( + "/foo/bar?a=1#newhash" + ); + }); }); describe('
', () => { diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 6150a864dc..c40c7dd03a 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -1156,17 +1156,15 @@ export function useFormAction( let path = { ...useResolvedPath(action ? action : ".", { relative }) }; // 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 + // `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. // https://github.com/remix-run/remix/issues/927 let location = useLocation(); if (action == null) { - // Safe to write to these directly here since if action was undefined, we + // Safe to write to this directly here since if action was undefined, we // would have called useResolvedPath(".") which will never include a search - // or hash path.search = location.search; - path.hash = location.hash; // When grabbing search params from the URL, remove the automatically // inserted ?index param so we match the useResolvedPath search behavior From de78d21cdfb721895c75b4e3e07642f503979199 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 2 Aug 2023 15:35:30 -0400 Subject: [PATCH 2/2] Update packages/react-router-dom/__tests__/data-browser-router-test.tsx Co-authored-by: Jacob Ebey --- .../react-router-dom/__tests__/data-browser-router-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx index 0c33573c3d..3ef5bc3ec2 100644 --- a/packages/react-router-dom/__tests__/data-browser-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-browser-router-test.tsx @@ -2939,7 +2939,7 @@ function testDomRouter( }); }); - it("allows uer to specify search params and hash", async () => { + it("allows user to specify search params and hash", async () => { let router = createTestRouter( createRoutesFromElements(