Skip to content

Commit

Permalink
Do not include hash in useFormAction() for unspecified actions (#10758)
Browse files Browse the repository at this point in the history 
Co-authored-by: Jacob Ebey <jacob.ebey@live.com>
  • Loading branch information
@brophdawg11 @jacob-ebey
brophdawg11 and jacob-ebey committed Aug 2, 2023
1 parent 46806a4 commit fb70fed
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 26 deletions.
5 changes: 5 additions & 0 deletions .changeset/form-action-hash.md
View file
Original file line number Diff line number Diff line change
@@ -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
60 changes: 39 additions & 21 deletions packages/react-router-dom/__tests__/data-browser-router-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<Route path="/">
Expand All @@ -2545,11 +2545,11 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand All @@ -2605,11 +2605,11 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand All @@ -2673,11 +2673,11 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand Down Expand Up @@ -2777,7 +2777,7 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo/bar?index&a=1#hash"
"/foo/bar?index&a=1"
);
});

Expand Down Expand Up @@ -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(
<Route path="/">
Expand All @@ -2832,11 +2832,11 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand All @@ -2894,11 +2894,11 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

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(
<Route path="/">
Expand All @@ -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(
<Route path="/">
Expand All @@ -2938,6 +2938,24 @@ function testDomRouter(
);
});
});

it("allows user to specify search params and hash", async () => {
let router = createTestRouter(
createRoutesFromElements(
<Route path="/">
<Route path="foo">
<Route path="bar" element={<Form action=".?a=1#newhash" />} />
</Route>
</Route>
),
{ window: getWindow("/foo/bar?a=1#hash") }
);
let { container } = render(<RouterProvider router={router} />);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo/bar?a=1#newhash"
);
});
});

describe('<Form action relative="path">', () => {
Expand Down
8 changes: 3 additions & 5 deletions packages/react-router-dom/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1157,17 +1157,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
Expand Down

0 comments on commit fb70fed

Please sign in to comment.