diff --git a/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js b/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js index 8f874e2cdb8c4..c46f028b8244f 100644 --- a/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js +++ b/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js @@ -14,11 +14,60 @@ import type {EventSystemFlags} from '../EventSystemFlags'; import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; import type {FormStatus} from 'react-dom-bindings/src/shared/ReactDOMFormActions'; +import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; import {getFiberCurrentPropsFromNode} from '../../client/ReactDOMComponentTree'; import {startHostTransition} from 'react-reconciler/src/ReactFiberReconciler'; +import {didCurrentEventScheduleTransition} from 'react-reconciler/src/ReactFiberRootScheduler'; +import sanitizeURL from 'react-dom-bindings/src/shared/sanitizeURL'; +import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {SyntheticEvent} from '../SyntheticEvent'; +function coerceFormActionProp( + actionProp: mixed, +): string | (FormData => void | Promise) | null { + if ( + actionProp == null || + typeof actionProp === 'symbol' || + typeof actionProp === 'boolean' + ) { + return null; + } else if (typeof actionProp === 'function') { + return (actionProp: any); + } else { + if (__DEV__) { + checkAttributeStringCoercion(actionProp, 'action'); + } + return (sanitizeURL( + enableTrustedTypesIntegration ? actionProp : '' + (actionProp: any), + ): any); + } +} + +function createFormDataWithSubmitter( + form: HTMLFormElement, + submitter: HTMLInputElement | HTMLButtonElement, +) { + // The submitter's value should be included in the FormData. + // 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. 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 temp = submitter.ownerDocument.createElement('input'); + temp.name = submitter.name; + temp.value = submitter.value; + if (form.id) { + temp.setAttribute('form', form.id); + } + (submitter.parentNode: any).insertBefore(temp, submitter); + const formData = new FormData(form); + (temp.parentNode: any).removeChild(temp); + return formData; +} + /** * This plugin invokes action functions on forms, inputs and buttons if * the form doesn't prevent default. @@ -42,8 +91,10 @@ function extractEvents( } const formInst = maybeTargetInst; const form: HTMLFormElement = (nativeEventTarget: any); - let action = (getFiberCurrentPropsFromNode(form): any).action; - let submitter: null | HTMLInputElement | HTMLButtonElement = + let action = coerceFormActionProp( + (getFiberCurrentPropsFromNode(form): any).action, + ); + let submitter: null | void | HTMLInputElement | HTMLButtonElement = (nativeEvent: any).submitter; let submitterAction; if (submitter) { @@ -60,10 +111,6 @@ function extractEvents( } } - if (typeof action !== 'function') { - return; - } - const event = new SyntheticEvent( 'action', 'action', @@ -74,44 +121,60 @@ function extractEvents( function submitForm() { if (nativeEvent.defaultPrevented) { - // We let earlier events to prevent the action from submitting. - return; - } - // Prevent native navigation. - event.preventDefault(); - let formData; - if (submitter) { - // The submitter's value should be included in the FormData. - // 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. 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 temp = submitter.ownerDocument.createElement('input'); - temp.name = submitter.name; - temp.value = submitter.value; - if (form.id) { - temp.setAttribute('form', form.id); + // An earlier event prevented form submission. If a transition update was + // also scheduled, we should trigger a pending form status — even if + // no action function was provided. + if (didCurrentEventScheduleTransition()) { + // We're going to set the pending form status, but because the submission + // was prevented, we should not fire the action function. + const formData = submitter + ? createFormDataWithSubmitter(form, submitter) + : new FormData(form); + const pendingState: FormStatus = { + pending: true, + data: formData, + method: form.method, + action: action, + }; + if (__DEV__) { + Object.freeze(pendingState); + } + startHostTransition( + formInst, + pendingState, + // Pass `null` as the action + // TODO: Consider splitting up startHostTransition into two separate + // functions, one that sets the form status and one that invokes + // the action. + null, + formData, + ); + } else { + // No earlier event scheduled a transition. Exit without setting a + // pending form status. } - (submitter.parentNode: any).insertBefore(temp, submitter); - formData = new FormData(form); - (temp.parentNode: any).removeChild(temp); - } else { - formData = new FormData(form); - } + } else if (typeof action === 'function') { + // A form action was provided. Prevent native navigation. + event.preventDefault(); - const pendingState: FormStatus = { - pending: true, - data: formData, - method: form.method, - action: action, - }; - if (__DEV__) { - Object.freeze(pendingState); + // Dispatch the action and set a pending form status. + const formData = submitter + ? createFormDataWithSubmitter(form, submitter) + : new FormData(form); + const pendingState: FormStatus = { + pending: true, + data: formData, + method: form.method, + action: action, + }; + if (__DEV__) { + Object.freeze(pendingState); + } + startHostTransition(formInst, pendingState, action, formData); + } else { + // No earlier event prevented the default submission, and no action was + // provided. Exit without setting a pending form status. } - startHostTransition(formInst, pendingState, action, formData); } dispatchQueue.push({ diff --git a/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js b/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js index 0b3f478e33b19..ab36fc11199b1 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js @@ -25,7 +25,7 @@ type FormStatusPending = {| pending: true, data: FormData, method: string, - action: string | (FormData => void | Promise), + action: string | (FormData => void | Promise) | null, |}; export type FormStatus = FormStatusPending | FormStatusNotPending; diff --git a/packages/react-dom/src/__tests__/ReactDOMForm-test.js b/packages/react-dom/src/__tests__/ReactDOMForm-test.js index 5db3b2fd8a5be..824e41aa0398f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMForm-test.js @@ -35,10 +35,12 @@ describe('ReactDOMForm', () => { let ReactDOMClient; let Scheduler; let assertLog; + let assertConsoleErrorDev; let waitForThrow; let useState; let Suspense; let startTransition; + let useTransition; let use; let textCache; let useFormStatus; @@ -54,9 +56,12 @@ describe('ReactDOMForm', () => { act = require('internal-test-utils').act; assertLog = require('internal-test-utils').assertLog; waitForThrow = require('internal-test-utils').waitForThrow; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; useState = React.useState; Suspense = React.Suspense; startTransition = React.startTransition; + useTransition = React.useTransition; use = React.use; useFormStatus = ReactDOM.useFormStatus; requestFormReset = ReactDOM.requestFormReset; @@ -1782,4 +1787,262 @@ describe('ReactDOMForm', () => { // The form was reset even though the action didn't finish. expect(inputRef.current.value).toBe('Initial'); }); + + test( + 'useFormStatus is activated if startTransition is called ' + + 'inside preventDefault-ed submit event', + async () => { + function Output({value}) { + const {pending} = useFormStatus(); + return ; + } + + function App({value}) { + const [, startFormTransition] = useTransition(); + + function onSubmit(event) { + event.preventDefault(); + startFormTransition(async () => { + const updatedValue = event.target.elements.search.value; + Scheduler.log('Action started'); + await getText('Wait'); + Scheduler.log('Action finished'); + startTransition(() => root.render()); + }); + } + return ( +
+ +
+ +
+
+ ); + } + + const formRef = React.createRef(); + const inputRef = React.createRef(); + const outputRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + assertLog(['Initial']); + + // Update the input to something different + inputRef.current.value = 'Updated'; + + // Submit the form. + await submit(formRef.current); + // The form switches into a pending state. + assertLog(['Action started', 'Initial (pending...)']); + expect(outputRef.current.textContent).toBe('Initial (pending...)'); + + // While the submission is still pending, update the input again so we + // can check whether the form is reset after the action finishes. + inputRef.current.value = 'Updated again after submission'; + + // Resolve the async action + await act(() => resolveText('Wait')); + assertLog(['Action finished', 'Updated']); + expect(outputRef.current.textContent).toBe('Updated'); + + // Confirm that the form was not automatically reset (should call + // requestFormReset(formRef.current) to opt into this behavior) + expect(inputRef.current.value).toBe('Updated again after submission'); + }, + ); + + test.only('useFormStatus is not activated if startTransition is not called', async () => { + function Output({value}) { + const {pending} = useFormStatus(); + + return ( + + ); + } + + function App({value}) { + async function onSubmit(event) { + event.preventDefault(); + const updatedValue = event.target.elements.search.value; + Scheduler.log('Async event handler started'); + await getText('Wait'); + Scheduler.log('Async event handler finished'); + startTransition(() => root.render()); + } + return ( +
+ +
+ +
+
+ ); + } + + const formRef = React.createRef(); + const inputRef = React.createRef(); + const outputRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + assertLog(['Initial']); + + // Update the input to something different + inputRef.current.value = 'Updated'; + + // Submit the form. + await submit(formRef.current); + // Unlike the previous test, which uses startTransition to manually dispatch + // an action, this test uses a regular event handler, so useFormStatus is + // not activated. + assertLog(['Async event handler started']); + expect(outputRef.current.textContent).toBe('Initial'); + + // While the submission is still pending, update the input again so we + // can check whether the form is reset after the action finishes. + inputRef.current.value = 'Updated again after submission'; + + // Resolve the async action + await act(() => resolveText('Wait')); + assertLog(['Async event handler finished', 'Updated']); + expect(outputRef.current.textContent).toBe('Updated'); + + // Confirm that the form was not automatically reset (should call + // requestFormReset(formRef.current) to opt into this behavior) + expect(inputRef.current.value).toBe('Updated again after submission'); + }); + + test('useFormStatus is not activated if event is not preventDefault-ed ', async () => { + function Output({value}) { + const {pending} = useFormStatus(); + return ; + } + + function App({value}) { + const [, startFormTransition] = useTransition(); + + function onSubmit(event) { + // This event is not preventDefault-ed, so the default form submission + // happens, and useFormStatus is not activated. + startFormTransition(async () => { + const updatedValue = event.target.elements.search.value; + Scheduler.log('Action started'); + await getText('Wait'); + Scheduler.log('Action finished'); + startTransition(() => root.render()); + }); + } + return ( +
+ +
+ +
+
+ ); + } + + const formRef = React.createRef(); + const inputRef = React.createRef(); + const outputRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + assertLog(['Initial']); + + // Update the input to something different + inputRef.current.value = 'Updated'; + + // Submitting the form should trigger the default navigation behavior + await expect(submit(formRef.current)).rejects.toThrow( + 'Navigate to: http://localhost/', + ); + + // The useFormStatus hook was not activated + assertLog(['Action started', 'Initial']); + expect(outputRef.current.textContent).toBe('Initial'); + }); + + test('useFormStatus coerces the value of the "action" prop', async () => { + function Status() { + const {pending, action} = useFormStatus(); + + if (pending) { + Scheduler.log(action); + return 'Pending'; + } else { + return 'Not pending'; + } + } + + function Form({action}) { + const [, startFormTransition] = useTransition(); + + function onSubmit(event) { + event.preventDefault(); + // Schedule an empty action for no other purpose than to trigger the + // pending state. + startFormTransition(async () => {}); + } + return ( +
+ + + ); + } + + const formRef = React.createRef(); + const root = ReactDOMClient.createRoot(container); + + // Symbols are coerced to null + await act(() => root.render(
)); + assertConsoleErrorDev(['Invalid value for prop `action`']); + await submit(formRef.current); + assertLog([null]); + + // Booleans are coerced to null + await act(() => root.render()); + await submit(formRef.current); + assertLog([null]); + + // Strings are passed through + await act(() => root.render()); + await submit(formRef.current); + assertLog(['delete']); + + // Functions are passed through + const actionFn = () => {}; + await act(() => root.render()); + await submit(formRef.current); + assertLog([actionFn]); + + // Everything else is toString-ed + class MyAction { + toString() { + return 'stringified action'; + } + } + await act(() => root.render()); + await submit(formRef.current); + assertLog(['stringified action']); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 8a65e6170fb80..d49b8367d099b 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -2944,16 +2944,20 @@ function startTransition( } } +const noop = () => {}; + export function startHostTransition( formFiber: Fiber, pendingState: TransitionStatus, - callback: F => mixed, + action: (F => mixed) | null, formData: F, ): void { if (!enableAsyncActions) { // Form actions are enabled, but async actions are not. Call the function, // but don't handle any pending or error states. - callback(formData); + if (action !== null) { + action(formData); + } return; } @@ -2976,13 +2980,19 @@ export function startHostTransition( queue, pendingState, NoPendingHostTransition, - // TODO: We can avoid this extra wrapper, somehow. Figure out layering - // once more of this function is implemented. - () => { - // Automatically reset the form when the action completes. - requestFormReset(formFiber); - return callback(formData); - }, + // TODO: `startTransition` both sets the pending state and dispatches + // the action, if one is provided. Consider refactoring these two + // concerns to avoid the extra lambda. + + action === null + ? // No action was provided, but we still call `startTransition` to + // set the pending form status. + noop + : () => { + // Automatically reset the form when the action completes. + requestFormReset(formFiber); + return action(formData); + }, ); } diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index eee02d2cb162a..dc3dd366b8da9 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -482,3 +482,7 @@ export function requestTransitionLane( } return currentEventTransitionLane; } + +export function didCurrentEventScheduleTransition(): boolean { + return currentEventTransitionLane !== NoLane; +}