From 12c2e0c44b15f256c6144f2451473105a656b112 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Sun, 28 Mar 2021 21:05:28 -0500
Subject: [PATCH] flushSync: flush sync passive effects

If there are pending passive effects that were the result of a
sync/discrete update, `flushSync()` should flush them, so the caller can
observe the result.

We can skip passive effects that don't have sync/discrete priority.

This would replace the need for `flushDiscreteUpdates()`, which is
identical to `flushSync()` except for this.
---
 .../src/ReactFiberWorkLoop.new.js             |  8 +++
 .../src/ReactFiberWorkLoop.old.js             |  8 +++
 .../src/__tests__/ReactFlushSync-test.js      | 51 +++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 1cff8db1fe654..1a7d32de5db79 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -1148,6 +1148,14 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
 }
 
 export function flushSync<A, R>(fn: A => R, a: A): R {
+  if (includesSomeLane(pendingPassiveEffectsLanes, SyncLane)) {
+    // If there are pending passive effects with sync priority, flush them now,
+    // so the callback can observe the result.
+    if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
+      flushPassiveEffects();
+    }
+  }
+
   const prevExecutionContext = executionContext;
   executionContext |= BatchedContext;
 
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index 98f4d119e4088..14de690d759e5 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -1148,6 +1148,14 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
 }
 
 export function flushSync<A, R>(fn: A => R, a: A): R {
+  if (includesSomeLane(pendingPassiveEffectsLanes, SyncLane)) {
+    // If there are pending passive effects with sync priority, flush them now,
+    // so the callback can observe the result.
+    if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
+      flushPassiveEffects();
+    }
+  }
+
   const prevExecutionContext = executionContext;
   executionContext |= BatchedContext;
 
diff --git a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
index 3c96f5ee65914..907a240f6de83 100644
--- a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
@@ -54,4 +54,55 @@ describe('ReactFlushSync', () => {
     });
     expect(root).toMatchRenderedOutput('1, 1');
   });
+
+  test('flushes pending passive effects if they were the result of a sync render', async () => {
+    function App() {
+      useEffect(() => {
+        Scheduler.unstable_yieldValue('Effect');
+      }, []);
+      return <Text text="Child" />;
+    }
+
+    const root = ReactNoop.createRoot();
+    await ReactNoop.act(async () => {
+      ReactNoop.flushSync(() => {
+        root.render(<App />);
+      });
+      expect(Scheduler).toHaveYielded([
+        'Child',
+        // Passive effect hasn't fired yet
+      ]);
+      expect(root).toMatchRenderedOutput('Child');
+
+      // Because the pending effect was the result of a sync update, calling
+      // flushSync should flush it.
+      ReactNoop.flushSync();
+      expect(Scheduler).toHaveYielded(['Effect']);
+    });
+  });
+
+  test("does not flush pending passive effects if they weren't the result of a sync render", async () => {
+    function App() {
+      useEffect(() => {
+        Scheduler.unstable_yieldValue('Effect');
+      }, []);
+      return <Text text="Child" />;
+    }
+
+    const root = ReactNoop.createRoot();
+    await ReactNoop.act(async () => {
+      root.render(<App />);
+      expect(Scheduler).toFlushUntilNextPaint([
+        'Child',
+        // Passive effect hasn't fired yet
+      ]);
+      expect(root).toMatchRenderedOutput('Child');
+
+      // Because the pending effect was not the result of a sync update, calling
+      // flushSync should not flush it.
+      ReactNoop.flushSync();
+      expect(Scheduler).toHaveYielded([]);
+    });
+    expect(Scheduler).toHaveYielded(['Effect']);
+  });
 });