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 (#23111)

* Fix setState being ignored in Safari

* Add a regression test

* Add comment
  • Loading branch information
gaearon committed Jan 18, 2022
1 parent 51947a1 commit 790b524
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

1 comment on commit 790b524

@Nafsun
Copy link

@Nafsun Nafsun commented on 790b524 Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Please sign in to comment.