Skip to content

Commit f77c7b9

Browse files
trueadmtrueadm
authored
Re-add discrete flushing timeStamp heuristic (behind flag) (#19540)
1 parent e67a6b1 commit f77c7b9

13 files changed

+78
-17
lines changed

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

+15-4
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,17 @@ describe('ReactDOMFiber', () => {
10311031
consthandlerA=()=>ops.push('A');
10321032
consthandlerB=()=>ops.push('B');
10331033

1034+
functionclick(){
1035+
constevent=newMouseEvent('click',{
1036+
bubbles: true,
1037+
cancelable: true,
1038+
});
1039+
Object.defineProperty(event,'timeStamp',{
1040+
value: 0,
1041+
});
1042+
node.dispatchEvent(event);
1043+
}
1044+
10341045
classExampleextendsReact.Component{
10351046
state={flip: false,count: 0};
10361047
flip(){
@@ -1064,23 +1075,23 @@ describe('ReactDOMFiber', () => {
10641075
constnode=container.firstChild;
10651076
expect(node.tagName).toEqual('DIV');
10661077

1067-
node.click();
1078+
click();
10681079

10691080
expect(ops).toEqual(['A']);
10701081
ops=[];
10711082

10721083
// Render with the other event handler.
10731084
inst.flip();
10741085

1075-
node.click();
1086+
click();
10761087

10771088
expect(ops).toEqual(['B']);
10781089
ops=[];
10791090

10801091
// Rerender without changing any props.
10811092
inst.tick();
10821093

1083-
node.click();
1094+
click();
10841095

10851096
expect(ops).toEqual(['B']);
10861097
ops=[];
@@ -1100,7 +1111,7 @@ describe('ReactDOMFiber', () => {
11001111
ops=[];
11011112

11021113
// Any click that happens after commit, should invoke A.
1103-
node.click();
1114+
click();
11041115
expect(ops).toEqual(['A']);
11051116
});
11061117

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ function dispatchDiscreteEvent(
130130
// flushed for this event and we don't need to do it again.
131131
(eventSystemFlags&IS_LEGACY_FB_SUPPORT_MODE)===0
132132
){
133-
flushDiscreteUpdatesIfNeeded();
133+
flushDiscreteUpdatesIfNeeded(nativeEvent.timeStamp);
134134
}
135135
discreteUpdates(
136136
dispatchEvent,

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

+27-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
needsStateRestore,
1010
restoreStateIfNeeded,
1111
}from'./ReactDOMControlledComponent';
12+
import{enableDiscreteEventFlushingChange}from'shared/ReactFeatureFlags';
1213

1314
// Used as a way to call batchedUpdates when we don't have a reference to
1415
// the renderer. Such as when we're dispatching events or if third party
@@ -87,9 +88,32 @@ export function discreteUpdates(fn, a, b, c, d) {
8788
}
8889
}
8990

90-
exportfunctionflushDiscreteUpdatesIfNeeded(){
91-
if(!isInsideEventHandler){
92-
flushDiscreteUpdatesImpl();
91+
letlastFlushedEventTimeStamp=0;
92+
exportfunctionflushDiscreteUpdatesIfNeeded(timeStamp: number){
93+
if(enableDiscreteEventFlushingChange){
94+
// event.timeStamp isn't overly reliable due to inconsistencies in
95+
// how different browsers have historically provided the time stamp.
96+
// Some browsers provide high-resolution time stamps for all events,
97+
// some provide low-resolution time stamps for all events. FF < 52
98+
// even mixes both time stamps together. Some browsers even report
99+
// negative time stamps or time stamps that are 0 (iOS9) in some cases.
100+
// Given we are only comparing two time stamps with equality (!==),
101+
// we are safe from the resolution differences. If the time stamp is 0
102+
// we bail-out of preventing the flush, which can affect semantics,
103+
// such as if an earlier flush removes or adds event listeners that
104+
// are fired in the subsequent flush. However, this is the same
105+
// behaviour as we had before this change, so the risks are low.
106+
if(
107+
!isInsideEventHandler&&
108+
(timeStamp===0||lastFlushedEventTimeStamp!==timeStamp)
109+
){
110+
lastFlushedEventTimeStamp=timeStamp;
111+
flushDiscreteUpdatesImpl();
112+
}
113+
}else{
114+
if(!isInsideEventHandler){
115+
flushDiscreteUpdatesImpl();
116+
}
93117
}
94118
}
95119

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

+24-9
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,14 @@ describe('SimpleEventPlugin', function() {
275275
expect(Scheduler).toFlushAndYield(['render button: enabled']);
276276

277277
functionclick(){
278-
button.dispatchEvent(
279-
newMouseEvent('click',{bubbles: true,cancelable: true}),
280-
);
278+
constevent=newMouseEvent('click',{
279+
bubbles: true,
280+
cancelable: true,
281+
});
282+
Object.defineProperty(event,'timeStamp',{
283+
value: 0,
284+
});
285+
button.dispatchEvent(event);
281286
}
282287

283288
// Click the button to trigger the side-effect
@@ -340,9 +345,14 @@ describe('SimpleEventPlugin', function() {
340345
expect(button.textContent).toEqual('Count: 0');
341346

342347
functionclick(){
343-
button.dispatchEvent(
344-
newMouseEvent('click',{bubbles: true,cancelable: true}),
345-
);
348+
constevent=newMouseEvent('click',{
349+
bubbles: true,
350+
cancelable: true,
351+
});
352+
Object.defineProperty(event,'timeStamp',{
353+
value: 0,
354+
});
355+
button.dispatchEvent(event);
346356
}
347357

348358
// Click the button a single time
@@ -421,9 +431,14 @@ describe('SimpleEventPlugin', function() {
421431
expect(button.textContent).toEqual('High-pri count: 0, Low-pri count: 0');
422432

423433
functionclick(){
424-
button.dispatchEvent(
425-
newMouseEvent('click',{bubbles: true,cancelable: true}),
426-
);
434+
constevent=newMouseEvent('click',{
435+
bubbles: true,
436+
cancelable: true,
437+
});
438+
Object.defineProperty(event,'timeStamp',{
439+
value: 0,
440+
});
441+
button.dispatchEvent(event);
427442
}
428443

429444
// Click the button a single time

‎packages/shared/ReactFeatureFlags.js

+2
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,5 @@ export const deferRenderPhaseUpdateToNextBatch = true;
125125

126126
// Replacement for runWithPriority in React internals.
127127
exportconstdecoupleUpdatePriorityFromScheduler=false;
128+
129+
exportconstenableDiscreteEventFlushingChange=false;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4848
exportconstenableNewReconciler=false;
4949
exportconstdeferRenderPhaseUpdateToNextBatch=true;
5050
exportconstdecoupleUpdatePriorityFromScheduler=false;
51+
exportconstenableDiscreteEventFlushingChange=false;
5152

5253
// Flow magic to verify the exports of this file match the original version.
5354
// eslint-disable-next-line no-unused-vars

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
exportconstenableNewReconciler=false;
4848
exportconstdeferRenderPhaseUpdateToNextBatch=true;
4949
exportconstdecoupleUpdatePriorityFromScheduler=false;
50+
exportconstenableDiscreteEventFlushingChange=false;
5051

5152
// Flow magic to verify the exports of this file match the original version.
5253
// eslint-disable-next-line no-unused-vars

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
exportconstenableNewReconciler=false;
4848
exportconstdeferRenderPhaseUpdateToNextBatch=true;
4949
exportconstdecoupleUpdatePriorityFromScheduler=false;
50+
exportconstenableDiscreteEventFlushingChange=false;
5051

5152
// Flow magic to verify the exports of this file match the original version.
5253
// eslint-disable-next-line no-unused-vars

‎packages/shared/forks/ReactFeatureFlags.test-renderer.native.js

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
exportconstenableNewReconciler=false;
4848
exportconstdeferRenderPhaseUpdateToNextBatch=true;
4949
exportconstdecoupleUpdatePriorityFromScheduler=false;
50+
exportconstenableDiscreteEventFlushingChange=false;
5051

5152
// Flow magic to verify the exports of this file match the original version.
5253
// eslint-disable-next-line no-unused-vars

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
exportconstenableNewReconciler=false;
4848
exportconstdeferRenderPhaseUpdateToNextBatch=true;
4949
exportconstdecoupleUpdatePriorityFromScheduler=false;
50+
exportconstenableDiscreteEventFlushingChange=false;
5051

5152
// Flow magic to verify the exports of this file match the original version.
5253
// eslint-disable-next-line no-unused-vars

‎packages/shared/forks/ReactFeatureFlags.testing.js

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
exportconstenableNewReconciler=false;
4848
exportconstdeferRenderPhaseUpdateToNextBatch=true;
4949
exportconstdecoupleUpdatePriorityFromScheduler=false;
50+
exportconstenableDiscreteEventFlushingChange=false;
5051

5152
// Flow magic to verify the exports of this file match the original version.
5253
// eslint-disable-next-line no-unused-vars

‎packages/shared/forks/ReactFeatureFlags.testing.www.js

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
exportconstenableNewReconciler=false;
4848
exportconstdeferRenderPhaseUpdateToNextBatch=true;
4949
exportconstdecoupleUpdatePriorityFromScheduler=false;
50+
exportconstenableDiscreteEventFlushingChange=true;
5051

5152
// Flow magic to verify the exports of this file match the original version.
5253
// eslint-disable-next-line no-unused-vars

‎packages/shared/forks/ReactFeatureFlags.www.js

+2
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ export const disableTextareaChildren = __EXPERIMENTAL__;
7575

7676
exportconstwarnUnstableRenderSubtreeIntoContainer=false;
7777

78+
exportconstenableDiscreteEventFlushingChange=true;
79+
7880
// Enable forked reconciler. Piggy-backing on the "variant" global so that we
7981
// don't have to add another test dimension. The build system will compile this
8082
// to the correct value.

0 commit comments

Comments
 (0)
close