Skip to content

Commit d29e5bf

Browse files
sebmarkbageAndyPengc12
authored andcommitted
Insert temporary input node to polyfill submitter argument in FormData (facebook#26714)
Insert temporary input node to polyfill submitter argument in FormData. This works for buttons too and fixes a bug where the type attribute wasn't reset. I also exclude the submitter if it's a function action. This ensures that we don't include the generated "name" when the action is a server action. Conceptually that name doesn't exist.
1 parent 9f1b584 commit d29e5bf

File tree

3 files changed

+83
-16
lines changed

3 files changed

+83
-16
lines changed

packages/react-dom-bindings/src/client/ReactDOMComponent.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ function validateFormActionInDevelopment(
132132
props: any,
133133
) {
134134
if (__DEV__) {
135+
if (value == null) {
136+
return;
137+
}
135138
if (tag === 'form') {
136139
if (key === 'formAction') {
137140
console.error(
@@ -483,6 +486,9 @@ function setProp(
483486
case 'action':
484487
case 'formAction': {
485488
// TODO: Consider moving these special cases to the form, input and button tags.
489+
if (__DEV__) {
490+
validateFormActionInDevelopment(tag, key, value, props);
491+
}
486492
if (enableFormActions) {
487493
if (typeof value === 'function') {
488494
// Set a javascript URL that doesn't do anything. We don't expect this to be invoked
@@ -554,9 +560,6 @@ function setProp(
554560
domElement.removeAttribute(key);
555561
break;
556562
}
557-
if (__DEV__) {
558-
validateFormActionInDevelopment(tag, key, value, props);
559-
}
560563
// `setAttribute` with objects becomes only `[object]` in IE8/9,
561564
// ('' + value) makes it output the correct toString()-value.
562565
if (__DEV__) {

packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ function extractEvents(
4242
const formInst = maybeTargetInst;
4343
const form: HTMLFormElement = (nativeEventTarget: any);
4444
let action = (getFiberCurrentPropsFromNode(form): any).action;
45-
const submitter: null | HTMLInputElement | HTMLButtonElement =
45+
let submitter: null | HTMLInputElement | HTMLButtonElement =
4646
(nativeEvent: any).submitter;
4747
let submitterAction;
4848
if (submitter) {
@@ -53,6 +53,9 @@ function extractEvents(
5353
if (submitterAction != null) {
5454
// The submitter overrides the form action.
5555
action = submitterAction;
56+
// If the action is a function, we don't want to pass its name
57+
// value to the FormData since it's controlled by the server.
58+
submitter = null;
5659
}
5760
}
5861

@@ -81,18 +84,16 @@ function extractEvents(
8184
// It should be in the document order in the form.
8285
// Since the FormData constructor invokes the formdata event it also
8386
// needs to be available before that happens so after construction it's too
84-
// late. The easiest way to do this is to switch the form field to hidden,
85-
// which is always included, and then back again. This does means that this
86-
// is observable from the formdata event though.
87-
// TODO: This tricky doesn't work on button elements. Consider inserting
88-
// a fake node instead for that case.
87+
// late. We use a temporary fake node for the duration of this event.
8988
// TODO: FormData takes a second argument that it's the submitter but this
9089
// is fairly new so not all browsers support it yet. Switch to that technique
9190
// when available.
92-
const type = submitter.type;
93-
submitter.type = 'hidden';
91+
const temp = submitter.ownerDocument.createElement('input');
92+
temp.name = submitter.name;
93+
temp.value = submitter.value;
94+
(submitter.parentNode: any).insertBefore(temp, submitter);
9495
formData = new FormData(form);
95-
submitter.type = type;
96+
(temp.parentNode: any).removeChild(temp);
9697
} else {
9798
formData = new FormData(form);
9899
}

packages/react-dom/src/__tests__/ReactDOMForm-test.js

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,8 @@ describe('ReactDOMForm', () => {
475475

476476
// @gate enableFormActions
477477
it('can read the clicked button in the formdata event', async () => {
478-
const ref = React.createRef();
478+
const inputRef = React.createRef();
479+
const buttonRef = React.createRef();
479480
let button;
480481
let title;
481482

@@ -487,11 +488,13 @@ describe('ReactDOMForm', () => {
487488
const root = ReactDOMClient.createRoot(container);
488489
await act(async () => {
489490
root.render(
490-
// TODO: Test button element too.
491491
<form action={action}>
492492
<input type="text" name="title" defaultValue="hello" />
493493
<input type="submit" name="button" value="save" />
494-
<input type="submit" name="button" value="delete" ref={ref} />
494+
<input type="submit" name="button" value="delete" ref={inputRef} />
495+
<button name="button" value="edit" ref={buttonRef}>
496+
Edit
497+
</button>
495498
</form>,
496499
);
497500
});
@@ -503,10 +506,70 @@ describe('ReactDOMForm', () => {
503506
}
504507
});
505508

506-
await submit(ref.current);
509+
await submit(inputRef.current);
507510

508511
expect(button).toBe('delete');
509512
expect(title).toBe(null);
513+
514+
await submit(buttonRef.current);
515+
516+
expect(button).toBe('edit');
517+
expect(title).toBe('hello');
518+
519+
// Ensure that the type field got correctly restored
520+
expect(inputRef.current.getAttribute('type')).toBe('submit');
521+
expect(buttonRef.current.getAttribute('type')).toBe(null);
522+
});
523+
524+
// @gate enableFormActions
525+
it('excludes the submitter name when the submitter is a function action', async () => {
526+
const inputRef = React.createRef();
527+
const buttonRef = React.createRef();
528+
let button;
529+
530+
function action(formData) {
531+
// A function action cannot control the name since it might be controlled by the server
532+
// so we need to make sure it doesn't get into the FormData.
533+
button = formData.get('button');
534+
}
535+
536+
const root = ReactDOMClient.createRoot(container);
537+
await expect(async () => {
538+
await act(async () => {
539+
root.render(
540+
<form>
541+
<input
542+
type="submit"
543+
name="button"
544+
value="delete"
545+
ref={inputRef}
546+
formAction={action}
547+
/>
548+
<button
549+
name="button"
550+
value="edit"
551+
ref={buttonRef}
552+
formAction={action}>
553+
Edit
554+
</button>
555+
</form>,
556+
);
557+
});
558+
}).toErrorDev([
559+
'Cannot specify a "name" prop for a button that specifies a function as a formAction.',
560+
]);
561+
562+
await submit(inputRef.current);
563+
564+
expect(button).toBe(null);
565+
566+
await submit(buttonRef.current);
567+
568+
expect(button).toBe(null);
569+
570+
// Ensure that the type field got correctly restored
571+
expect(inputRef.current.getAttribute('type')).toBe('submit');
572+
expect(buttonRef.current.getAttribute('type')).toBe(null);
510573
});
511574

512575
// @gate enableFormActions || !__DEV__

0 commit comments

Comments
 (0)