Skip to content

Commit 361e84a

Browse files
acdliteacdlite
committed
Don't flush sync at end of discreteUpdates
All it should do is change the priority. The updates will be flushed by the microtask.
1 parent a155860 commit 361e84a

File tree

6 files changed

+41
-132
lines changed

6 files changed

+41
-132
lines changed

‎packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ describe('ReactDOMFiberAsync', () => {
427427
});
428428

429429
// @gate experimental
430-
it('ignores discrete events on a pending removed event listener', () => {
430+
it('ignores discrete events on a pending removed event listener', async () => {
431431
const disableButtonRef = React.createRef();
432432
const submitButtonRef = React.createRef();
433433

@@ -459,17 +459,19 @@ describe('ReactDOMFiberAsync', () => {
459459
}
460460

461461
const root = ReactDOM.unstable_createRoot(container);
462-
root.render(<Form />);
463-
// Flush
464-
Scheduler.unstable_flushAll();
462+
await act(async () => {
463+
root.render(<Form />);
464+
});
465465

466466
const disableButton = disableButtonRef.current;
467467
expect(disableButton.tagName).toBe('BUTTON');
468468

469469
// Dispatch a click event on the Disable-button.
470470
const firstEvent = document.createEvent('Event');
471471
firstEvent.initEvent('click', true, true);
472-
disableButton.dispatchEvent(firstEvent);
472+
await act(async () => {
473+
disableButton.dispatchEvent(firstEvent);
474+
});
473475

474476
// There should now be a pending update to disable the form.
475477

@@ -481,14 +483,16 @@ describe('ReactDOMFiberAsync', () => {
481483
const secondEvent = document.createEvent('Event');
482484
secondEvent.initEvent('click', true, true);
483485
// This should force the pending update to flush which disables the submit button before the event is invoked.
484-
submitButton.dispatchEvent(secondEvent);
486+
await act(async () => {
487+
submitButton.dispatchEvent(secondEvent);
488+
});
485489

486490
// Therefore the form should never have been submitted.
487491
expect(formSubmitted).toBe(false);
488492
});
489493

490494
// @gate experimental
491-
it('uses the newest discrete events on a pending changed event listener', () => {
495+
it('uses the newest discrete events on a pending changed event listener', async () => {
492496
const enableButtonRef = React.createRef();
493497
const submitButtonRef = React.createRef();
494498

@@ -515,17 +519,19 @@ describe('ReactDOMFiberAsync', () => {
515519
}
516520

517521
const root = ReactDOM.unstable_createRoot(container);
518-
root.render(<Form />);
519-
// Flush
520-
Scheduler.unstable_flushAll();
522+
await act(async () => {
523+
root.render(<Form />);
524+
});
521525

522526
const enableButton = enableButtonRef.current;
523527
expect(enableButton.tagName).toBe('BUTTON');
524528

525529
// Dispatch a click event on the Enable-button.
526530
const firstEvent = document.createEvent('Event');
527531
firstEvent.initEvent('click', true, true);
528-
enableButton.dispatchEvent(firstEvent);
532+
await act(async () => {
533+
enableButton.dispatchEvent(firstEvent);
534+
});
529535

530536
// There should now be a pending update to enable the form.
531537

@@ -537,7 +543,9 @@ describe('ReactDOMFiberAsync', () => {
537543
const secondEvent = document.createEvent('Event');
538544
secondEvent.initEvent('click', true, true);
539545
// This should force the pending update to flush which enables the submit button before the event is invoked.
540-
submitButton.dispatchEvent(secondEvent);
546+
await act(async () => {
547+
submitButton.dispatchEvent(secondEvent);
548+
});
541549

542550
// Therefore the form should have been submitted.
543551
expect(formSubmitted).toBe(true);

‎packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,9 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
357357
const pressEvent = document.createEvent('Event');
358358
pressEvent.initEvent('click', true, true);
359359
dispatchAndSetCurrentEvent(target.current, pressEvent);
360+
// Intentionally not using `act` so we can observe in between the press
361+
// event and the microtask, without batching.
362+
await null;
360363
// If this is 2, that means the `setCount` calls were not batched.
361364
expect(container.textContent).toEqual('Count: 1');
362365

@@ -409,11 +412,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
409412
dispatchAndSetCurrentEvent(target, pressEvent);
410413

411414
expect(Scheduler).toHaveYielded(['Count: 0 [after batchedUpdates]']);
412-
// TODO: There's a `flushDiscreteUpdates` call at the end of the event
413-
// delegation listener that gets called even if no React event handlers are
414-
// fired. Once that is removed, this will be 0, not 1.
415-
// expect(container.textContent).toEqual('Count: 0');
416-
expect(container.textContent).toEqual('Count: 1');
415+
expect(container.textContent).toEqual('Count: 0');
417416

418417
// Intentionally not using `act` so we can observe in between the click
419418
// event and the microtask, without batching.

‎packages/react-dom/src/events/ReactDOMUpdateBatching.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ function finishEventHandler() {
3939
// If a controlled event was fired, we may need to restore the state of
4040
// the DOM node back to the controlled value. This is necessary when React
4141
// bails out of the update without touching the DOM.
42+
// TODO: Restore state in the microtask, after the discrete updates flush,
43+
// instead of early flushing them here.
4244
flushDiscreteUpdatesImpl();
4345
restoreStateIfNeeded();
4446
}

‎packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js‎

Lines changed: 15 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ describe('SimpleEventPlugin', function() {
1313
let React;
1414
let ReactDOM;
1515
let Scheduler;
16-
let TestUtils;
16+
let act;
1717

1818
let onClick;
1919
let container;
@@ -40,7 +40,6 @@ describe('SimpleEventPlugin', function() {
4040
React = require('react');
4141
ReactDOM = require('react-dom');
4242
Scheduler = require('scheduler');
43-
TestUtils = require('react-dom/test-utils');
4443

4544
onClick = jest.fn();
4645
});
@@ -237,10 +236,12 @@ describe('SimpleEventPlugin', function() {
237236
React = require('react');
238237
ReactDOM = require('react-dom');
239238
Scheduler = require('scheduler');
239+
240+
act = require('react-dom/test-utils').unstable_concurrentAct;
240241
});
241242

242243
// @gate experimental
243-
it('flushes pending interactive work before exiting event handler', () => {
244+
it('flushes pending interactive work before exiting event handler', async () => {
244245
container = document.createElement('div');
245246
const root = ReactDOM.unstable_createRoot(container);
246247
document.body.appendChild(container);
@@ -288,7 +289,7 @@ describe('SimpleEventPlugin', function() {
288289
}
289290

290291
// Click the button to trigger the side-effect
291-
click();
292+
await act(async () => click());
292293
expect(Scheduler).toHaveYielded([
293294
// The handler fired
294295
'Side-effect',
@@ -312,6 +313,9 @@ describe('SimpleEventPlugin', function() {
312313
expect(Scheduler).toFlushAndYield([]);
313314
});
314315

316+
// NOTE: This test was written for the old behavior of discrete updates,
317+
// where they would be async, but flushed early if another discrete update
318+
// was dispatched.
315319
// @gate experimental
316320
it('end result of many interactive updates is deterministic', async () => {
317321
container = document.createElement('div');
@@ -355,121 +359,23 @@ describe('SimpleEventPlugin', function() {
355359
}
356360

357361
// Click the button a single time
358-
click();
362+
await act(async () => click());
359363
// The counter should update synchronously, even in concurrent mode.
360364
expect(button.textContent).toEqual('Count: 1');
361365

362366
// Click the button many more times
363-
await TestUtils.act(async () => {
364-
click();
365-
click();
366-
click();
367-
click();
368-
click();
369-
click();
370-
});
367+
await act(async () => click());
368+
await act(async () => click());
369+
await act(async () => click());
370+
await act(async () => click());
371+
await act(async () => click());
372+
await act(async () => click());
371373

372374
// Flush the remaining work
373375
Scheduler.unstable_flushAll();
374376
// The counter should equal the total number of clicks
375377
expect(button.textContent).toEqual('Count: 7');
376378
});
377-
378-
// @gate experimental
379-
it('flushes discrete updates in order', async () => {
380-
container = document.createElement('div');
381-
document.body.appendChild(container);
382-
383-
let button;
384-
class Button extends React.Component {
385-
state = {lowPriCount: 0};
386-
render() {
387-
const text = `High-pri count: ${this.props.highPriCount}, Low-pri count: ${this.state.lowPriCount}`;
388-
Scheduler.unstable_yieldValue(text);
389-
return (
390-
<button
391-
ref={el => (button = el)}
392-
onClick={() => {
393-
React.unstable_startTransition(() => {
394-
this.setState(state => ({
395-
lowPriCount: state.lowPriCount + 1,
396-
}));
397-
});
398-
}}>
399-
{text}
400-
</button>
401-
);
402-
}
403-
}
404-
405-
class Wrapper extends React.Component {
406-
state = {highPriCount: 0};
407-
render() {
408-
return (
409-
<div
410-
onClick={
411-
// Intentionally not using the updater form here, to test
412-
// that updates are serially processed.
413-
() => {
414-
this.setState({highPriCount: this.state.highPriCount + 1});
415-
}
416-
}>
417-
<Button highPriCount={this.state.highPriCount} />
418-
</div>
419-
);
420-
}
421-
}
422-
423-
// Initial mount
424-
const root = ReactDOM.unstable_createRoot(container);
425-
root.render(<Wrapper />);
426-
expect(Scheduler).toFlushAndYield([
427-
'High-pri count: 0, Low-pri count: 0',
428-
]);
429-
expect(button.textContent).toEqual('High-pri count: 0, Low-pri count: 0');
430-
431-
function click() {
432-
const event = new MouseEvent('click', {
433-
bubbles: true,
434-
cancelable: true,
435-
});
436-
Object.defineProperty(event, 'timeStamp', {
437-
value: 0,
438-
});
439-
button.dispatchEvent(event);
440-
}
441-
442-
// Click the button a single time.
443-
// This will flush at the end of the event, even in concurrent mode.
444-
click();
445-
expect(Scheduler).toHaveYielded(['High-pri count: 1, Low-pri count: 0']);
446-
447-
// Click the button many more times
448-
click();
449-
click();
450-
click();
451-
click();
452-
click();
453-
click();
454-
455-
// Each update should synchronously flush, even in concurrent mode.
456-
expect(Scheduler).toHaveYielded([
457-
'High-pri count: 2, Low-pri count: 0',
458-
'High-pri count: 3, Low-pri count: 0',
459-
'High-pri count: 4, Low-pri count: 0',
460-
'High-pri count: 5, Low-pri count: 0',
461-
'High-pri count: 6, Low-pri count: 0',
462-
'High-pri count: 7, Low-pri count: 0',
463-
]);
464-
465-
// Now flush the scheduler to apply the transition updates.
466-
// At the end, both counters should equal the total number of clicks.
467-
expect(Scheduler).toFlushAndYield([
468-
'High-pri count: 7, Low-pri count: 7',
469-
]);
470-
471-
expect(button.textContent).toEqual('High-pri count: 7, Low-pri count: 7');
472-
});
473379
});
474380

475381
describe('iOS bubbling click fix', function() {

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,9 +1160,6 @@ export function discreteUpdates<A, B, C, D, R>(
11601160
ReactCurrentBatchConfig.transition = prevTransition;
11611161
if (executionContext === NoContext) {
11621162
resetRenderTimer();
1163-
// TODO: This should only flush legacy sync updates. Not discrete updates
1164-
// in Concurrent Mode. Discrete updates will flush in a microtask.
1165-
flushSyncCallbacks();
11661163
}
11671164
}
11681165
}

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,9 +1160,6 @@ export function discreteUpdates<A, B, C, D, R>(
11601160
ReactCurrentBatchConfig.transition = prevTransition;
11611161
if (executionContext === NoContext) {
11621162
resetRenderTimer();
1163-
// TODO: This should only flush legacy sync updates. Not discrete updates
1164-
// in Concurrent Mode. Discrete updates will flush in a microtask.
1165-
flushSyncCallbacks();
11661163
}
11671164
}
11681165
}

0 commit comments

Comments
 (0)