Fix form status reset when component state is updated

Project: facebook/react | Language: JavaScript | Concepts: consistency-models, error-handling, testing-strategy | Difficulty: senior
Link: https://github.com/react/react/pull/34075

The problem

When a form is submitted in React, the useFormStatus() hook is supposed to reflect the pending status of the form action. However, if a component inside the form updates its own local state (e.g., via useState), the form's pending status would incorrectly reset to false, even though the form submission was still in progress. This led to UI inconsistencies, such as loading spinners disappearing prematurely.

Why it's non-trivial

A naive fix might just update the form status in one place, but React's reconciliation and context propagation are complex. The host context for transitions must be managed carefully so that state changes within components do not break the propagation of the form's pending status. The timing and location of context updates are critical, especially in the presence of suspension, errors, or nested updates.

The approach

1. Added a regression test to capture the broken behavior:

+  it('form actions should retain status when nested state changes', async () => {
+    // ...setup omitted for brevity...
+    let rerenderUnrelatedStatus;
+    function UnrelatedStatus() {
+      const {pending} = useFormStatus();
+      const [counter, setCounter] = useState(0);
+      rerenderUnrelatedStatus = () => setCounter(n => n + 1);
+      Scheduler.log(`[unrelated form] pending: ${pending}, state: ${counter}`);
+    }
+    let rerenderTargetStatus;
+    function TargetStatus() {
+      const {pending} = useFormStatus();
+      const [counter, setCounter] = useState(0);
+      Scheduler.log(`[target form] pending: ${pending}, state: ${counter}`);
+      rerenderTargetStatus = () => setCounter(n => n + 1);
+    }
+    // ...render and assertions...
+    await submit(formRef.current);
+    assertLog(['[target form] pending: true, state: 0']);
+    await act(() => rerenderTargetStatus());
+    assertLog(['[target form] pending: true, state: 1']);
+    await act(() => rerenderUnrelatedStatus());
+    assertLog(['[unrelated form] pending: false, state: 1']);
+  });

This test ensures that updating local state in a form does not reset the pending status, and that unrelated forms are unaffected.

2. Propagate transition status in pushHostContext:

@@ -92,6 +96,21 @@ function pushHostContext(fiber: Fiber): void {
   const stateHook: Hook | null = fiber.memoizedState;
   if (stateHook !== null) {
+    // Propagate the current state to all the descendents.
+    // We use Context as an implementation detail for this.
+    //
+    // NOTE: This assumes that there cannot be nested transition providers,
+    // because the only renderer that implements this feature is React DOM,
+    // and forms cannot be nested. If we did support nested providers, then
+    // we would need to push a context value even for host fibers that
+    // haven't been upgraded yet.
+    const transitionStatus: TransitionStatus = stateHook.memoizedState;
+    if (isPrimaryRenderer) {
+      HostTransitionContext._currentValue = transitionStatus;
+    } else {
+      HostTransitionContext._currentValue2 = transitionStatus;
+    }

Here, the transition status from the form's state is explicitly pushed into the context at the right time, ensuring descendants see the correct value even if local state updates occur.

3. Clarified comment in updateHostComponent:

-    // This is intentionally set here instead of pushHostContext because
+    // We need to update it here because
     // pushHostContext gets called before we process the state hook, to avoid
     // a state mismatch in the event that something suspends.

This comment was updated to clarify the reasoning behind the timing of context updates, reflecting the new, safer approach.

From the review thread

Reviewer concern about context timing:

eps1lon: I'm not sure I fully understand this comment. If something suspends we pop back up. And the Component we're rendering (here: TransitionAwareHostComponent) should not be able to read the current Context just yet which is what pushing the context before the render would do, right? We can't nest Host Context anyway though. So this seems safe to move the push after the render.

Response and rationale:

sebmarkbage: A common issue is that CompleteWork and UnwindWork unconditionally calls popHostContext. That's called even when something in the component suspends because it doesn't know if we suspended before or after the push. There's no state to indicate that.

That's why we always pushHostContext and other push things immediately when we enter the update before anything could error or suspend.

Since renderTransitionAwareHostComponentWithHooks might suspend, I don't think it's safe to move the pushHostContext after it. In fact I'm surprised we don't have a test that fails for that case. Maybe we're lacking coverage or there's some other thing that covers it but it shouldn't be safe in general.

Takeaways

• Context propagation must occur before any code that might suspend or error, to ensure correct unwinding and state restoration.
• Tests for UI state consistency should cover both direct state changes and indirect effects (such as context propagation) to prevent regressions.
• When context is used as an implementation detail, document assumptions (like lack of nesting) to clarify why certain optimizations are safe.