The Lost Effect, chapter 3

wow, writing code is hard

packages/react-reconciler/src/ReactFiberClassComponent.js

@@ -51,7 +51,7 @@ import {
   requestCurrentTime,
   computeExpirationForFiber,
   scheduleWork,
-  flushPassiveEffectsBeforeSchedulingUpdateOnFiber,
+  flushPassiveEffects,
 } from './ReactFiberScheduler';
 
 const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
@@ -200,7 +200,7 @@ const classComponentUpdater = {
       update.callback = callback;
     }
 
-    flushPassiveEffectsBeforeSchedulingUpdateOnFiber(fiber);
+    flushPassiveEffects();
     enqueueUpdate(fiber, update);
     scheduleWork(fiber, expirationTime);
   },
@@ -220,7 +220,7 @@ const classComponentUpdater = {
       update.callback = callback;
     }
 
-    flushPassiveEffectsBeforeSchedulingUpdateOnFiber(fiber);
+    flushPassiveEffects();
     enqueueUpdate(fiber, update);
     scheduleWork(fiber, expirationTime);
   },
@@ -239,7 +239,7 @@ const classComponentUpdater = {
       update.callback = callback;
     }
 
-    flushPassiveEffectsBeforeSchedulingUpdateOnFiber(fiber);
+    flushPassiveEffects();
     enqueueUpdate(fiber, update);
     scheduleWork(fiber, expirationTime);
   },

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -85,7 +85,7 @@ import {
 } from './ReactFiberHostConfig';
 import {
   captureCommitPhaseError,
-  flushPassiveEffectsBeforeSchedulingUpdateOnFiber,
+  flushPassiveEffects,
   requestCurrentTime,
   scheduleWork,
 } from './ReactFiberScheduler';
@@ -457,7 +457,7 @@ function commitLifeCycles(
           timedOutAt: NoWork,
         };
         finishedWork.memoizedState = newState;
-        flushPassiveEffectsBeforeSchedulingUpdateOnFiber(finishedWork);
+        flushPassiveEffects();
         scheduleWork(finishedWork, Sync);
         return;
       }

packages/react-reconciler/src/ReactFiberHooks.js

@@ -32,7 +32,7 @@ import {
 import {
   scheduleWork,
   computeExpirationForFiber,
-  flushPassiveEffectsBeforeSchedulingUpdateOnFiber,
+  flushPassiveEffects,
   requestCurrentTime,
 } from './ReactFiberScheduler';
 
@@ -725,7 +725,7 @@ function dispatchAction<S, A>(
       callback: callback !== undefined ? callback : null,
       next: null,
     };
-    flushPassiveEffectsBeforeSchedulingUpdateOnFiber(fiber);
+    flushPassiveEffects();
     // Append the update to the end of the list.
     const last = queue.last;
     if (last === null) {

packages/react-reconciler/src/ReactFiberReconciler.js

@@ -52,7 +52,7 @@ import {
   syncUpdates,
   interactiveUpdates,
   flushInteractiveUpdates,
-  flushPassiveEffectsBeforeSchedulingUpdateOnFiber,
+  flushPassiveEffects,
 } from './ReactFiberScheduler';
 import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
 import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -147,7 +147,7 @@ function scheduleRootUpdate(
     update.callback = callback;
   }
 
-  flushPassiveEffectsBeforeSchedulingUpdateOnFiber(current);
+  flushPassiveEffects();
   enqueueUpdate(current, update);
   scheduleWork(current, expirationTime);
 

packages/react-reconciler/src/ReactFiberScheduler.js

@@ -578,33 +578,8 @@ function markLegacyErrorBoundaryAsFailed(instance: mixed) {
   }
 }
 
-function flushPassiveEffectsBeforeSchedulingUpdateOnFiber(fiber: Fiber) {
-  if (rootWithPendingPassiveEffects !== null) {
-    // TODO: This is an unfortunate extra loop. We end up traversing to the root
-    // again in scheduleWorkToRoot. But we have to do this one first because it
-    // needs to happen before adding an update to the queue, and
-    // scheduleWorkToRoot may perform a synchronous re-render. Maybe we can
-    // solve this with batchedUpdates, or with the equivalent in the Scheduler
-    // package.
-    let node = fiber;
-    do {
-      switch (node.tag) {
-        case HostRoot: {
-          const root: FiberRoot = node.stateNode;
-          flushPassiveEffects(root);
-          return;
-        }
-      }
-      node = node.return;
-    } while (node !== null);
-  }
-}
-
-function flushPassiveEffects(root: FiberRoot) {
-  if (
-    passiveEffectCallback !== null &&
-    root === rootWithPendingPassiveEffects
-  ) {
+function flushPassiveEffects() {
+  if (passiveEffectCallback !== null) {
     Schedule_cancelCallback(passiveEffectCallbackHandle);
     // We call the scheduled callback instead of commitPassiveEffects directly
     // to ensure tracing works correctly.
@@ -1248,7 +1223,7 @@ function renderRoot(
       'by a bug in React. Please file an issue.',
   );
 
-  flushPassiveEffects(root);
+  flushPassiveEffects();
 
   isWorking = true;
   ReactCurrentOwner.currentDispatcher = Dispatcher;
@@ -2613,5 +2588,5 @@ export {
   interactiveUpdates,
   flushInteractiveUpdates,
   computeUniqueAsyncExpiration,
-  flushPassiveEffectsBeforeSchedulingUpdateOnFiber,
+  flushPassiveEffects,
 };

packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js

@@ -717,6 +717,35 @@ describe('ReactHooks', () => {
       ]);
     });
 
+    it('flushes passive effects even if siblings schedule a new root', () => {
+      function PassiveEffect(props) {
+        useEffect(() => {
+          ReactNoop.yield('Passive effect');
+        }, []);
+        return <Text text="Passive" />;
+      }
+      function LayoutEffect(props) {
+        useLayoutEffect(() => {
+          ReactNoop.yield('Layout effect');
+          // Scheduling work shouldn't interfere with the queued passive effect
+          ReactNoop.renderToRootWithID(<Text text="New Root" />, 'root2');
+        });
+        return <Text text="Layout" />;
+      }
+      ReactNoop.render([<PassiveEffect key="p" />, <LayoutEffect key="l" />]);
+      expect(ReactNoop.flush()).toEqual([
+        'Passive',
+        'Layout',
+        'Layout effect',
+        'Passive effect',
+        'New Root',
+      ]);
+      expect(ReactNoop.getChildren()).toEqual([
+        span('Passive'),
+        span('Layout'),
+      ]);
+    });
+
     it(
       'flushes effects serially by flushing old effects before flushing ' +
         "new ones, if they haven't already fired",