[pull] main from react:main

packages/react-reconciler/src/ReactFiberHooks.js

@@ -92,6 +92,7 @@ import {
   FormReset,
 } from './ReactFiberFlags';
 import {
+  NoFlags as HookNoFlags,
   HasEffect as HookHasEffect,
   Layout as HookLayout,
   Passive as HookPassive,
@@ -1774,21 +1775,31 @@ function updateSyncExternalStore<T>(
   // commit phase if there was an interleaved mutation. In concurrent mode
   // this can happen all the time, but even in synchronous mode, an earlier
   // effect may have mutated the store.
-  if (
+  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)
-  ) {
+      (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,
+  );
+
+  if (storeChanged) {
     fiber.flags |= PassiveEffect;
-    pushSimpleEffect(
-      HookHasEffect | HookPassive,
-      createEffectInstance(),
-      updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot),
-      null,
-    );
 
     // Unless we're rendering a blocking lane, schedule a consistency check.
     // Right before committing, we will walk the tree and check if any of the
@@ -1848,7 +1859,8 @@ function updateStoreInstance<T>(
   // Something may have been mutated in between render and commit. This could
   // have been in an event that fired before the passive effects, or it could
   // have been in a layout effect. In that case, we would have used the old
-  // snapsho and getSnapshot values to bail out. We need to check one more time.
+  // snapshot and getSnapshot values to bail out. We need to check one more
+  // time. This effect also re-runs when a hidden Activity tree is revealed.
   if (checkIfSnapshotChanged(inst)) {
     // Force a re-render.
     // We intentionally don't log update times and stacks here because this

packages/react-reconciler/src/__tests__/useSyncExternalStore-test.js

@@ -352,6 +352,132 @@ describe('useSyncExternalStore', () => {
     },
   );
 
+  // Regression test for https://github.com/facebook/react/issues/27670
+  it('detects store mutations from a layout effect while an Activity subtree is being revealed', async () => {
+    const store = createExternalStore('revision:1');
+
+    function App({mode, revision}) {
+      return (
+        <React.Activity mode={mode}>
+          <Wrapper revision={revision}>
+            <Subscriber />
+          </Wrapper>
+        </React.Activity>
+      );
+    }
+
+    function Wrapper({children, revision}) {
+      useLayoutEffect(() => {
+        store.set('revision:' + revision);
+      }, [revision]);
+
+      return (
+        <>
+          wrapper:{revision}
+          {', '}
+          {children}
+        </>
+      );
+    }
+
+    function Subscriber() {
+      const revision = useSyncExternalStore(store.subscribe, store.getState);
+      return <Text text={revision} />;
+    }
+
+    const root = ReactNoop.createRoot();
+
+    // Mount the app
+    await act(() => {
+      root.render(<App mode="visible" revision="1" />);
+    });
+    assertLog(['revision:1']);
+    expect(root).toMatchRenderedOutput('wrapper:1, revision:1');
+    expect(store.getSubscriberCount()).toBe(1);
+
+    // Hide the subtree. React unsubscribes from the store.
+    await act(() => {
+      root.render(<App mode="hidden" revision="1" />);
+    });
+    assertLog(['revision:1']);
+    expect(store.getSubscriberCount()).toBe(0);
+
+    // Show the subtree again. A layout effect mutates the store during the
+    // reveal, after the Subscriber rendered but before it resubscribed. When
+    // it resubscribes, it must detect the mutation it missed.
+    await act(() => {
+      root.render(<App mode="visible" revision="2" />);
+    });
+    assertLog(['revision:1', 'revision:2']);
+    expect(store.getSubscriberCount()).toBe(1);
+    expect(root).toMatchRenderedOutput('wrapper:2, revision:2');
+  });
+
+  // Regression test for https://github.com/facebook/react/issues/27670
+  it(
+    'detects store mutations that happened while an Activity subtree was ' +
+      'hidden, even if the subtree bails out of rendering when revealed',
+    async () => {
+      const store = createExternalStore('initial');
+
+      // Memoized so that revealing the Activity boundary doesn't re-render
+      // the subscriber. This matches components memoized by React.memo or
+      // React Compiler.
+      const Subscriber = React.memo(({label}) => {
+        const value = useSyncExternalStore(store.subscribe, store.getState);
+        return <Text text={label + ':' + value} />;
+      });
+
+      function App({mode, label}) {
+        return (
+          <React.Activity mode={mode}>
+            <Subscriber label={label} />
+          </React.Activity>
+        );
+      }
+
+      const root = ReactNoop.createRoot();
+      await act(() => {
+        root.render(<App mode="visible" label="a" />);
+      });
+      assertLog(['a:initial']);
+      expect(root).toMatchRenderedOutput('a:initial');
+      expect(store.getSubscriberCount()).toBe(1);
+
+      // Re-render the subscriber once with different props, with no store
+      // change. This replaces its effect list with one that contains only
+      // the subscription effect, no interleaved mutation check.
+      await act(() => {
+        root.render(<App mode="visible" label="b" />);
+      });
+      assertLog(['b:initial']);
+      expect(root).toMatchRenderedOutput('b:initial');
+
+      // Hide the subtree. React unsubscribes from the store.
+      await act(() => {
+        root.render(<App mode="hidden" label="b" />);
+      });
+      expect(store.getSubscriberCount()).toBe(0);
+
+      // Mutate the store while the subtree is hidden. Nothing is subscribed,
+      // so no update is scheduled.
+      await act(() => {
+        store.set('updated');
+      });
+      assertLog([]);
+
+      // Show the subtree again. The memoized component bails out of
+      // rendering, so resubscribing to the store is the only chance to
+      // detect the mutation that happened while it was hidden.
+      await act(() => {
+        root.render(<App mode="visible" label="b" />);
+      });
+      assertLog(['b:updated']);
+      expect(store.getSubscriberCount()).toBe(1);
+      expect(root).toMatchRenderedOutput('b:updated');
+    },
+  );
+
   it('regression: does not infinite loop for only changing store reference in render', async () => {
     let store = {value: {}};
     let listeners = [];