Support useFormStatus in progressively-enhanced forms #29019
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
|
Comparing: 04b0588...a23e517 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
| return (actionProp: any); | ||
| } else { | ||
| if (__DEV__) { | ||
| checkAttributeStringCoercion(actionProp, 'action'); |
There was a problem hiding this comment.
Kind of annoying to need these even though we've checked that it's not a symbol already.
| const pendingState: FormStatus = { | ||
| pending: true, | ||
| data: formData, | ||
| method: form.method, |
There was a problem hiding this comment.
I noticed we were already reading the method from the form element here. Should I change this to coerce the React prop like we're doing with action? Should this be getAttribute instead? Unsure of all the implications.
There was a problem hiding this comment.
Yea, it's tricky. In this case it's good because we get the right default and it gets normalized to something that's easy to compare. E.g. you don't have to think of this enum having different cases.
However, for the action if it gets normalized the same thing is actually kind of annoying because now you have to think first normalizing your own url string to whatever the browser does.
So I could see the value in the normalization diverging here.
| @@ -60,10 +111,6 @@ function extractEvents( | |||
| } | |||
| } | |||
There was a problem hiding this comment.
The submitterAction that's coming from formAction needs to be coerced too. Technically it was wrong before because it wasn't ignoring boolean/symbol and was treating them as the submitter taking over.
There was a problem hiding this comment.
Also the submitter being set to null in that branch is not correct in the case where it's not a function. It previously just assumed that if it wasn't a function it would've exited early anyway.
|
Noting that this currently doesn't fully cover isomorphic startTransition which I assume is a follow up. Another thing is that even if you don't prevent default and it's not a function, ideally we should still update |
Yeah I'm going to do that in its own PR since it's technically a separate issue |
Makes sense, I'll think I'll do that in a separate PR too |
| if (submitterAction != null) { | ||
| submitterAction = coerceFormActionProp( | ||
| submitterProps | ||
| ? (submitterProps: any).formAction |
There was a problem hiding this comment.
Nit: You only really need to coerce this case, the getAttribute one will already be null | string.
When handling form action functions, we should coerce the submitter formAction prop using the same logic as ReactDOMComponent before deciding whether it should override the action on the <form>. Before this fix, if the submitter's `formAction` prop was a symbol or a boolean, it would block the form's action function from firing, but the correct behavior is that symbols and booleans should be treated as if they were null/empty, because that's how we treat those values when setting DOM properties.
Before this change, `useFormStatus` is only activated if a form is
submitted by an action function (either <form action={actionFn}> or
<button formAction={actionFn}>).
After this change, `useFormStatus` will also be activated if you call
`startTransition(actionFn)` inside a submit event handler that is
`preventDefault`-ed.
This is the last missing piece for implementing a custom `action` prop
that is progressively enhanced using `onSubmit` while maintaining the
same behavior as built-in form actions.
Here's the basic recipe for implementing a progressively-enhanced
form action:
```js
import {requestFormReset} from 'react-dom';
// To implement progressive enhancement, pass both a form action *and* a
// submit event handler. The action is used for submissions that happen
// before hydration, and the submit handler is used for submissions that
// happen after.
<form
action={action}
onSubmit={(event) => {
// After hydration, we upgrade the form with additional client-
// only behavior.
event.preventDefault();
// Manually dispatch the action.
startTransition(async () => {
// (Optional) Reset any uncontrolled inputs once the action is
// complete, like built-in form actions do.
requestFormReset(event.target);
// ...Do extra action-y stuff in here, like setting a custom
// optimistic state...
// Call the user-provided action
const formData = new FormData(event.target);
await action(formData);
});
}}
/>
```
Before this change,
useFormStatusis only activated if a form is submitted by an action function (either<form action={actionFn}>or<button formAction={actionFn}>).After this change,
useFormStatuswill also be activated if you callstartTransition(actionFn)inside a submit event handler that ispreventDefault-ed.This is the last missing piece for implementing a custom
actionprop that is progressively enhanced usingonSubmitwhile maintaining the same behavior as built-in form actions.Here's the basic recipe for implementing a progressively-enhanced form action. This would typically be implemented in your UI component library, not regular application code: