diff --git a/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js b/packages/react-dom/src/__tests__/ReactDOMSafariMicrotaskBug-test.js index 0ba0848d8e3c..ef820ac77c6c 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 ( + + ); + } + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); + 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 89c880d86855..c99bacafb7bd 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 2f5dbe8530ce..f896c6022f03 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(); } });