Add (Client) Functions as Form Actions #26674
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
There was a problem hiding this comment.
Stamping the parts I’m familiar with, will let @sophiebits handle the rest
| @@ -377,6 +475,50 @@ function setProp( | |||
| domElement.setAttribute(key, sanitizedValue); | |||
| break; | |||
| } | |||
| case 'action': | |||
| case 'formAction': { | |||
| // TODO: Consider moving these special cases to the form, input and button tags. | |||
There was a problem hiding this comment.
I decided to leave these here for now and not special case per tag. To keep the code smaller. Ideally these should follow the pattern of input, select and textarea instead.
That way we could more easily clear out props like target, method and encType when they're not relevant or set them to what they're supposed to be. That's what Fizz does. That probably also lead to hydration warnings but they also warn just in general.
| "consider using form.requestSubmit() instead. If you're trying to use " + | ||
| 'event.stopPropagation() in a submit event handler, consider also calling ' + | ||
| 'event.preventDefault().' + | ||
| "')", |
There was a problem hiding this comment.
This doesn't get minified but it doesn't have access to the shared helper anyway so we can't really do that the same way anyway. We could consider a shorter message which is what I did for SSR.
sebmarkbage
force-pushed
the
formactions
branch
4 times, most recently
from
a14141b
to
ffee4ce
Compare
Buttons with actions have to be wrapped in form elements. They can't be standalone unfortunately because it's the act of submitting the form that triggers the action. This also ensures that the progressive enhancement can work properly too.
sebmarkbage
force-pushed
the
formactions
branch
from
ffee4ce
to
c6b06b7
Compare
| didWarnFormActionTarget = true; | ||
| console.error( | ||
| 'Cannot specify a target for a form that specifies a function as the action. ' + | ||
| 'The function will always be executed in the same scope.', |
There was a problem hiding this comment.
maybe "same window" would be clearer?
packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js
Outdated
Show resolved
Hide resolved
| @@ -635,6 +636,83 @@ function pushStringAttribute( | |||
| } | |||
| } | |||
|
|
|||
| // Since this will likely be repeated a lot in the HTML, we use a more concise message | |||
There was a problem hiding this comment.
could make a $RF helper for this I guess?
There was a problem hiding this comment.
oh you mean to call a global. Yea, but this might also be opened in new tabs etc. which is annoying.
There was a problem hiding this comment.
I suppose it doesn't matter much if the error message isn't great for that case.
| innerHTML = propValue; | ||
| break; | ||
| case 'action': | ||
| formAction = propValue; |
There was a problem hiding this comment.
nit: confusing to call these variables form_
| continue; | ||
| } else if (hasFormActionURL) { | ||
| extraAttributes.delete(propKey.toLowerCase()); | ||
| warnForPropDifference(propKey, 'function', value); |
There was a problem hiding this comment.
nit: quotes in the warning :(
There was a problem hiding this comment.
The reason I did it this way for now, instead of a custom warning, is that we probably want to join all the differences together in a larger diff. So this will have to change as part of that. We could pass a fake function or something to indicate how this should be rendered I guess.
This can still possibly have a null formAction in which case React still handles it.
| const root = ReactDOMClient.createRoot(container); | ||
| await act(async () => { | ||
| root.render( | ||
| <form |
There was a problem hiding this comment.
If the form is not a React element is already handled by the event system itself.
This lets you pass a function to `<form action={...}>` or `<button
formAction={...}>` or `<input type="submit formAction={...}>`. This will
behave basically like a `javascript:` URL except not quite implemented
that way. This is a convenience for the `onSubmit={e => {
e.preventDefault(); const fromData = new FormData(e.target); ... }`
pattern.
You can still implement a custom `onSubmit` handler and if it calls
`preventDefault`, it won't invoke the action, just like it would if you
used a full page form navigation or javascript urls. It behaves just
like a navigation and we might implement it with the Navigation API in
the future.
Currently this is just a synchronous function but in a follow up this
will accept async functions, handle pending states and handle errors.
This is implemented by setting `javascript:` URLs, but these only exist
to trigger an error message if something goes wrong instead of
navigating away. Like if you called `stopPropagation` to prevent React
from handling it or if you called `form.submit()` instead of
`form.requestSubmit()` which by-passes the `submit` event. If CSP is
used to ban `javascript:` urls, those will trigger errors when these
URLs are invoked which would be a different error message but it's still
there to notify the user that something went wrong in the plumbing.
Next up is improving the SSR state with action replaying and progressive
enhancement.
| // 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 |
There was a problem hiding this comment.
You might consider leveraging https://www.npmjs.com/package/formdata-submitter-polyfill or borrowing its approach ... in addition to handling <button>, it also does the right thing with <input type="image">
There was a problem hiding this comment.
Or alternatively don't attempt to emulate it at all, but instead just pass the submitter to the constructor so that new browsers do the right thing for all submitter types, and if developers want to support old browsers they can polyfill it.
| // 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. | ||
| // TODO: FormData takes a second argument that it's the submitter but this |
There was a problem hiding this comment.
This is available in the latest versions of all major browsers now: https://developer.mozilla.org/en-US/docs/Web/API/FormData/FormData#browser_compatibility ... should be available in jest+jsdom very soon too 🤞
There was a problem hiding this comment.
Unfortunately not all browsers are truly evergreen. We support a lot of old Samsung TVs and many iOS users don't upgrade for years. So we can't use it yet. Not worth the added code to do the image one.
There was a problem hiding this comment.
Note that we do it a little differently now. https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js#L92-L97
There was a problem hiding this comment.
Edit GitHub didn't show me your second message when I wrote this reply, web apps are hard 😆
That makes sense, thanks for the added context 👍
Though if you're supporting old browsers, many don't support SubmitEvent so you won't have a submitter here in the first place. And for modern browsers, a typical edit new way looks more robust 👍submitter won't work with the type="hidden" hack (i.e. IME <button> is much more prevalent than <input type=submit>).
Since browsers that support SubmitEvent are fairly likely to be evergreen, would it be better to just remove the hack altogether and pass the submitter (if any) to FormData? Then you'd get more correct 1 behavior with less code.
Footnotes
-
In addition to not supporting image button submitters, the latest approach also doesn't support form-associated submitters that aren't descendants of the form (e.g.
<form id=foo>...</form><button form=foo name=outside />) ↩
There was a problem hiding this comment.
We don't support very old, like we don't support IE anymore, however we support somewhat old browser like TVs and iOS versions a year or so old. Point taken that maybe it's not even enough to rely on the submitter property on the event.
I'd expect us to only keep this hack around for maybe 6 months or so until we switch to the native version given this awkward transition stage.
This lets you pass a function to `<form action={...}>` or `<button
formAction={...}>` or `<input type="submit formAction={...}>`. This will
behave basically like a `javascript:` URL except not quite implemented
that way. This is a convenience for the `onSubmit={e => {
e.preventDefault(); const fromData = new FormData(e.target); ... }`
pattern.
You can still implement a custom `onSubmit` handler and if it calls
`preventDefault`, it won't invoke the action, just like it would if you
used a full page form navigation or javascript urls. It behaves just
like a navigation and we might implement it with the Navigation API in
the future.
Currently this is just a synchronous function but in a follow up this
will accept async functions, handle pending states and handle errors.
This is implemented by setting `javascript:` URLs, but these only exist
to trigger an error message if something goes wrong instead of
navigating away. Like if you called `stopPropagation` to prevent React
from handling it or if you called `form.submit()` instead of
`form.requestSubmit()` which by-passes the `submit` event. If CSP is
used to ban `javascript:` urls, those will trigger errors when these
URLs are invoked which would be a different error message but it's still
there to notify the user that something went wrong in the plumbing.
Next up is improving the SSR state with action replaying and progressive
enhancement.
This lets you pass a function to `<form action={...}>` or `<button
formAction={...}>` or `<input type="submit formAction={...}>`. This will
behave basically like a `javascript:` URL except not quite implemented
that way. This is a convenience for the `onSubmit={e => {
e.preventDefault(); const fromData = new FormData(e.target); ... }`
pattern.
You can still implement a custom `onSubmit` handler and if it calls
`preventDefault`, it won't invoke the action, just like it would if you
used a full page form navigation or javascript urls. It behaves just
like a navigation and we might implement it with the Navigation API in
the future.
Currently this is just a synchronous function but in a follow up this
will accept async functions, handle pending states and handle errors.
This is implemented by setting `javascript:` URLs, but these only exist
to trigger an error message if something goes wrong instead of
navigating away. Like if you called `stopPropagation` to prevent React
from handling it or if you called `form.submit()` instead of
`form.requestSubmit()` which by-passes the `submit` event. If CSP is
used to ban `javascript:` urls, those will trigger errors when these
URLs are invoked which would be a different error message but it's still
there to notify the user that something went wrong in the plumbing.
Next up is improving the SSR state with action replaying and progressive
enhancement.
DiffTrain build for commit c826dc5.
This lets you pass a function to
<form action={...}>or<button formAction={...}>or<input type="submit formAction={...}>. This will behave basically like ajavascript:URL except not quite implemented that way. This is a convenience for theonSubmit={e => { e.preventDefault(); const fromData = new FormData(e.target); ... }pattern.You can still implement a custom
onSubmithandler and if it callspreventDefault, it won't invoke the action, just like it would if you used a full page form navigation or javascript urls. It behaves just like a navigation and we might implement it with the Navigation API in the future.Currently this is just a synchronous function but in a follow up this will accept async functions, handle pending states and handle errors.
This is implemented by setting
javascript:URLs, but these only exist to trigger an error message if something goes wrong instead of navigating away. Like if you calledstopPropagationto prevent React from handling it or if you calledform.submit()instead ofform.requestSubmit()which by-passes thesubmitevent. If CSP is used to banjavascript:urls, those will trigger errors when these URLs are invoked which would be a different error message but it's still there to notify the user that something went wrong in the plumbing.Next up is improving the SSR state with action replaying and progressive enhancement.