Skip to content

Commit f63774c

Browse files
jensengjenseng
authored
fix(react-router-dom): fix submitter serialization (#9865)
1 parent a3b21eb commit f63774c

File tree

7 files changed

+249
-16
lines changed

7 files changed

+249
-16
lines changed

‎.changeset/formdata-submitter.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"react-router-dom": patch
3+
---
4+
5+
When submitting a form from a `submitter` element, prefer the built-in `new FormData(form, submitter)` instead of the previous manual approach in modern browsers (those that support the new `submitter` parameter). For browsers that don't support it, we continue to just append the submit button's entry to the end, and we also add rudimentary support for `type="image"` buttons. If developers want full spec-compliant support for legacy browsers, they can use the `formdata-submitter-polyfill`.

‎package.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"resolutions": {
4141
"@types/react": "^18.0.0",
4242
"@types/react-dom": "^18.0.0",
43-
"jsdom": "22.0.0"
43+
"jsdom": "22.1.0"
4444
},
4545
"dependencies": {
4646
"@ampproject/filesize": "^4.3.0",
@@ -118,10 +118,10 @@
118118
"none": "16.2 kB"
119119
},
120120
"packages/react-router-dom/dist/react-router-dom.production.min.js": {
121-
"none": "12.4 kB"
121+
"none": "12.5 kB"
122122
},
123123
"packages/react-router-dom/dist/umd/react-router-dom.production.min.js": {
124-
"none": "18.5 kB"
124+
"none": "18.6 kB"
125125
}
126126
}
127127
}

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import { JSDOM } from "jsdom";
2+
// Drop support for the submitter parameter, as in a legacy browser. This
3+
// needs to be done before react-router-dom is required, since it does some
4+
// FormData detection.
5+
import "./polyfills/drop-FormData-submitter";
6+
import * as React from "react";
7+
import { render, fireEvent, screen } from "@testing-library/react";
8+
import "@testing-library/jest-dom";
9+
import {
10+
Form,
11+
Route,
12+
RouterProvider,
13+
createBrowserRouter,
14+
createHashRouter,
15+
createRoutesFromElements,
16+
} from "react-router-dom";
17+
18+
testDomRouter("<DataBrowserRouter>", createBrowserRouter, (url) =>
19+
getWindowImpl(url, false)
20+
);
21+
22+
testDomRouter("<DataHashRouter>", createHashRouter, (url) =>
23+
getWindowImpl(url, true)
24+
);
25+
26+
function testDomRouter(
27+
name: string,
28+
createTestRouter: typeof createBrowserRouter | typeof createHashRouter,
29+
getWindow: (initialUrl: string, isHash?: boolean) => Window
30+
) {
31+
describe(`Router: ${name} with a legacy FormData implementation`, () => {
32+
let consoleWarn: jest.SpyInstance;
33+
let consoleError: jest.SpyInstance;
34+
35+
beforeEach(() => {
36+
consoleWarn = jest.spyOn(console, "warn").mockImplementation(() => {});
37+
consoleError = jest.spyOn(console, "error").mockImplementation(() => {});
38+
});
39+
40+
afterEach(() => {
41+
window.__staticRouterHydrationData = undefined;
42+
consoleWarn.mockRestore();
43+
consoleError.mockRestore();
44+
});
45+
46+
describe("useSubmit/Form FormData", () => {
47+
it("appends basic submitter value(s)", async () => {
48+
let actionSpy = jest.fn();
49+
actionSpy.mockReturnValue({});
50+
async function getPayload() {
51+
let formData = await actionSpy.mock.calls[
52+
actionSpy.mock.calls.length - 1
53+
][0].request.formData();
54+
return new URLSearchParams(formData.entries()).toString();
55+
}
56+
57+
let router = createTestRouter(
58+
createRoutesFromElements(
59+
<Route path="/" action={actionSpy} element={<FormPage />} />
60+
),
61+
{ window: getWindow("/") }
62+
);
63+
render(<RouterProvider router={router} />);
64+
65+
function FormPage() {
66+
return (
67+
<>
68+
<button name="tasks" value="outside" form="myform">
69+
Outside
70+
</button>
71+
<Form id="myform" method="post">
72+
<input type="text" name="tasks" defaultValue="first" />
73+
<input type="text" name="tasks" defaultValue="second" />
74+
75+
<button name="tasks" value="">
76+
Add Task
77+
</button>
78+
<button value="">No Name</button>
79+
<input type="image" name="tasks" alt="Add Task" />
80+
<input type="image" alt="No Name" />
81+
82+
<input type="text" name="tasks" defaultValue="last" />
83+
</Form>
84+
</>
85+
);
86+
}
87+
88+
fireEvent.click(screen.getByText("Add Task"));
89+
expect(await getPayload()).toEqual(
90+
"tasks=first&tasks=second&tasks=last&tasks="
91+
);
92+
93+
fireEvent.click(screen.getByText("No Name"));
94+
expect(await getPayload()).toEqual(
95+
"tasks=first&tasks=second&tasks=last"
96+
);
97+
98+
fireEvent.click(screen.getByAltText("Add Task"), {
99+
clientX: 1,
100+
clientY: 2,
101+
});
102+
expect(await getPayload()).toEqual(
103+
"tasks=first&tasks=second&tasks=last&tasks.x=0&tasks.y=0"
104+
);
105+
106+
fireEvent.click(screen.getByAltText("No Name"), {
107+
clientX: 1,
108+
clientY: 2,
109+
});
110+
expect(await getPayload()).toEqual(
111+
"tasks=first&tasks=second&tasks=last&x=0&y=0"
112+
);
113+
114+
fireEvent.click(screen.getByText("Outside"));
115+
expect(await getPayload()).toEqual(
116+
"tasks=first&tasks=second&tasks=last&tasks=outside"
117+
);
118+
});
119+
});
120+
});
121+
}
122+
123+
function getWindowImpl(initialUrl: string, isHash = false): Window {
124+
// Need to use our own custom DOM in order to get a working history
125+
const dom = new JSDOM(`<!DOCTYPE html>`, { url: "http://localhost/" });
126+
dom.window.history.replaceState(null, "", (isHash ? "#" : "") + initialUrl);
127+
return dom.window as unknown as Window;
128+
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3465,6 +3465,79 @@ function testDomRouter(
34653465
expect(formData.get("a")).toBe("1");
34663466
expect(formData.getAll("b")).toEqual(["2", "3"]);
34673467
});
3468+
3469+
it("includes the correct submitter value(s) in tree order", async () => {
3470+
let actionSpy = jest.fn();
3471+
actionSpy.mockReturnValue({});
3472+
async function getPayload() {
3473+
let formData = await actionSpy.mock.calls[
3474+
actionSpy.mock.calls.length - 1
3475+
][0].request.formData();
3476+
return new URLSearchParams(formData.entries()).toString();
3477+
}
3478+
3479+
let router = createTestRouter(
3480+
createRoutesFromElements(
3481+
<Route path="/" action={actionSpy} element={<FormPage />} />
3482+
),
3483+
{ window: getWindow("/") }
3484+
);
3485+
render(<RouterProvider router={router} />);
3486+
3487+
function FormPage() {
3488+
return (
3489+
<>
3490+
<button name="tasks" value="outside" form="myform">
3491+
Outside
3492+
</button>
3493+
<Form id="myform" method="post">
3494+
<input type="text" name="tasks" defaultValue="first" />
3495+
<input type="text" name="tasks" defaultValue="second" />
3496+
3497+
<button name="tasks" value="">
3498+
Add Task
3499+
</button>
3500+
<button value="">No Name</button>
3501+
<input type="image" name="tasks" alt="Add Task" />
3502+
<input type="image" alt="No Name" />
3503+
3504+
<input type="text" name="tasks" defaultValue="last" />
3505+
</Form>
3506+
</>
3507+
);
3508+
}
3509+
3510+
fireEvent.click(screen.getByText("Add Task"));
3511+
expect(await getPayload()).toEqual(
3512+
"tasks=first&tasks=second&tasks=&tasks=last"
3513+
);
3514+
3515+
fireEvent.click(screen.getByText("No Name"));
3516+
expect(await getPayload()).toEqual(
3517+
"tasks=first&tasks=second&tasks=last"
3518+
);
3519+
3520+
fireEvent.click(screen.getByAltText("Add Task"), {
3521+
clientX: 1,
3522+
clientY: 2,
3523+
});
3524+
expect(await getPayload()).toMatch(
3525+
"tasks=first&tasks=second&tasks.x=1&tasks.y=2&tasks=last"
3526+
);
3527+
3528+
fireEvent.click(screen.getByAltText("No Name"), {
3529+
clientX: 1,
3530+
clientY: 2,
3531+
});
3532+
expect(await getPayload()).toMatch(
3533+
"tasks=first&tasks=second&x=1&y=2&tasks=last"
3534+
);
3535+
3536+
fireEvent.click(screen.getByText("Outside"));
3537+
expect(await getPayload()).toEqual(
3538+
"tasks=outside&tasks=first&tasks=second&tasks=last"
3539+
);
3540+
});
34683541
});
34693542

34703543
describe("useFetcher(s)", () => {

‎packages/react-router-dom/__tests__/polyfills/drop-FormData-submitter.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Drop support for the submitter parameter, as in a legacy browser. This needs
2+
// to be a standalone module due to how jest requires things (i.e. we can't
3+
// just do this inline in data-browser-router-legacy-formdata-test.tsx)
4+
window.FormData = class FormData extends window["FormData"] {
5+
constructor(form?: HTMLFormElement) {
6+
super(form, undefined);
7+
}
8+
};

‎packages/react-router-dom/dom.ts‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,15 @@ export type SubmitTarget =
126126
| JsonValue
127127
| null;
128128

129+
// One-time check for submitter support
130+
let formDataSupportsSubmitter = false;
131+
try {
132+
// @ts-expect-error if FormData supports the submitter parameter, this will throw
133+
new FormData(undefined, 0);
134+
} catch (e) {
135+
formDataSupportsSubmitter = true;
136+
}
137+
129138
export interface SubmitOptions {
130139
/**
131140
* The HTTP method used to submit the form. Overrides `<form method>`.
@@ -241,12 +250,22 @@ export function getFormSubmissionInfo(
241250
getFormEncType(form.getAttribute("enctype")) ||
242251
defaultEncType;
243252

244-
formData = new FormData(form);
245-
246-
// Include name + value from a <button>, appending in case the button name
247-
// matches an existing input name
248-
if (target.name) {
249-
formData.append(target.name, target.value);
253+
// Build a FormData object populated from a form and submitter
254+
formData = new FormData(form, target);
255+
256+
// If this browser doesn't support the `FormData(el, submitter)` format,
257+
// then tack on the submitter value at the end. This is a lightweight
258+
// solution that is not 100% spec compliant. For complete support in older
259+
// browsers, consider using the `formdata-submitter-polyfill` package
260+
if (!formDataSupportsSubmitter) {
261+
let { name, type, value } = target;
262+
if (type === "image") {
263+
let prefix = name ? `${name}.` : "";
264+
formData.append(`${prefix}x`, "0");
265+
formData.append(`${prefix}y`, "0");
266+
} else if (name) {
267+
formData.append(name, value);
268+
}
250269
}
251270
} else if (isHtmlElement(target)) {
252271
throw new Error(

‎yarn.lock‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,9 +2332,9 @@
23322332
"@types/react" "*"
23332333

23342334
"@types/react@*", "@types/react@^18.0.0", "@types/react@^18.0.15":
2335-
version "18.0.20"
2336-
resolved "https://registry.yarnpkg.com/@types/react/-/react-18.0.20.tgz#e4c36be3a55eb5b456ecf501bd4a00fd4fd0c9ab"
2337-
integrity sha512-MWul1teSPxujEHVwZl4a5HxQ9vVNsjTchVA+xRqv/VYGCuKGAU6UhfrTdF5aBefwD1BHUD8i/zq+O/vyCm/FrA==
2335+
version "18.2.5"
2336+
resolved "https://registry.yarnpkg.com/@types/react/-/react-18.2.5.tgz#f9403e1113b12b53f7edcdd9a900c10dd4b49a59"
2337+
integrity sha512-RuoMedzJ5AOh23Dvws13LU9jpZHIc/k90AgmK7CecAYeWmSr3553L4u5rk4sWAPBuQosfT7HmTfG4Rg5o4nGEA==
23382338
dependencies:
23392339
"@types/prop-types" "*"
23402340
"@types/scheduler" "*"
@@ -6097,10 +6097,10 @@ jscodeshift@^0.13.1:
60976097
temp "^0.8.4"
60986098
write-file-atomic "^2.3.0"
60996099

6100-
jsdom@22.0.0, jsdom@^20.0.0:
6101-
version "22.0.0"
6102-
resolved "https://registry.yarnpkg.com/jsdom/-/jsdom-22.0.0.tgz#3295c6992c70089c4b8f5cf060489fddf7ee9816"
6103-
integrity sha512-p5ZTEb5h+O+iU02t0GfEjAnkdYPrQSkfuTSMkMYyIoMvUNEHsbG0bHHbfXIcfTqD2UfvjQX7mmgiFsyRwGscVw==
6100+
jsdom@22.1.0, jsdom@^20.0.0:
6101+
version "22.1.0"
6102+
resolved "https://registry.yarnpkg.com/jsdom/-/jsdom-22.1.0.tgz#0fca6d1a37fbeb7f4aac93d1090d782c56b611c8"
6103+
integrity sha512-/9AVW7xNbsBv6GfWho4TTNjEo9fe6Zhf9O7s0Fhhr3u+awPwAJMKwAMXnkk5vBxflqLW9hTHX/0cs+P3gW+cQw==
61046104
dependencies:
61056105
abab "^2.0.6"
61066106
cssstyle "^3.0.0"

0 commit comments

Comments
 (0)