Flush passive effects before interactive updates

facebook · Mar 18, 2019 · 0342959 · 0342959
1 parent 4162f60
commit 0342959
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 8 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js
@@ -179,4 +179,36 @@ describe('ReactDOMHooks', () => {
 
     expect(labelRef.current.innerHTML).toBe('abc');
   });
+
+  it('should flush passive effects before interactive events', () => {
+    const {useState, useEffect} = React;
+
+    function Foo() {
+      const [count, setCount] = useState(0);
+      const [enabled, setEnabled] = useState(true);
+      useEffect(() => {
+        return () => {
+          setEnabled(false);
+        };
+      });
+      function handleClick() {
+        setCount(x => x + 1);
+      }
+      return <button onClick={enabled ? handleClick : null}>{count}</button>;
+    }
+
+    ReactDOM.render(<Foo />, container);
+    container.firstChild.dispatchEvent(
+      new Event('click', {bubbles: true, cancelable: true}),
+    );
+    // Cleanup from first passive effect should remove the handler.
+    container.firstChild.dispatchEvent(
+      new Event('click', {bubbles: true, cancelable: true}),
+    );
+    container.firstChild.dispatchEvent(
+      new Event('click', {bubbles: true, cancelable: true}),
+    );
+    jest.runAllTimers();
+    expect(container.textContent).toBe('1');
+  });
 });
diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js
@@ -2541,14 +2541,8 @@ function interactiveUpdates<A, B, C, R>(
   // This needs to happen before we read any handlers, because the effect of
   // the previous event may influence which handlers are called during
   // this event.
-  if (
-    !isBatchingUpdates &&
-    !isRendering &&
-    lowestPriorityPendingInteractiveExpirationTime !== NoWork
-  ) {
-    // Synchronously flush pending interactive updates.
-    performWork(lowestPriorityPendingInteractiveExpirationTime, false);
-    lowestPriorityPendingInteractiveExpirationTime = NoWork;
+  if (!isBatchingUpdates) {
+    flushInteractiveUpdates();
   }
   const previousIsBatchingInteractiveUpdates = isBatchingInteractiveUpdates;
   const previousIsBatchingUpdates = isBatchingUpdates;
@@ -2571,6 +2565,7 @@ function flushInteractiveUpdates() {
     lowestPriorityPendingInteractiveExpirationTime !== NoWork
   ) {
     // Synchronously flush pending interactive updates.
+    flushPassiveEffects();
     performWork(lowestPriorityPendingInteractiveExpirationTime, false);
     lowestPriorityPendingInteractiveExpirationTime = NoWork;
   }