Skip to content

Commit

Permalink
Insert temporary input node to polyfill submitter argument in FormData (
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
sebmarkbage authored Apr 24, 2023
1 parent 9c58a0b commit 5e5342b
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 16 deletions.
9 changes: 6 additions & 3 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ function validateFormActionInDevelopment(
props: any,
) {
if (__DEV__) {
if (value == null) {
return;
}
if (tag === 'form') {
if (key === 'formAction') {
console.error(
Expand Down Expand Up @@ -483,6 +486,9 @@ function setProp(
case 'action':
case 'formAction': {
// TODO: Consider moving these special cases to the form, input and button tags.
if (__DEV__) {
validateFormActionInDevelopment(tag, key, value, props);
}
if (enableFormActions) {
if (typeof value === 'function') {
// Set a javascript URL that doesn't do anything. We don't expect this to be invoked
Expand Down Expand Up @@ -554,9 +560,6 @@ function setProp(
domElement.removeAttribute(key);
break;
}
if (__DEV__) {
validateFormActionInDevelopment(tag, key, value, props);
}
// `setAttribute` with objects becomes only `[object]` in IE8/9,
// ('' + value) makes it output the correct toString()-value.
if (__DEV__) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function extractEvents(
const formInst = maybeTargetInst;
const form: HTMLFormElement = (nativeEventTarget: any);
let action = (getFiberCurrentPropsFromNode(form): any).action;
const submitter: null | HTMLInputElement | HTMLButtonElement =
let submitter: null | HTMLInputElement | HTMLButtonElement =
(nativeEvent: any).submitter;
let submitterAction;
if (submitter) {
Expand All @@ -53,6 +53,9 @@ function extractEvents(
if (submitterAction != null) {
// The submitter overrides the form action.
action = submitterAction;
// If the action is a function, we don't want to pass its name
// value to the FormData since it's controlled by the server.
submitter = null;
}
}

Expand Down Expand Up @@ -81,18 +84,16 @@ function extractEvents(
// It should be in the document order in the form.
// Since the FormData constructor invokes the formdata event it also
// needs to be available before that happens so after construction it's too
// late. The easiest way to do this is to switch the form field to hidden,
// which is always included, and then back again. This does means that this
// is observable from the formdata event though.
// TODO: This tricky doesn't work on button elements. Consider inserting
// a fake node instead for that case.
// late. We use a temporary fake node for the duration of this event.
// TODO: FormData takes a second argument that it's the submitter but this
// is fairly new so not all browsers support it yet. Switch to that technique
// when available.
const type = submitter.type;
submitter.type = 'hidden';
const temp = submitter.ownerDocument.createElement('input');
temp.name = submitter.name;
temp.value = submitter.value;
(submitter.parentNode: any).insertBefore(temp, submitter);
formData = new FormData(form);
submitter.type = type;
(temp.parentNode: any).removeChild(temp);
} else {
formData = new FormData(form);
}
Expand Down
71 changes: 67 additions & 4 deletions packages/react-dom/src/__tests__/ReactDOMForm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ describe('ReactDOMForm', () => {

// @gate enableFormActions
it('can read the clicked button in the formdata event', async () => {
const ref = React.createRef();
const inputRef = React.createRef();
const buttonRef = React.createRef();
let button;
let title;

Expand All @@ -487,11 +488,13 @@ describe('ReactDOMForm', () => {
const root = ReactDOMClient.createRoot(container);
await act(async () => {
root.render(
// TODO: Test button element too.
<form action={action}>
<input type="text" name="title" defaultValue="hello" />
<input type="submit" name="button" value="save" />
<input type="submit" name="button" value="delete" ref={ref} />
<input type="submit" name="button" value="delete" ref={inputRef} />
<button name="button" value="edit" ref={buttonRef}>
Edit
</button>
</form>,
);
});
Expand All @@ -503,10 +506,70 @@ describe('ReactDOMForm', () => {
}
});

await submit(ref.current);
await submit(inputRef.current);

expect(button).toBe('delete');
expect(title).toBe(null);

await submit(buttonRef.current);

expect(button).toBe('edit');
expect(title).toBe('hello');

// Ensure that the type field got correctly restored
expect(inputRef.current.getAttribute('type')).toBe('submit');
expect(buttonRef.current.getAttribute('type')).toBe(null);
});

// @gate enableFormActions
it('excludes the submitter name when the submitter is a function action', async () => {
const inputRef = React.createRef();
const buttonRef = React.createRef();
let button;

function action(formData) {
// A function action cannot control the name since it might be controlled by the server
// so we need to make sure it doesn't get into the FormData.
button = formData.get('button');
}

const root = ReactDOMClient.createRoot(container);
await expect(async () => {
await act(async () => {
root.render(
<form>
<input
type="submit"
name="button"
value="delete"
ref={inputRef}
formAction={action}
/>
<button
name="button"
value="edit"
ref={buttonRef}
formAction={action}>
Edit
</button>
</form>,
);
});
}).toErrorDev([
'Cannot specify a "name" prop for a button that specifies a function as a formAction.',
]);

await submit(inputRef.current);

expect(button).toBe(null);

await submit(buttonRef.current);

expect(button).toBe(null);

// Ensure that the type field got correctly restored
expect(inputRef.current.getAttribute('type')).toBe('submit');
expect(buttonRef.current.getAttribute('type')).toBe(null);
});

// @gate enableFormActions || !__DEV__
Expand Down

0 comments on commit 5e5342b

Please sign in to comment.