From 7a833dad95b3059ebfdfba44d3fa68e1301d8e6a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 29 Mar 2018 02:41:42 +0100 Subject: [PATCH] setState() in componentDidMount() should flush synchronously even with createBatch() (#12466) * Add a failing test for setState in cDM during batch.commit() * Copy pasta * Flush all follow-up Sync work on the committed batch * Nit: Use performSyncWork Call performSyncWork right after flushing the batch. Does effectively the same thing by reusing the existing function. Also added some comments. * Delete accidentally duplicated test --- .../__tests__/ReactDOMRoot-test.internal.js | 40 +++++++++++++++++++ .../src/ReactFiberScheduler.js | 12 +++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMRoot-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMRoot-test.internal.js index cd00c561b6320..0ece449718c1a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMRoot-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMRoot-test.internal.js @@ -185,6 +185,46 @@ describe('ReactDOMRoot', () => { expect(container.textContent).toEqual('Hi'); }); + it('applies setState in componentDidMount synchronously in a batch', done => { + class App extends React.Component { + state = {mounted: false}; + componentDidMount() { + this.setState({ + mounted: true, + }); + } + render() { + return this.state.mounted ? 'Hi' : 'Bye'; + } + } + + const root = ReactDOM.createRoot(container); + const batch = root.createBatch(); + batch.render( + + + , + ); + + flush(); + + // Hasn't updated yet + expect(container.textContent).toEqual(''); + + let ops = []; + batch.then(() => { + // Still hasn't updated + ops.push(container.textContent); + + // Should synchronously commit + batch.commit(); + ops.push(container.textContent); + + expect(ops).toEqual(['', 'Hi']); + done(); + }); + }); + it('does not restart a completed batch when committing if there were no intervening updates', () => { let ops = []; function Foo(props) { diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 7f90fdba7ba40..1aaa20a4c03f7 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -1169,7 +1169,15 @@ export default function( interruptedBy = fiber; resetStack(); } - if (nextRoot !== root || !isWorking) { + if ( + // If we're in the render phase, we don't need to schedule this root + // for an update, because we'll do it before we exit... + !isWorking || + isCommitting || + // ...unless this is a different root than the one we're rendering. + nextRoot !== root + ) { + // Add this root to the root schedule. requestWork(root, expirationTime); } if (nestedUpdateCount > NESTED_UPDATE_LIMIT) { @@ -1500,6 +1508,8 @@ export default function( nextFlushedRoot = root; nextFlushedExpirationTime = expirationTime; performWorkOnRoot(root, expirationTime, false); + // Flush any sync work that was scheduled by lifecycles + performSyncWork(); finishRendering(); }