Rethrow errors from form actions (#26689)

This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and
throw the error so it can be captured by an error boundary. The behavior
is the same if the `<form />` had an internal useTransition hook, which
is pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is
"upgraded" to become stateful, by lazily mounting a list of hooks. The
rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using
useTransition under-the-hood, it also handles pending states, too.
However, this pending state can't be observed until we add a new hook
for that purpose. I'll add this next.

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

@@ -14,6 +14,7 @@ import type {EventSystemFlags} from '../EventSystemFlags';
 import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
 
 import {getFiberCurrentPropsFromNode} from '../../client/ReactDOMComponentTree';
+import {startHostTransition} from 'react-reconciler/src/ReactFiberReconciler';
 
 import {SyntheticEvent} from '../SyntheticEvent';
 
@@ -24,7 +25,7 @@ import {SyntheticEvent} from '../SyntheticEvent';
 function extractEvents(
   dispatchQueue: DispatchQueue,
   domEventName: DOMEventName,
-  targetInst: null | Fiber,
+  maybeTargetInst: null | Fiber,
   nativeEvent: AnyNativeEvent,
   nativeEventTarget: null | EventTarget,
   eventSystemFlags: EventSystemFlags,
@@ -33,11 +34,12 @@ function extractEvents(
   if (domEventName !== 'submit') {
     return;
   }
-  if (!targetInst || targetInst.stateNode !== nativeEventTarget) {
+  if (!maybeTargetInst || maybeTargetInst.stateNode !== nativeEventTarget) {
     // If we're inside a parent root that itself is a parent of this root, then
     // its deepest target won't be the actual form that's being submitted.
     return;
   }
+  const formInst = maybeTargetInst;
   const form: HTMLFormElement = (nativeEventTarget: any);
   let action = (getFiberCurrentPropsFromNode(form): any).action;
   const submitter: null | HTMLInputElement | HTMLButtonElement =
@@ -94,8 +96,8 @@ function extractEvents(
     } else {
       formData = new FormData(form);
     }
-    // TODO: Deal with errors and pending state.
-    action(formData);
+
+    startHostTransition(formInst, action, formData);
   }
 
   dispatchQueue.push({

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

@@ -33,37 +33,163 @@ describe('ReactDOMForm', () => {
   let React;
   let ReactDOM;
   let ReactDOMClient;
+  let Scheduler;
+  let assertLog;
+  let useState;
+  let Suspense;
+  let startTransition;
+  let textCache;
 
   beforeEach(() => {
     jest.resetModules();
     React = require('react');
     ReactDOM = require('react-dom');
     ReactDOMClient = require('react-dom/client');
+    Scheduler = require('scheduler');
     act = require('internal-test-utils').act;
+    assertLog = require('internal-test-utils').assertLog;
+    useState = React.useState;
+    Suspense = React.Suspense;
+    startTransition = React.startTransition;
     container = document.createElement('div');
     document.body.appendChild(container);
+
+    textCache = new Map();
   });
 
+  function resolveText(text) {
+    const record = textCache.get(text);
+    if (record === undefined) {
+      const newRecord = {
+        status: 'resolved',
+        value: text,
+      };
+      textCache.set(text, newRecord);
+    } else if (record.status === 'pending') {
+      const thenable = record.value;
+      record.status = 'resolved';
+      record.value = text;
+      thenable.pings.forEach(t => t());
+    }
+  }
+  function resolveText(text) {
+    const record = textCache.get(text);
+    if (record === undefined) {
+      const newRecord = {
+        status: 'resolved',
+        value: text,
+      };
+      textCache.set(text, newRecord);
+    } else if (record.status === 'pending') {
+      const thenable = record.value;
+      record.status = 'resolved';
+      record.value = text;
+      thenable.pings.forEach(t => t());
+    }
+  }
+
+  function readText(text) {
+    const record = textCache.get(text);
+    if (record !== undefined) {
+      switch (record.status) {
+        case 'pending':
+          Scheduler.log(`Suspend! [${text}]`);
+          throw record.value;
+        case 'rejected':
+          throw record.value;
+        case 'resolved':
+          return record.value;
+      }
+    } else {
+      Scheduler.log(`Suspend! [${text}]`);
+      const thenable = {
+        pings: [],
+        then(resolve) {
+          if (newRecord.status === 'pending') {
+            thenable.pings.push(resolve);
+          } else {
+            Promise.resolve().then(() => resolve(newRecord.value));
+          }
+        },
+      };
+
+      const newRecord = {
+        status: 'pending',
+        value: thenable,
+      };
+      textCache.set(text, newRecord);
+
+      throw thenable;
+    }
+  }
+
+  function getText(text) {
+    const record = textCache.get(text);
+    if (record === undefined) {
+      const thenable = {
+        pings: [],
+        then(resolve) {
+          if (newRecord.status === 'pending') {
+            thenable.pings.push(resolve);
+          } else {
+            Promise.resolve().then(() => resolve(newRecord.value));
+          }
+        },
+      };
+      const newRecord = {
+        status: 'pending',
+        value: thenable,
+      };
+      textCache.set(text, newRecord);
+      return thenable;
+    } else {
+      switch (record.status) {
+        case 'pending':
+          return record.value;
+        case 'rejected':
+          return Promise.reject(record.value);
+        case 'resolved':
+          return Promise.resolve(record.value);
+      }
+    }
+  }
+
+  function Text({text}) {
+    Scheduler.log(text);
+    return text;
+  }
+
+  function AsyncText({text}) {
+    readText(text);
+    Scheduler.log(text);
+    return text;
+  }
+
   afterEach(() => {
     document.body.removeChild(container);
   });
 
-  function submit(submitter) {
-    const form = submitter.form || submitter;
-    if (!submitter.form) {
-      submitter = undefined;
-    }
-    const submitEvent = new Event('submit', {bubbles: true, cancelable: true});
-    submitEvent.submitter = submitter;
-    const returnValue = form.dispatchEvent(submitEvent);
-    if (!returnValue) {
-      return;
-    }
-    const action =
-      (submitter && submitter.getAttribute('formaction')) || form.action;
-    if (!/\s*javascript:/i.test(action)) {
-      throw new Error('Navigate to: ' + action);
-    }
+  async function submit(submitter) {
+    await act(() => {
+      const form = submitter.form || submitter;
+      if (!submitter.form) {
+        submitter = undefined;
+      }
+      const submitEvent = new Event('submit', {
+        bubbles: true,
+        cancelable: true,
+      });
+      submitEvent.submitter = submitter;
+      const returnValue = form.dispatchEvent(submitEvent);
+      if (!returnValue) {
+        return;
+      }
+      const action =
+        (submitter && submitter.getAttribute('formaction')) || form.action;
+      if (!/\s*javascript:/i.test(action)) {
+        throw new Error('Navigate to: ' + action);
+      }
+    });
   }
 
   // @gate enableFormActions
@@ -84,7 +210,7 @@ describe('ReactDOMForm', () => {
       );
     });
 
-    submit(ref.current);
+    await submit(ref.current);
 
     expect(foo).toBe('bar');
 
@@ -102,7 +228,7 @@ describe('ReactDOMForm', () => {
       );
     });
 
-    submit(ref.current);
+    await submit(ref.current);
 
     expect(foo).toBe('bar2');
   });
@@ -148,12 +274,12 @@ describe('ReactDOMForm', () => {
     expect(savedTitle).toBe(null);
     expect(deletedTitle).toBe(null);
 
-    submit(inputRef.current);
+    await submit(inputRef.current);
     expect(savedTitle).toBe('Hello');
     expect(deletedTitle).toBe(null);
     savedTitle = null;
 
-    submit(buttonRef.current);
+    await submit(buttonRef.current);
     expect(savedTitle).toBe(null);
     expect(deletedTitle).toBe('Hello');
     deletedTitle = null;
@@ -188,12 +314,12 @@ describe('ReactDOMForm', () => {
     expect(savedTitle).toBe(null);
     expect(deletedTitle).toBe(null);
 
-    submit(inputRef.current);
+    await submit(inputRef.current);
     expect(savedTitle).toBe('Hello2');
     expect(deletedTitle).toBe(null);
     savedTitle = null;
 
-    submit(buttonRef.current);
+    await submit(buttonRef.current);
     expect(savedTitle).toBe(null);
     expect(deletedTitle).toBe('Hello2');
 
@@ -218,7 +344,7 @@ describe('ReactDOMForm', () => {
       );
     });
 
-    submit(ref.current);
+    await submit(ref.current);
 
     expect(actionCalled).toBe(false);
   });
@@ -254,7 +380,7 @@ describe('ReactDOMForm', () => {
         '\n    in form (at **)',
     ]);
 
-    submit(ref.current);
+    await submit(ref.current);
 
     expect(data).toBe('innerinner');
   });
@@ -296,7 +422,7 @@ describe('ReactDOMForm', () => {
       );
     });
 
-    submit(ref.current);
+    await submit(ref.current);
 
     expect(bubbledSubmit).toBe(true);
     expect(outerCalled).toBe(0);
@@ -340,7 +466,7 @@ describe('ReactDOMForm', () => {
 
     innerContainerRef.current.appendChild(innerContainer);
 
-    submit(ref.current);
+    await submit(ref.current);
 
     expect(bubbledSubmit).toBe(true);
     expect(outerCalled).toBe(0);
@@ -377,7 +503,7 @@ describe('ReactDOMForm', () => {
       }
     });
 
-    submit(ref.current);
+    await submit(ref.current);
 
     expect(button).toBe('delete');
     expect(title).toBe(null);
@@ -407,7 +533,7 @@ describe('ReactDOMForm', () => {
 
     let nav;
     try {
-      submit(ref.current);
+      await submit(ref.current);
     } catch (x) {
       nav = x.message;
     }
@@ -443,11 +569,218 @@ describe('ReactDOMForm', () => {
     const node = container.getElementsByTagName('input')[0];
     let nav;
     try {
-      submit(node);
+      await submit(node);
     } catch (x) {
       nav = x.message;
     }
     expect(nav).toBe('Navigate to: http://example.com/submit');
     expect(actionCalled).toBe(false);
   });
+
+  // @gate enableFormActions
+  // @gate enableAsyncActions
+  it('form actions are transitions', async () => {
+    const formRef = React.createRef();
+
+    function App() {
+      const [state, setState] = useState('Initial');
+      return (
+        <form action={() => setState('Updated')} ref={formRef}>
+          <Suspense fallback={<Text text="Loading..." />}>
+            <AsyncText text={state} />
+          </Suspense>
+        </form>
+      );
+    }
+
+    const root = ReactDOMClient.createRoot(container);
+    await resolveText('Initial');
+    await act(() => root.render(<App />));
+    assertLog(['Initial']);
+    expect(container.textContent).toBe('Initial');
+
+    // This should suspend because form actions are implicitly wrapped
+    // in startTransition.
+    await submit(formRef.current);
+    assertLog(['Suspend! [Updated]', 'Loading...']);
+    expect(container.textContent).toBe('Initial');
+
+    await act(() => resolveText('Updated'));
+    assertLog(['Updated']);
+    expect(container.textContent).toBe('Updated');
+  });
+
+  // @gate enableFormActions
+  // @gate enableAsyncActions
+  it('multiple form actions', async () => {
+    const formRef = React.createRef();
+
+    function App() {
+      const [state, setState] = useState(0);
+      return (
+        <form action={() => setState(n => n + 1)} ref={formRef}>
+          <Suspense fallback={<Text text="Loading..." />}>
+            <AsyncText text={'Count: ' + state} />
+          </Suspense>
+        </form>
+      );
+    }
+
+    const root = ReactDOMClient.createRoot(container);
+    await resolveText('Count: 0');
+    await act(() => root.render(<App />));
+    assertLog(['Count: 0']);
+    expect(container.textContent).toBe('Count: 0');
+
+    // Update
+    await submit(formRef.current);
+    assertLog(['Suspend! [Count: 1]', 'Loading...']);
+    expect(container.textContent).toBe('Count: 0');
+
+    await act(() => resolveText('Count: 1'));
+    assertLog(['Count: 1']);
+    expect(container.textContent).toBe('Count: 1');
+
+    // Update again
+    await submit(formRef.current);
+    assertLog(['Suspend! [Count: 2]', 'Loading...']);
+    expect(container.textContent).toBe('Count: 1');
+
+    await act(() => resolveText('Count: 2'));
+    assertLog(['Count: 2']);
+    expect(container.textContent).toBe('Count: 2');
+  });
+
+  // @gate enableFormActions
+  it('form actions can be asynchronous', async () => {
+    const formRef = React.createRef();
+
+    function App() {
+      const [state, setState] = useState('Initial');
+      return (
+        <form
+          action={async () => {
+            Scheduler.log('Async action started');
+            await getText('Wait');
+            startTransition(() => setState('Updated'));
+          }}
+          ref={formRef}>
+          <Suspense fallback={<Text text="Loading..." />}>
+            <AsyncText text={state} />
+          </Suspense>
+        </form>
+      );
+    }
+
+    const root = ReactDOMClient.createRoot(container);
+    await resolveText('Initial');
+    await act(() => root.render(<App />));
+    assertLog(['Initial']);
+    expect(container.textContent).toBe('Initial');
+
+    await submit(formRef.current);
+    assertLog(['Async action started']);
+
+    await act(() => resolveText('Wait'));
+    assertLog(['Suspend! [Updated]', 'Loading...']);
+    expect(container.textContent).toBe('Initial');
+  });
+
+  it('sync errors in form actions can be captured by an error boundary', async () => {
+    if (gate(flags => !(flags.enableFormActions && flags.enableAsyncActions))) {
+      // TODO: Uncaught JSDOM errors fail the test after the scope has finished
+      // so don't work with the `gate` mechanism.
+      return;
+    }
+
+    class ErrorBoundary extends React.Component {
+      state = {error: null};
+      static getDerivedStateFromError(error) {
+        return {error};
+      }
+      render() {
+        if (this.state.error !== null) {
+          return <Text text={this.state.error.message} />;
+        }
+        return this.props.children;
+      }
+    }
+
+    const formRef = React.createRef();
+
+    function App() {
+      return (
+        <ErrorBoundary>
+          <form
+            action={() => {
+              throw new Error('Oh no!');
+            }}
+            ref={formRef}>
+            <Text text="Everything is fine" />
+          </form>
+        </ErrorBoundary>
+      );
+    }
+
+    const root = ReactDOMClient.createRoot(container);
+    await act(() => root.render(<App />));
+    assertLog(['Everything is fine']);
+    expect(container.textContent).toBe('Everything is fine');
+
+    await submit(formRef.current);
+    assertLog(['Oh no!', 'Oh no!']);
+    expect(container.textContent).toBe('Oh no!');
+  });
+
+  it('async errors in form actions can be captured by an error boundary', async () => {
+    if (gate(flags => !(flags.enableFormActions && flags.enableAsyncActions))) {
+      // TODO: Uncaught JSDOM errors fail the test after the scope has finished
+      // so don't work with the `gate` mechanism.
+      return;
+    }
+
+    class ErrorBoundary extends React.Component {
+      state = {error: null};
+      static getDerivedStateFromError(error) {
+        return {error};
+      }
+      render() {
+        if (this.state.error !== null) {
+          return <Text text={this.state.error.message} />;
+        }
+        return this.props.children;
+      }
+    }
+
+    const formRef = React.createRef();
+
+    function App() {
+      return (
+        <ErrorBoundary>
+          <form
+            action={async () => {
+              Scheduler.log('Async action started');
+              await getText('Wait');
+              throw new Error('Oh no!');
+            }}
+            ref={formRef}>
+            <Text text="Everything is fine" />
+          </form>
+        </ErrorBoundary>
+      );
+    }
+
+    const root = ReactDOMClient.createRoot(container);
+    await act(() => root.render(<App />));
+    assertLog(['Everything is fine']);
+    expect(container.textContent).toBe('Everything is fine');
+
+    await submit(formRef.current);
+    assertLog(['Async action started']);
+    expect(container.textContent).toBe('Everything is fine');
+
+    await act(() => resolveText('Wait'));
+    assertLog(['Oh no!', 'Oh no!']);
+    expect(container.textContent).toBe('Oh no!');
+  });
 });

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -107,6 +107,8 @@ import {
   enableUseMutableSource,
   enableFloat,
   enableHostSingletons,
+  enableFormActions,
+  enableAsyncActions,
 } from 'shared/ReactFeatureFlags';
 import isArray from 'shared/isArray';
 import shallowEqual from 'shared/shallowEqual';
@@ -208,6 +210,7 @@ import {
   checkDidRenderIdHook,
   bailoutHooks,
   replaySuspendedComponentWithHooks,
+  renderTransitionAwareHostComponentWithHooks,
 } from './ReactFiberHooks';
 import {stopProfilerTimerIfRunning} from './ReactProfilerTimer';
 import {
@@ -1620,6 +1623,23 @@ function updateHostComponent(
     workInProgress.flags |= ContentReset;
   }
 
+  if (enableFormActions && enableAsyncActions) {
+    const memoizedState = workInProgress.memoizedState;
+    if (memoizedState !== null) {
+      // This fiber has been upgraded to a stateful component. The only way
+      // happens currently is for form actions. We use hooks to track the
+      // pending and error state of the form.
+      //
+      // Once a fiber is upgraded to be stateful, it remains stateful for the
+      // rest of its lifetime.
+      renderTransitionAwareHostComponentWithHooks(
+        current,
+        workInProgress,
+        renderLanes,
+      );
+    }
+  }
+
   markRef(current, workInProgress);
   reconcileChildren(current, workInProgress, nextChildren, renderLanes);
   return workInProgress.child;

packages/react-reconciler/src/ReactFiberHooks.js

@@ -43,6 +43,7 @@ import {
   enableLegacyCache,
   debugRenderPhaseSideEffectsForStrictMode,
   enableAsyncActions,
+  enableFormActions,
 } from 'shared/ReactFeatureFlags';
 import {
   REACT_CONTEXT_TYPE,
@@ -80,7 +81,7 @@ import {
   higherEventPriority,
 } from './ReactEventPriorities';
 import {readContext, checkIfContextChanged} from './ReactFiberNewContext';
-import {HostRoot, CacheComponent} from './ReactWorkTags';
+import {HostRoot, CacheComponent, HostComponent} from './ReactWorkTags';
 import {
   LayoutStatic as LayoutStaticEffect,
   Passive as PassiveEffect,
@@ -753,6 +754,33 @@ function renderWithHooksAgain<Props, SecondArg>(
   return children;
 }
 
+export function renderTransitionAwareHostComponentWithHooks(
+  current: Fiber | null,
+  workInProgress: Fiber,
+  lanes: Lanes,
+): boolean {
+  if (!(enableFormActions && enableAsyncActions)) {
+    return false;
+  }
+  return renderWithHooks(
+    current,
+    workInProgress,
+    TransitionAwareHostComponent,
+    null,
+    null,
+    lanes,
+  );
+}
+
+export function TransitionAwareHostComponent(): boolean {
+  if (!(enableFormActions && enableAsyncActions)) {
+    return false;
+  }
+  const dispatcher = ReactCurrentDispatcher.current;
+  const [isPending] = dispatcher.useTransition();
+  return isPending;
+}
+
 export function checkDidRenderIdHook(): boolean {
   // This should be called immediately after every renderWithHooks call.
   // Conceptually, it's part of the return value of renderWithHooks; it's only a
@@ -2483,6 +2511,97 @@ function startTransition(
   }
 }
 
+export function startHostTransition<F>(
+  formFiber: Fiber,
+  callback: F => mixed,
+  formData: F,
+): void {
+  if (!enableFormActions) {
+    // Not implemented.
+    return;
+  }
+
+  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);
+    return;
+  }
+
+  if (formFiber.tag !== HostComponent) {
+    throw new Error(
+      'Expected the form instance to be a HostComponent. This ' +
+        'is a bug in React.',
+    );
+  }
+
+  let setPending;
+  if (formFiber.memoizedState === null) {
+    // Upgrade this host component fiber to be stateful. We're going to pretend
+    // it was stateful all along so we can reuse most of the implementation
+    // for function components and useTransition.
+    //
+    // Create the initial hooks used by useTransition. This is essentially an
+    // inlined version of mountTransition.
+    const queue: UpdateQueue<
+      Thenable<boolean> | boolean,
+      Thenable<boolean> | boolean,
+    > = {
+      pending: null,
+      lanes: NoLanes,
+      dispatch: null,
+      lastRenderedReducer: basicStateReducer,
+      lastRenderedState: false,
+    };
+    const stateHook: Hook = {
+      memoizedState: false,
+      baseState: false,
+      baseQueue: null,
+      queue: queue,
+      next: null,
+    };
+
+    const dispatch: (Thenable<boolean> | boolean) => void =
+      (dispatchSetState.bind(null, formFiber, queue): any);
+    setPending = queue.dispatch = dispatch;
+
+    // TODO: The only reason this second hook exists is to save a reference to
+    // the `dispatch` function. But we already store this on the state hook. So
+    // we can cheat and read it from there. Need to make this change to the
+    // regular `useTransition` implementation, too.
+    const transitionHook: Hook = {
+      memoizedState: dispatch,
+      baseState: null,
+      baseQueue: null,
+      queue: null,
+      next: null,
+    };
+
+    stateHook.next = transitionHook;
+
+    // Add the initial list of hooks to both fiber alternates. The idea is that
+    // the fiber had these hooks all along.
+    formFiber.memoizedState = stateHook;
+    const alternate = formFiber.alternate;
+    if (alternate !== null) {
+      alternate.memoizedState = stateHook;
+    }
+  } else {
+    // This fiber was already upgraded to be stateful.
+    const transitionHook: Hook = formFiber.memoizedState.next;
+    const dispatch: (Thenable<boolean> | boolean) => void =
+      transitionHook.memoizedState;
+    setPending = dispatch;
+  }
+
+  startTransition(
+    setPending,
+    // TODO: We can avoid this extra wrapper, somehow. Figure out layering
+    // once more of this function is implemented.
+    () => callback(formData),
+  );
+}
+
 function mountTransition(): [
   boolean,
   (callback: () => void, options?: StartTransitionOptions) => void,

packages/react-reconciler/src/ReactFiberReconciler.js

@@ -111,6 +111,7 @@ export {
   focusWithin,
   observeVisibleRects,
 } from './ReactTestSelectors';
+export {startHostTransition} from './ReactFiberHooks';
 
 type OpaqueRoot = FiberRoot;
 

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -2330,6 +2330,16 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
       );
       break;
     }
+    case HostComponent: {
+      // Some host components are stateful (that's how we implement form
+      // actions) but we don't bother to reuse the memoized state because it's
+      // not worth the extra code. The main reason to reuse the previous hooks
+      // is to reuse uncached promises, but we happen to know that the only
+      // promises that a host component might suspend on are definitely cached
+      // because they are controlled by us. So don't bother.
+      resetHooksOnUnwind();
+      // Fallthrough to the next branch.
+    }
     default: {
       // Other types besides function components are reset completely before
       // being replayed. Currently this only happens when a Usable type is

scripts/error-codes/codes.json

@@ -460,5 +460,6 @@
   "472": "Type %s is not supported as an argument to a Server Function.",
   "473": "React doesn't accept base64 encoded file uploads because we don't except form data passed from a browser to ever encode data that way. If that's the wrong assumption, we can easily fix it.",
   "474": "Suspense Exception: This is not a real error, and should not leak into userspace. If you're seeing this, it's likely a bug in React.",
-  "475": "Internal React Error: suspendedState null when it was expected to exists. Please report this as a React bug."
+  "475": "Internal React Error: suspendedState null when it was expected to exists. Please report this as a React bug.",
+  "476": "Expected the form instance to be a HostComponent. This is a bug in React."
 }