Fix ignored setState in Safari when iframe is touched (#24459)

facebook · May 12, 2022 · 2c8a145 · 2c8a145
1 parent 6266263
commit 2c8a145
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 12 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js b/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js
@@ -16,7 +16,7 @@ let act;
 
 describe('ReactDOMSafariMicrotaskBug-test', () => {
   let container;
-  let simulateSafariBug;
+  let flushMicrotasksPrematurely;
 
   beforeEach(() => {
     // In Safari, microtasks don't always run on clean stack.
@@ -27,9 +27,12 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
     window.queueMicrotask = function(cb) {
       queue.push(cb);
     };
-    simulateSafariBug = function() {
-      queue.forEach(cb => cb());
-      queue = [];
+    flushMicrotasksPrematurely = function() {
+      while (queue.length > 0) {
+        const prevQueue = queue;
+        queue = [];
+        prevQueue.forEach(cb => cb());
+      }
     };
 
     jest.resetModules();
@@ -45,7 +48,7 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
     document.body.removeChild(container);
   });
 
-  it('should be resilient to buggy queueMicrotask', async () => {
+  it('should deal with premature microtask in commit phase', async () => {
     let ran = false;
     function Foo() {
       const [state, setState] = React.useState(0);
@@ -55,7 +58,7 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
             if (!ran) {
               ran = true;
               setState(1);
-              simulateSafariBug();
+              flushMicrotasksPrematurely();
             }
           }}>
           {state}
@@ -68,4 +71,30 @@ describe('ReactDOMSafariMicrotaskBug-test', () => {
     });
     expect(container.textContent).toBe('1');
   });
+
+  it('should deal with premature microtask in event handler', async () => {
+    function Foo() {
+      const [state, setState] = React.useState(0);
+      return (
+        <button
+          onClick={() => {
+            setState(1);
+            flushMicrotasksPrematurely();
+          }}>
+          {state}
+        </button>
+      );
+    }
+    const root = ReactDOMClient.createRoot(container);
+    await act(async () => {
+      root.render(<Foo />);
+    });
+    expect(container.textContent).toBe('0');
+    await act(async () => {
+      container.firstChild.dispatchEvent(
+        new MouseEvent('click', {bubbles: true}),
+      );
+    });
+    expect(container.textContent).toBe('1');
+  });
 });
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -835,9 +835,12 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
           // https://github.com/facebook/react/issues/22459
           // We don't support running callbacks in the middle of render
           // or commit so we need to check against that.
-          if (executionContext === NoContext) {
-            // It's only safe to do this conditionally because we always
-            // check for pending work before we exit the task.
+          if (
+            (executionContext & (RenderContext | CommitContext)) ===
+            NoContext
+          ) {
+            // Note that this would still prematurely flush the callbacks
+            // if this happens outside render or commit phase (e.g. in an event).
             flushSyncCallbacks();
           }
         });

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -835,9 +835,12 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
           // https://github.com/facebook/react/issues/22459
           // We don't support running callbacks in the middle of render
           // or commit so we need to check against that.
-          if (executionContext === NoContext) {
-            // It's only safe to do this conditionally because we always
-            // check for pending work before we exit the task.
+          if (
+            (executionContext & (RenderContext | CommitContext)) ===
+            NoContext
+          ) {
+            // Note that this would still prematurely flush the callbacks
+            // if this happens outside render or commit phase (e.g. in an event).
             flushSyncCallbacks();
           }
         });