Skip to content

Commit b714ec3

Browse files
brophdawg11brophdawg11
authored
Update replace logic for form submissions/redirects (#9734)
1 parent 36d2b38 commit b714ec3

File tree

6 files changed

+273
-23
lines changed

6 files changed

+273
-23
lines changed

‎.changeset/kind-gifts-reflect.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@remix-run/router": patch
3+
---
4+
5+
Fix explicit `replace` on submissions and `PUSH` on submission to new paths

‎docs/components/form.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,15 @@ Instructs the form to replace the current entry in the history stack, instead of
179179
<Form replace />
180180
```
181181

182-
The default behavior is conditional on the form `method`:
183-
184-
- `get` defaults to `false`
185-
- every other method defaults to `true` if your `action` is successful
186-
- if your `action` redirects or throws, then it will still push by default
182+
The default behavior is conditional on the form behavior:
183+
184+
- `method=get` forms default to `false`
185+
- submission methods depend on the `formAction` and `action` behavior:
186+
- if your `action` throws, then it will default to `false`
187+
- if your `action` redirects to the current location, it defaults to `true`
188+
- if your `action` redirects elsewhere, it defaults to `false`
189+
- if your `formAction` is the current location, it defaults to `true`
190+
- otherwise it defaults to `false`
187191

188192
We've found with `get` you often want the user to be able to click "back" to see the previous search results/filters, etc. But with the other methods the default is `true` to avoid the "are you sure you want to resubmit the form?" prompt. Note that even if `replace={false}` React Router _will not_ resubmit the form when the back button is clicked and the method is post, put, patch, or delete.
189193

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@
113113
"none": "12.5 kB"
114114
},
115115
"packages/react-router/dist/umd/react-router.production.min.js": {
116-
"none": "14.5 kB"
116+
"none": "15 kB"
117117
},
118118
"packages/react-router-dom/dist/react-router-dom.production.min.js": {
119119
"none": "11 kB"

‎packages/react-router-dom/__tests__/data-browser-router-test.tsx‎

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1311,7 +1311,7 @@ function testDomRouter(
13111311
`);
13121312
});
13131313

1314-
it('defaults useSubmit({ method: "post" }) to be a REPLACE navigation', async () => {
1314+
it('defaults useSubmit({ method: "post" }) to a new location to be a PUSH navigation', async () => {
13151315
let { container } = render(
13161316
<TestDataRouter window={getWindow("/")} hydrationData={{}}>
13171317
<Route element={<Layout />}>
@@ -1387,6 +1387,100 @@ function testDomRouter(
13871387
</div>"
13881388
`);
13891389

1390+
fireEvent.click(screen.getByText("Go back"));
1391+
await waitFor(() => screen.getByText("Page 1"));
1392+
expect(getHtml(container.querySelector(".output")))
1393+
.toMatchInlineSnapshot(`
1394+
"<div
1395+
class=\\"output\\"
1396+
>
1397+
<h1>
1398+
Page 1
1399+
</h1>
1400+
</div>"
1401+
`);
1402+
});
1403+
1404+
it('defaults useSubmit({ method: "post" }) to the same location to be a REPLACE navigation', async () => {
1405+
let { container } = render(
1406+
<TestDataRouter window={getWindow("/")} hydrationData={{}}>
1407+
<Route element={<Layout />}>
1408+
<Route index loader={() => "index"} element={<h1>index</h1>} />
1409+
<Route
1410+
path="1"
1411+
action={() => "action"}
1412+
loader={() => "1"}
1413+
element={<h1>Page 1</h1>}
1414+
/>
1415+
</Route>
1416+
</TestDataRouter>
1417+
);
1418+
1419+
function Layout() {
1420+
let navigate = useNavigate();
1421+
let submit = useSubmit();
1422+
let actionData = useActionData();
1423+
let formData = new FormData();
1424+
formData.append("test", "value");
1425+
return (
1426+
<>
1427+
<Link to="1">Go to 1</Link>
1428+
<button
1429+
onClick={() => {
1430+
submit(formData, { action: "1", method: "post" });
1431+
}}
1432+
>
1433+
Submit
1434+
</button>
1435+
<button onClick={() => navigate(-1)}>Go back</button>
1436+
<div className="output">
1437+
{actionData ? <p>{actionData}</p> : null}
1438+
<Outlet />
1439+
</div>
1440+
</>
1441+
);
1442+
}
1443+
1444+
expect(getHtml(container.querySelector(".output")))
1445+
.toMatchInlineSnapshot(`
1446+
"<div
1447+
class=\\"output\\"
1448+
>
1449+
<h1>
1450+
index
1451+
</h1>
1452+
</div>"
1453+
`);
1454+
1455+
fireEvent.click(screen.getByText("Go to 1"));
1456+
await waitFor(() => screen.getByText("Page 1"));
1457+
expect(getHtml(container.querySelector(".output")))
1458+
.toMatchInlineSnapshot(`
1459+
"<div
1460+
class=\\"output\\"
1461+
>
1462+
<h1>
1463+
Page 1
1464+
</h1>
1465+
</div>"
1466+
`);
1467+
1468+
fireEvent.click(screen.getByText("Submit"));
1469+
await waitFor(() => screen.getByText("action"));
1470+
expect(getHtml(container.querySelector(".output")))
1471+
.toMatchInlineSnapshot(`
1472+
"<div
1473+
class=\\"output\\"
1474+
>
1475+
<p>
1476+
action
1477+
</p>
1478+
<h1>
1479+
Page 1
1480+
</h1>
1481+
</div>"
1482+
`);
1483+
13901484
fireEvent.click(screen.getByText("Go back"));
13911485
await waitFor(() => screen.getByText("index"));
13921486
expect(getHtml(container.querySelector(".output")))

‎packages/router/__tests__/router-test.ts‎

Lines changed: 132 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2519,6 +2519,35 @@ describe("a router", () => {
25192519
expect(t.router.state.location.pathname).toEqual("/foo");
25202520
});
25212521

2522+
it("navigates correctly using POP navigations across actions to new locations", async () => {
2523+
let t = initializeTmTest();
2524+
2525+
// Navigate to /foo
2526+
let A = await t.navigate("/foo");
2527+
await A.loaders.foo.resolve("FOO");
2528+
expect(t.router.state.location.pathname).toEqual("/foo");
2529+
2530+
// Navigate to /bar
2531+
let B = await t.navigate("/bar");
2532+
await B.loaders.bar.resolve("BAR");
2533+
expect(t.router.state.location.pathname).toEqual("/bar");
2534+
2535+
// Post to /baz (should not replace)
2536+
let C = await t.navigate("/baz", {
2537+
formMethod: "post",
2538+
formData: createFormData({ key: "value" }),
2539+
});
2540+
await C.actions.baz.resolve("BAZ ACTION");
2541+
await C.loaders.root.resolve("ROOT");
2542+
await C.loaders.baz.resolve("BAZ");
2543+
expect(t.router.state.location.pathname).toEqual("/baz");
2544+
2545+
// POP to /bar
2546+
let D = await t.navigate(-1);
2547+
await D.loaders.bar.resolve("BAR");
2548+
expect(t.router.state.location.pathname).toEqual("/bar");
2549+
});
2550+
25222551
it("navigates correctly using POP navigations across action errors", async () => {
25232552
let t = initializeTmTest();
25242553

@@ -2635,6 +2664,42 @@ describe("a router", () => {
26352664
expect(t.router.state.location.key).not.toBe(postBarKey);
26362665
});
26372666

2667+
it("navigates correctly using POP navigations across action redirects to the same location", async () => {
2668+
let t = initializeTmTest();
2669+
2670+
// Navigate to /foo
2671+
let A = await t.navigate("/foo");
2672+
let fooKey = t.router.state.navigation.location?.key;
2673+
await A.loaders.foo.resolve("FOO");
2674+
expect(t.router.state.location.pathname).toEqual("/foo");
2675+
2676+
// Navigate to /bar
2677+
let B = await t.navigate("/bar");
2678+
await B.loaders.bar.resolve("BAR");
2679+
expect(t.router.state.historyAction).toEqual("PUSH");
2680+
expect(t.router.state.location.pathname).toEqual("/bar");
2681+
2682+
// Post to /bar, redirect to /bar
2683+
let C = await t.navigate("/bar", {
2684+
formMethod: "post",
2685+
formData: createFormData({ key: "value" }),
2686+
});
2687+
let postBarKey = t.router.state.navigation.location?.key;
2688+
let D = await C.actions.bar.redirect("/bar");
2689+
await D.loaders.root.resolve("ROOT");
2690+
await D.loaders.bar.resolve("BAR");
2691+
expect(t.router.state.historyAction).toEqual("REPLACE");
2692+
expect(t.router.state.location.pathname).toEqual("/bar");
2693+
2694+
// POP to /foo
2695+
let E = await t.navigate(-1);
2696+
await E.loaders.foo.resolve("FOO");
2697+
expect(t.router.state.historyAction).toEqual("POP");
2698+
expect(t.router.state.location.pathname).toEqual("/foo");
2699+
expect(t.router.state.location.key).toBe(fooKey);
2700+
expect(t.router.state.location.key).not.toBe(postBarKey);
2701+
});
2702+
26382703
it("navigates correctly using POP navigations across <Form replace> redirects", async () => {
26392704
let t = initializeTmTest();
26402705

@@ -2667,6 +2732,67 @@ describe("a router", () => {
26672732
expect(t.router.state.historyAction).toEqual("POP");
26682733
expect(t.router.state.location.pathname).toEqual("/foo");
26692734
});
2735+
2736+
it("should respect explicit replace:false on non-redirected actions to new locations", async () => {
2737+
// start at / (history stack: [/])
2738+
let t = initializeTmTest();
2739+
2740+
// Link to /foo (history stack: [/, /foo])
2741+
let A = await t.navigate("/foo");
2742+
await A.loaders.root.resolve("ROOT");
2743+
await A.loaders.foo.resolve("FOO");
2744+
expect(t.router.state.historyAction).toEqual("PUSH");
2745+
expect(t.router.state.location.pathname).toEqual("/foo");
2746+
2747+
// POST /bar (history stack: [/, /foo, /bar])
2748+
let B = await t.navigate("/bar", {
2749+
formMethod: "post",
2750+
formData: createFormData({ gosh: "dang" }),
2751+
replace: false,
2752+
});
2753+
await B.actions.bar.resolve("BAR");
2754+
await B.loaders.root.resolve("ROOT");
2755+
await B.loaders.bar.resolve("BAR");
2756+
expect(t.router.state.historyAction).toEqual("PUSH");
2757+
expect(t.router.state.location.pathname).toEqual("/bar");
2758+
2759+
// POP /foo (history stack: [GET /, GET /foo])
2760+
let C = await t.navigate(-1);
2761+
await C.loaders.foo.resolve("FOO");
2762+
expect(t.router.state.historyAction).toEqual("POP");
2763+
expect(t.router.state.location.pathname).toEqual("/foo");
2764+
});
2765+
2766+
it("should respect explicit replace:false on non-redirected actions to the same location", async () => {
2767+
// start at / (history stack: [/])
2768+
let t = initializeTmTest();
2769+
2770+
// Link to /foo (history stack: [/, /foo])
2771+
let A = await t.navigate("/foo");
2772+
await A.loaders.root.resolve("ROOT");
2773+
await A.loaders.foo.resolve("FOO");
2774+
expect(t.router.state.historyAction).toEqual("PUSH");
2775+
expect(t.router.state.location.pathname).toEqual("/foo");
2776+
2777+
// POST /foo (history stack: [/, /foo, /foo])
2778+
let B = await t.navigate("/foo", {
2779+
formMethod: "post",
2780+
formData: createFormData({ gosh: "dang" }),
2781+
replace: false,
2782+
});
2783+
await B.actions.foo.resolve("FOO2 ACTION");
2784+
await B.loaders.root.resolve("ROOT2");
2785+
await B.loaders.foo.resolve("FOO2");
2786+
expect(t.router.state.historyAction).toEqual("PUSH");
2787+
expect(t.router.state.location.pathname).toEqual("/foo");
2788+
2789+
// POP /foo (history stack: [/, /foo])
2790+
let C = await t.navigate(-1);
2791+
await C.loaders.root.resolve("ROOT3");
2792+
await C.loaders.foo.resolve("FOO3");
2793+
expect(t.router.state.historyAction).toEqual("POP");
2794+
expect(t.router.state.location.pathname).toEqual("/foo");
2795+
});
26702796
});
26712797

26722798
describe("submission navigations", () => {
@@ -6384,7 +6510,7 @@ describe("a router", () => {
63846510
await N.loaders.root.resolve("ROOT_DATA*");
63856511
await N.loaders.tasks.resolve("TASKS_DATA");
63866512
expect(t.router.state).toMatchObject({
6387-
historyAction: "REPLACE",
6513+
historyAction: "PUSH",
63886514
location: { pathname: "/tasks" },
63896515
navigation: IDLE_NAVIGATION,
63906516
revalidation: "idle",
@@ -6396,7 +6522,7 @@ describe("a router", () => {
63966522
tasks: "TASKS_ACTION",
63976523
},
63986524
});
6399-
expect(t.history.replace).toHaveBeenCalledWith(
6525+
expect(t.history.push).toHaveBeenCalledWith(
64006526
t.router.state.location,
64016527
t.router.state.location.state
64026528
);
@@ -6596,7 +6722,7 @@ describe("a router", () => {
65966722
await R.loaders.root.resolve("ROOT_DATA*");
65976723
await R.loaders.tasks.resolve("TASKS_DATA*");
65986724
expect(t.router.state).toMatchObject({
6599-
historyAction: "REPLACE",
6725+
historyAction: "PUSH",
66006726
location: { pathname: "/tasks" },
66016727
navigation: IDLE_NAVIGATION,
66026728
revalidation: "idle",
@@ -6605,7 +6731,7 @@ describe("a router", () => {
66056731
tasks: "TASKS_DATA*",
66066732
},
66076733
});
6608-
expect(t.history.replace).toHaveBeenCalledWith(
6734+
expect(t.history.push).toHaveBeenCalledWith(
66096735
t.router.state.location,
66106736
t.router.state.location.state
66116737
);
@@ -6683,7 +6809,7 @@ describe("a router", () => {
66836809
await R.loaders.root.resolve("ROOT_DATA*");
66846810
await R.loaders.tasks.resolve("TASKS_DATA*");
66856811
expect(t.router.state).toMatchObject({
6686-
historyAction: "REPLACE",
6812+
historyAction: "PUSH",
66876813
location: { pathname: "/tasks" },
66886814
navigation: IDLE_NAVIGATION,
66896815
revalidation: "idle",
@@ -6695,7 +6821,7 @@ describe("a router", () => {
66956821
tasks: "TASKS_ACTION",
66966822
},
66976823
});
6698-
expect(t.history.replace).toHaveBeenCalledWith(
6824+
expect(t.history.push).toHaveBeenCalledWith(
66996825
t.router.state.location,
67006826
t.router.state.location.state
67016827
);

0 commit comments

Comments
 (0)