From bfa66bf611f4023ba44c2bf502ae8c2255a2fe0f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 14 Jan 2022 18:51:57 +0000 Subject: [PATCH 1/3] Fix setState being ignored in Safari --- .../react-reconciler/src/ReactFiberWorkLoop.new.js | 10 +++++++++- .../react-reconciler/src/ReactFiberWorkLoop.old.js | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index b108da0106e42..df701edbb4917 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -708,7 +708,15 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // of `act`. ReactCurrentActQueue.current.push(flushSyncCallbacks); } else { - scheduleMicrotask(flushSyncCallbacks); + scheduleMicrotask(() => { + // In Safari, appending an iframe forces microtasks to run. + // 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) { + flushSyncCallbacks(); + } + }); } } else { // Flush the queue in an Immediate task. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index a32f8853337e6..c86f02fbb0481 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -708,7 +708,15 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // of `act`. ReactCurrentActQueue.current.push(flushSyncCallbacks); } else { - scheduleMicrotask(flushSyncCallbacks); + scheduleMicrotask(() => { + // In Safari, appending an iframe forces microtasks to run. + // 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) { + flushSyncCallbacks(); + } + }); } } else { // Flush the queue in an Immediate task. From 2547b440d9ddb3a2cab526e5a64d09ac9400f445 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 18 Jan 2022 17:59:46 +0000 Subject: [PATCH 2/3] Add a regression test --- .../ReactDOMSafariMicrotaskBug-test.js | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js diff --git a/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js b/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js new file mode 100644 index 0000000000000..1e12bb611ca78 --- /dev/null +++ b/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js @@ -0,0 +1,71 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; + +let ReactDOM; +let act; + +describe('ReactDOMSafariMicrotaskBug-test', () => { + let container; + let simulateSafariBug; + + beforeEach(() => { + // In Safari, microtasks don't always run on clean stack. + // This setup crudely approximates it. + // In reality, the sync flush happens when an iframe is added to the page. + // https://github.com/facebook/react/issues/22459 + let queue = []; + window.queueMicrotask = function(cb) { + queue.push(cb); + }; + simulateSafariBug = function() { + queue.forEach(cb => cb()); + queue = []; + }; + + jest.resetModules(); + container = document.createElement('div'); + React = require('react'); + ReactDOM = require('react-dom'); + act = require('jest-react').act; + + document.body.appendChild(container); + }); + + afterEach(() => { + document.body.removeChild(container); + }); + + it('should be resilient to buggy queueMicrotask', async () => { + let ran = false; + function Foo() { + const [state, setState] = React.useState(0); + return ( +
{ + if (!ran) { + ran = true; + setState(1); + simulateSafariBug(); + } + }}> + {state} +
+ ); + } + const root = ReactDOM.createRoot(container); + await act(async () => { + root.render(); + }); + expect(container.textContent).toBe('1'); + }); +}); From d610003e099e8e4e642d61b44eec70283cb17a55 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 18 Jan 2022 18:09:28 +0000 Subject: [PATCH 3/3] Add comment --- packages/react-reconciler/src/ReactFiberWorkLoop.new.js | 2 ++ packages/react-reconciler/src/ReactFiberWorkLoop.old.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index df701edbb4917..b4b333547c194 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -714,6 +714,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // 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. flushSyncCallbacks(); } }); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index c86f02fbb0481..d8bb61af50c84 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -714,6 +714,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // 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. flushSyncCallbacks(); } });