[Fiber] Detect useSyncExternalStore mutations missed while Activity tree was hidden

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

The problem

When a React Activity subtree is hidden, its passive effects (including store subscriptions from useSyncExternalStore) are disconnected. If the store changes while the subtree is hidden, and then the subtree is shown again (revealed) without a re-render, components could miss updates that happened while they were disconnected. This left some components with stale data, violating React's consistency guarantees.

Why it's non-trivial

A naive fix (such as always re-rendering on reveal, or only reconnecting effects if a render happened) would miss cases where:

• The store changed while the subtree was hidden and the component bailed out of rendering on reveal (e.g., due to memoization).
• A layout effect mutated the store during the reveal commit, after rendering but before resubscription. The solution must ensure that missed store updates are detected and handled, even if the component doesn't re-render on reveal.

The approach

The core fix is to always push the updateStoreInstance effect into the effect list, but only tag it as needing to run ("HasEffect") if something changed during render. This ensures the effect is always present to be reconnected, and can check for missed updates on reveal.

Key code changes:

1. Always push the effect, but only sometimes tag as needing to run

+  const storeChanged =
+    inst.getSnapshot !== getSnapshot ||
+    snapshotChanged ||
+    // Check if the subscribe function changed. We can save some memory by
+    // checking whether we scheduled a subscription effect above.
+    (workInProgressHook !== null &&
+      (workInProgressHook.memoizedState.tag & HookHasEffect) !== HookNoFlags);
+
+  // Even if nothing changed during this render, we push the effect so it is
+  // always in the effect list. That way it re-runs whenever the passive
+  // effects are reconnected, like when a hidden Activity tree is shown again.
+  // While the tree was hidden we were not subscribed to the store, so
+  // mutations during that window notified nobody, and if the reveal didn't
+  // re-render this component (or rendered before the mutation), nothing
+  // would ever detect them. When nothing changed, the effect is pushed
+  // without the HasEffect tag so a regular commit skips it.
+  pushSimpleEffect(
+    storeChanged ? HookHasEffect | HookPassive : HookPassive,
+    createEffectInstance(),
+    updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
+    null,
+  );

What this does:

• Calculates if the store or subscription changed (storeChanged).
• Always pushes the effect, but only marks it as needing to run (HookHasEffect) if something changed.
• This ensures the effect is present for reconnection after being hidden, so it can check for missed updates.

2. Only set the fiber's PassiveEffect flag if something changed

+  if (storeChanged) {
     fiber.flags |= PassiveEffect;
     // ... (existing consistency check scheduling)
   }

What this does:

• Only schedules the passive effect for the commit phase if something actually changed, to avoid unnecessary work.

3. Update comments to clarify the new logic

+  // Even if nothing changed during this render, we push the effect so it is
+  // always in the effect list. That way it re-runs whenever the passive
+  // effects are reconnected, like when a hidden Activity tree is shown again.
+  // While the tree was hidden we were not subscribed to the store, so
+  // mutations during that window notified nobody, and if the reveal didn't
+  // re-render this component (or rendered before the mutation), nothing
+  // would ever detect them. When nothing changed, the effect is pushed
+  // without the HasEffect tag so a regular commit skips it.

What this does:

• Documents the rationale for always pushing the effect, even if it doesn't need to run except on reconnection.

4. Add regression tests for both scenarios

Two new tests were added to cover:

• Store mutation in a layout effect during reveal.
• Store mutation while hidden, with the subtree bailing out of rendering on reveal (e.g., due to React.memo).

+  it('detects store mutations from a layout effect while an Activity subtree is being revealed', async () => {
+    // ... test logic ...
+  });
+
+  it(
+    'detects store mutations that happened while an Activity subtree was ' +
+      'hidden, even if the subtree bails out of rendering when revealed',
+    async () => {
+      // ... test logic ...
+    },
+  );

What this does:

• Ensures the new logic works for both subtle cases that previously led to stale state.

From the review thread

No notable review discussion on this PR.

Takeaways

• Always ensure effect lists are complete for all lifecycle transitions, not just renders. Effects that are only pushed during certain renders can lead to missed updates when subtrees are hidden and revealed.
• Test for edge cases involving memoization and lifecycle boundaries. Components that bail out of rendering (e.g., via React.memo) can expose subtle bugs in effect management.
• Passive effect reconnection must account for missed updates while disconnected. When resubscribing to external sources, always check for changes that could have occurred while unsubscribed.