@samitouri / QOS-React / commits / 7a833dad95

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

Dan Abramov committed Mar 29, 2018 at 02:41 UTC 7a833dad95b3059ebfdfba44d3fa68e1301d8e6a
2 files changed +51 -1
packages/react-dom/src/__tests__/ReactDOMRoot-test.internal.js
+40
@@ -185,6 +185,46 @@ describe('ReactDOMRoot', () => {
185 expect(container.textContent).toEqual('Hi');
186 });
187
188 + it('applies setState in componentDidMount synchronously in a batch', done => {
189 + class App extends React.Component {
190 + state = {mounted: false};
191 + componentDidMount() {
192 + this.setState({
193 + mounted: true,
194 + });
195 + }
196 + render() {
197 + return this.state.mounted ? 'Hi' : 'Bye';
198 + }
199 + }
200 +
201 + const root = ReactDOM.createRoot(container);
202 + const batch = root.createBatch();
203 + batch.render(
204 + <AsyncMode>
205 + <App />
206 + </AsyncMode>,
207 + );
208 +
209 + flush();
210 +
211 + // Hasn't updated yet
212 + expect(container.textContent).toEqual('');
213 +
214 + let ops = [];
215 + batch.then(() => {
216 + // Still hasn't updated
217 + ops.push(container.textContent);
218 +
219 + // Should synchronously commit
220 + batch.commit();
221 + ops.push(container.textContent);
222 +
223 + expect(ops).toEqual(['', 'Hi']);
224 + done();
225 + });
226 + });
227 +
228 it('does not restart a completed batch when committing if there were no intervening updates', () => {
229 let ops = [];
230 function Foo(props) {
packages/react-reconciler/src/ReactFiberScheduler.js
+11 -1
@@ -1169,7 +1169,15 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
1169 interruptedBy = fiber;
1170 resetStack();
1171 }
1172 - if (nextRoot !== root || !isWorking) {
1172 + if (
1173 + // If we're in the render phase, we don't need to schedule this root
1174 + // for an update, because we'll do it before we exit...
1175 + !isWorking ||
1176 + isCommitting ||
1177 + // ...unless this is a different root than the one we're rendering.
1178 + nextRoot !== root
1179 + ) {
1180 + // Add this root to the root schedule.
1181 requestWork(root, expirationTime);
1182 }
1183 if (nestedUpdateCount > NESTED_UPDATE_LIMIT) {
@@ -1500,6 +1508,8 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
1508 nextFlushedRoot = root;
1509 nextFlushedExpirationTime = expirationTime;
1510 performWorkOnRoot(root, expirationTime, false);
1511 + // Flush any sync work that was scheduled by lifecycles
1512 + performSyncWork();
1513 finishRendering();
1514 }
1515