Skip to content

Commit

Permalink
Fix setState ignored in Safari when iframe is added to DOM in the sam…
Browse files Browse the repository at this point in the history
…e commit (facebook#23111)

* Fix setState being ignored in Safari

* Add a regression test

* Add comment
  • Loading branch information
gaearon authored and zhengjitf committed Apr 15, 2022
1 parent fd7595d commit acca91a
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -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 (
<div
ref={() => {
if (!ran) {
ran = true;
setState(1);
simulateSafariBug();
}
}}>
{state}
</div>
);
}
const root = ReactDOM.createRoot(container);
await act(async () => {
root.render(<Foo />);
});
expect(container.textContent).toBe('1');
});
});
12 changes: 11 additions & 1 deletion packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,17 @@ 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) {
// It's only safe to do this conditionally because we always
// check for pending work before we exit the task.
flushSyncCallbacks();
}
});
}
} else {
// Flush the queue in an Immediate task.
Expand Down
12 changes: 11 additions & 1 deletion packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,17 @@ 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) {
// It's only safe to do this conditionally because we always
// check for pending work before we exit the task.
flushSyncCallbacks();
}
});
}
} else {
// Flush the queue in an Immediate task.
Expand Down

0 comments on commit acca91a

Please sign in to comment.