Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not include hash in useFormAction() for unspecified actions #10758

Merged
merged 2 commits into from
Aug 2, 2023
Merged

Do not include hash in useFormAction() for unspecified actions #10758

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2aef4a2
Do not include hash in useFormAction() for unspecified actions
brophdawg11 Aug 2, 2023
de78d21
Update packages/react-router-dom/__tests__/data-browser-router-test.tsx
brophdawg11 Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/form-action-hash.md
View file
Open in desktop
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
Open in desktop
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 uer to specify search params and hash", async () => {
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down