@samitouri / QOS-React / commits / 2860e00cf8

[Fiber] Detect useSyncExternalStore mutations missed while Activity tree was hidden (#36947)

Fixes #27670. When an Activity subtree is hidden, its passive effects are disconnected, which unsubscribes from the store. On reveal, the effects are reconnected by replaying the fiber's effect list without a render, but if `updateStoreInstance` was not in the effect list then the component would be left stale. This happened both when a layout effect mutated the store during the reveal commit (after the subtree rendered but before it resubscribed) and when the store changed while hidden and the component bailed out of rendering during the reveal. We now push the updateStoreInstance effect unconditionally but tag it with HookHasEffect only under the same conditions as before, so regular commits skip it when nothing changed but reconnection always triggers it so it can trigger a rerender if appropriate.

Sophie Alpert committed Jul 20, 2026 at 23:39 UTC 2860e00cf8780dc2d59b87f3ff28ac88b908660f
2 files changed +148 -10
packages/react-reconciler/src/ReactFiberHooks.js
+22 -10
@@ -92,6 +92,7 @@ import {
92 FormReset,
93 } from './ReactFiberFlags';
94 import {
95 + NoFlags as HookNoFlags,
96 HasEffect as HookHasEffect,
97 Layout as HookLayout,
98 Passive as HookPassive,
@@ -1774,21 +1775,31 @@ function updateSyncExternalStore<T>(
1775 // commit phase if there was an interleaved mutation. In concurrent mode
1776 // this can happen all the time, but even in synchronous mode, an earlier
1777 // effect may have mutated the store.
1777 - if (
1778 + const storeChanged =
1779 inst.getSnapshot !== getSnapshot ||
1780 snapshotChanged ||
1781 // Check if the subscribe function changed. We can save some memory by
1782 // checking whether we scheduled a subscription effect above.
1783 (workInProgressHook !== null &&
1783 - workInProgressHook.memoizedState.tag & HookHasEffect)
1784 - ) {
1784 + (workInProgressHook.memoizedState.tag & HookHasEffect) !== HookNoFlags);
1785 +
1786 + // Even if nothing changed during this render, we push the effect so it is
1787 + // always in the effect list. That way it re-runs whenever the passive
1788 + // effects are reconnected, like when a hidden Activity tree is shown again.
1789 + // While the tree was hidden we were not subscribed to the store, so
1790 + // mutations during that window notified nobody, and if the reveal didn't
1791 + // re-render this component (or rendered before the mutation), nothing
1792 + // would ever detect them. When nothing changed, the effect is pushed
1793 + // without the HasEffect tag so a regular commit skips it.
1794 + pushSimpleEffect(
1795 + storeChanged ? HookHasEffect | HookPassive : HookPassive,
1796 + createEffectInstance(),
1797 + updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
1798 + null,
1799 + );
1800 +
1801 + if (storeChanged) {
1802 fiber.flags |= PassiveEffect;
1786 - pushSimpleEffect(
1787 - HookHasEffect | HookPassive,
1788 - createEffectInstance(),
1789 - updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
1790 - null,
1791 - );
1803
1804 // Unless we're rendering a blocking lane, schedule a consistency check.
1805 // Right before committing, we will walk the tree and check if any of the
@@ -1848,7 +1859,8 @@ function updateStoreInstance<T>(
1859 // Something may have been mutated in between render and commit. This could
1860 // have been in an event that fired before the passive effects, or it could
1861 // have been in a layout effect. In that case, we would have used the old
1851 - // snapsho and getSnapshot values to bail out. We need to check one more time.
1862 + // snapshot and getSnapshot values to bail out. We need to check one more
1863 + // time. This effect also re-runs when a hidden Activity tree is revealed.
1864 if (checkIfSnapshotChanged(inst)) {
1865 // Force a re-render.
1866 // We intentionally don't log update times and stacks here because this
packages/react-reconciler/src/__tests__/useSyncExternalStore-test.js
+126
@@ -352,6 +352,132 @@ describe('useSyncExternalStore', () => {
352 },
353 );
354
355 + // Regression test for https://github.com/facebook/react/issues/27670
356 + it('detects store mutations from a layout effect while an Activity subtree is being revealed', async () => {
357 + const store = createExternalStore('revision:1');
358 +
359 + function App({mode, revision}) {
360 + return (
361 + <React.Activity mode={mode}>
362 + <Wrapper revision={revision}>
363 + <Subscriber />
364 + </Wrapper>
365 + </React.Activity>
366 + );
367 + }
368 +
369 + function Wrapper({children, revision}) {
370 + useLayoutEffect(() => {
371 + store.set('revision:' + revision);
372 + }, [revision]);
373 +
374 + return (
375 + <>
376 + wrapper:{revision}
377 + {', '}
378 + {children}
379 + </>
380 + );
381 + }
382 +
383 + function Subscriber() {
384 + const revision = useSyncExternalStore(store.subscribe, store.getState);
385 + return <Text text={revision} />;
386 + }
387 +
388 + const root = ReactNoop.createRoot();
389 +
390 + // Mount the app
391 + await act(() => {
392 + root.render(<App mode="visible" revision="1" />);
393 + });
394 + assertLog(['revision:1']);
395 + expect(root).toMatchRenderedOutput('wrapper:1, revision:1');
396 + expect(store.getSubscriberCount()).toBe(1);
397 +
398 + // Hide the subtree. React unsubscribes from the store.
399 + await act(() => {
400 + root.render(<App mode="hidden" revision="1" />);
401 + });
402 + assertLog(['revision:1']);
403 + expect(store.getSubscriberCount()).toBe(0);
404 +
405 + // Show the subtree again. A layout effect mutates the store during the
406 + // reveal, after the Subscriber rendered but before it resubscribed. When
407 + // it resubscribes, it must detect the mutation it missed.
408 + await act(() => {
409 + root.render(<App mode="visible" revision="2" />);
410 + });
411 + assertLog(['revision:1', 'revision:2']);
412 + expect(store.getSubscriberCount()).toBe(1);
413 + expect(root).toMatchRenderedOutput('wrapper:2, revision:2');
414 + });
415 +
416 + // Regression test for https://github.com/facebook/react/issues/27670
417 + it(
418 + 'detects store mutations that happened while an Activity subtree was ' +
419 + 'hidden, even if the subtree bails out of rendering when revealed',
420 + async () => {
421 + const store = createExternalStore('initial');
422 +
423 + // Memoized so that revealing the Activity boundary doesn't re-render
424 + // the subscriber. This matches components memoized by React.memo or
425 + // React Compiler.
426 + const Subscriber = React.memo(({label}) => {
427 + const value = useSyncExternalStore(store.subscribe, store.getState);
428 + return <Text text={label + ':' + value} />;
429 + });
430 +
431 + function App({mode, label}) {
432 + return (
433 + <React.Activity mode={mode}>
434 + <Subscriber label={label} />
435 + </React.Activity>
436 + );
437 + }
438 +
439 + const root = ReactNoop.createRoot();
440 + await act(() => {
441 + root.render(<App mode="visible" label="a" />);
442 + });
443 + assertLog(['a:initial']);
444 + expect(root).toMatchRenderedOutput('a:initial');
445 + expect(store.getSubscriberCount()).toBe(1);
446 +
447 + // Re-render the subscriber once with different props, with no store
448 + // change. This replaces its effect list with one that contains only
449 + // the subscription effect, no interleaved mutation check.
450 + await act(() => {
451 + root.render(<App mode="visible" label="b" />);
452 + });
453 + assertLog(['b:initial']);
454 + expect(root).toMatchRenderedOutput('b:initial');
455 +
456 + // Hide the subtree. React unsubscribes from the store.
457 + await act(() => {
458 + root.render(<App mode="hidden" label="b" />);
459 + });
460 + expect(store.getSubscriberCount()).toBe(0);
461 +
462 + // Mutate the store while the subtree is hidden. Nothing is subscribed,
463 + // so no update is scheduled.
464 + await act(() => {
465 + store.set('updated');
466 + });
467 + assertLog([]);
468 +
469 + // Show the subtree again. The memoized component bails out of
470 + // rendering, so resubscribing to the store is the only chance to
471 + // detect the mutation that happened while it was hidden.
472 + await act(() => {
473 + root.render(<App mode="visible" label="b" />);
474 + });
475 + assertLog(['b:updated']);
476 + expect(store.getSubscriberCount()).toBe(1);
477 + expect(root).toMatchRenderedOutput('b:updated');
478 + },
479 + );
480 +
481 it('regression: does not infinite loop for only changing store reference in render', async () => {
482 let store = {value: {}};
483 let listeners = [];