bugfix: useSyncExternalStore() seems to be not solid in avoiding tearing

packages/react-reconciler/src/ReactFiberHooks.js

@@ -1617,21 +1617,20 @@ function updateSyncExternalStore<T>(
       createEffectInstance(),
       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
+  // stores were mutated.
+  const root: FiberRoot | null = getWorkInProgressRoot();
 
-    // 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
-    // stores were mutated.
-    const root: FiberRoot | null = getWorkInProgressRoot();
-
-    if (root === null) {
-      throw new Error(
-        'Expected a work-in-progress root. This is a bug in React. Please file an issue.',
-      );
-    }
+  if (root === null) {
+    throw new Error(
+      'Expected a work-in-progress root. This is a bug in React. Please file an issue.',
+    );
+  }
 
-    if (!isHydrating && !includesBlockingLane(root, renderLanes)) {
-      pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot);
-    }
+  if (!isHydrating && !includesBlockingLane(root, renderLanes)) {
+    pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot);
   }
 
   return nextSnapshot;

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

@@ -16,6 +16,7 @@ let Scheduler;
 let act;
 let useLayoutEffect;
 let forwardRef;
+let useEffect;
 let useImperativeHandle;
 let useRef;
 let useState;
@@ -39,6 +40,7 @@ describe('useSyncExternalStore', () => {
     useLayoutEffect = React.useLayoutEffect;
     useImperativeHandle = React.useImperativeHandle;
     forwardRef = React.forwardRef;
+    useEffect = React.useEffect;
     useRef = React.useRef;
     useState = React.useState;
     useSyncExternalStore = React.useSyncExternalStore;
@@ -67,6 +69,9 @@ describe('useSyncExternalStore', () => {
           listeners.forEach(listener => listener());
         });
       },
+      setWithoutEmit(text) {
+        currentState = text;
+      },
       subscribe(listener) {
         listeners.add(listener);
         return () => listeners.delete(listener);
@@ -184,6 +189,94 @@ describe('useSyncExternalStore', () => {
     },
   );
 
+  test(
+    'detects interleaved mutations(without emitting) during a concurrent read before ' +
+      'layout effects fire',
+    async () => {
+      const store1 = createExternalStore(0);
+
+      const Child = forwardRef(({store, label}, ref) => {
+        const value = useSyncExternalStore(store.subscribe, store.getState);
+        useImperativeHandle(
+          ref,
+          () => {
+            return value;
+          },
+          [value],
+        );
+        return <Text text={label + value} />;
+      });
+
+      function App({store}) {
+        // eslint-disabxle-next-line no-unused-vars
+        const [, setCount] = useState(0);
+        const refA = useRef(null);
+        const refB = useRef(null);
+        const refC = useRef(null);
+        useLayoutEffect(() => {
+          // This layout effect reads children that depend on an external store.
+          // This demostrates whether the children are consistent when the
+          // layout phase runs.
+          const aText = refA.current;
+          const bText = refB.current;
+          const cText = refC.current;
+          Scheduler.log(
+            `Children observed during layout: A${aText}B${bText}C${cText}`,
+          );
+        });
+
+        // trigger re-render in concurrent mode
+        useEffect(() => {
+          startTransition(() => {
+            setCount(count => count + 1);
+          });
+        }, []);
+
+        return (
+          <>
+            <Child store={store} ref={refA} label="A" />
+            <Child store={store} ref={refB} label="B" />
+            <Child store={store} ref={refC} label="C" />
+          </>
+        );
+      }
+
+      const root = ReactNoop.createRoot();
+      await act(async () => {
+        // Start a mount in legacy mode
+        root.render(<App store={store1} />);
+
+        await waitFor([
+          'A0',
+          'B0',
+          'C0',
+          'Children observed during layout: A0B0C0',
+          'A0', // re-render starts
+          'B0',
+        ]);
+
+        // During an interleaved event, the store is mutated without emitting
+        store1.setWithoutEmit(1);
+
+        // Then we continue rendering.
+        await waitForAll([
+          // C reads a newer value from the store than A or B, which means they
+          // are inconsistent.
+          'C1',
+
+          // Before committing the layout effects, React detects that the store
+          // has been mutated. So it throws out the entire completed tree and
+          // re-renders the new values.
+          'A1',
+          'B1',
+          'C1',
+          // The layout effects reads consistent children.
+          'Children observed during layout: A1B1C1',
+        ]);
+      });
+    },
+  );
+
   test('next value is correctly cached when state is dispatched in render phase', async () => {
     const store = createExternalStore('value:initial');