From b7b40cd3e28b4bf3d531fce4d7b58b62dba892c8 Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Thu, 28 Apr 2022 17:38:04 +0100
Subject: [PATCH] Fix ignored setState in Safari when iframe is touched
---
.../ReactDOMSafariMicrotaskBug-test.js | 41 ++++++++++++++++---
.../src/ReactFiberWorkLoop.new.js | 9 ++--
.../src/ReactFiberWorkLoop.old.js | 9 ++--
3 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js b/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js
index 0ba0848d8e3cd..ef820ac77c6c3 100644
--- 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
index 89c880d868554..c99bacafb7bd6 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -834,9 +834,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
index 2f5dbe8530ce5..f896c6022f030 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -834,9 +834,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();
}
});