Repository: facebook/react
Pull request: Fix ignored setState in Safari when iframe is touched

Fixes https://github.com/facebook/react/issues/24458.

Just like https://github.com/facebook/react/issues/22459, this is caused by a Safari bug (https://bugs.webkit.org/show_bug.cgi?id=235322).

Our original solution for https://github.com/facebook/react/issues/22459 was https://github.com/facebook/react/pull/23111: if the microtask callback runs prematurely, we ignore it. This relied on the assumption that we always check for more work at the end of the task anyway. However, we don't check for more work if we aren't either rendering or committing — notably, we don't check for more work at the end of the event handler.

This fix is to allow the update to flush early. This doesn't respect batching but matches what Safari is trying to do (which is to flush everything early).

Changed files:
filename: packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js
patch: @@ -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');
+  });
 });

filename: packages/react-reconciler/src/ReactFiberWorkLoop.new.js
patch: @@ -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();
           }
         });

filename: packages/react-reconciler/src/ReactFiberWorkLoop.old.js
patch: @@ -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();
           }
         });