Re-add discrete flushing timeStamp heuristic (behind flag) (#19540)
Dominic Gannaway committed
Aug 6, 2020 at 13:21 UTC
f77c7b9d76205d93908a06fb2d58ee8c31188d16
13 files changed
+78
-17
packages/react-dom/src/__tests__/ReactDOMFiber-test.js
+15
-4
@@ -1031,6 +1031,17 @@ describe('ReactDOMFiber', () => {
1031
const handlerA = () => ops.push('A');
1032
const handlerB = () => ops.push('B');
1033
1034
+ function click() {
1035
+ const event = new MouseEvent('click', {
1036
+ bubbles: true,
1037
+ cancelable: true,
1038
+ });
1039
+ Object.defineProperty(event, 'timeStamp', {
1040
+ value: 0,
1041
+ });
1042
+ node.dispatchEvent(event);
1043
+ }
1044
+
1045
class Example extends React.Component {
1046
state = {flip: false, count: 0};
1047
flip() {
@@ -1064,7 +1075,7 @@ describe('ReactDOMFiber', () => {
1075
const node = container.firstChild;
1076
expect(node.tagName).toEqual('DIV');
1077
1067
- node.click();
1078
+ click();
1079
1080
expect(ops).toEqual(['A']);
1081
ops = [];
@@ -1072,7 +1083,7 @@ describe('ReactDOMFiber', () => {
1083
// Render with the other event handler.
1084
inst.flip();
1085
1075
- node.click();
1086
+ click();
1087
1088
expect(ops).toEqual(['B']);
1089
ops = [];
@@ -1080,7 +1091,7 @@ describe('ReactDOMFiber', () => {
1091
// Rerender without changing any props.
1092
inst.tick();
1093
1083
- node.click();
1094
+ click();
1095
1096
expect(ops).toEqual(['B']);
1097
ops = [];
@@ -1100,7 +1111,7 @@ describe('ReactDOMFiber', () => {
1111
ops = [];
1112
1113
// Any click that happens after commit, should invoke A.
1103
- node.click();
1114
+ click();
1115
expect(ops).toEqual(['A']);
1116
});
1117
packages/react-dom/src/events/ReactDOMEventListener.js
+1
-1
@@ -130,7 +130,7 @@ function dispatchDiscreteEvent(
130
// flushed for this event and we don't need to do it again.
131
(eventSystemFlags & IS_LEGACY_FB_SUPPORT_MODE) === 0
132
) {
133
- flushDiscreteUpdatesIfNeeded();
133
+ flushDiscreteUpdatesIfNeeded(nativeEvent.timeStamp);
134
}
135
discreteUpdates(
136
dispatchEvent,
packages/react-dom/src/events/ReactDOMUpdateBatching.js
+27
-3
@@ -9,6 +9,7 @@ import {
9
needsStateRestore,
10
restoreStateIfNeeded,
11
} from './ReactDOMControlledComponent';
12
+import {enableDiscreteEventFlushingChange} from 'shared/ReactFeatureFlags';
13
14
// Used as a way to call batchedUpdates when we don't have a reference to
15
// 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) {
88
}
89
}
90
90
-export function flushDiscreteUpdatesIfNeeded() {
91
- if (!isInsideEventHandler) {
92
- flushDiscreteUpdatesImpl();
91
+let lastFlushedEventTimeStamp = 0;
92
+export function flushDiscreteUpdatesIfNeeded(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
+ }
117
}
118
}
119
packages/react-dom/src/events/plugins/__tests__/ModernSimpleEventPlugin-test.js
+24
-9
@@ -275,9 +275,14 @@ describe('SimpleEventPlugin', function() {
275
expect(Scheduler).toFlushAndYield(['render button: enabled']);
276
277
function click() {
278
- button.dispatchEvent(
279
- new MouseEvent('click', {bubbles: true, cancelable: true}),
280
- );
278
+ const event = new MouseEvent('click', {
279
+ bubbles: true,
280
+ cancelable: true,
281
+ });
282
+ Object.defineProperty(event, 'timeStamp', {
283
+ value: 0,
284
+ });
285
+ button.dispatchEvent(event);
286
}
287
288
// Click the button to trigger the side-effect
@@ -340,9 +345,14 @@ describe('SimpleEventPlugin', function() {
345
expect(button.textContent).toEqual('Count: 0');
346
347
function click() {
343
- button.dispatchEvent(
344
- new MouseEvent('click', {bubbles: true, cancelable: true}),
345
- );
348
+ const event = new MouseEvent('click', {
349
+ bubbles: true,
350
+ cancelable: true,
351
+ });
352
+ Object.defineProperty(event, 'timeStamp', {
353
+ value: 0,
354
+ });
355
+ button.dispatchEvent(event);
356
}
357
358
// Click the button a single time
@@ -421,9 +431,14 @@ describe('SimpleEventPlugin', function() {
431
expect(button.textContent).toEqual('High-pri count: 0, Low-pri count: 0');
432
433
function click() {
424
- button.dispatchEvent(
425
- new MouseEvent('click', {bubbles: true, cancelable: true}),
426
- );
434
+ const event = new MouseEvent('click', {
435
+ bubbles: true,
436
+ cancelable: true,
437
+ });
438
+ Object.defineProperty(event, 'timeStamp', {
439
+ value: 0,
440
+ });
441
+ button.dispatchEvent(event);
442
}
443
444
// Click the button a single time
packages/shared/ReactFeatureFlags.js
+2
@@ -125,3 +125,5 @@ export const deferRenderPhaseUpdateToNextBatch = true;
125
126
// Replacement for runWithPriority in React internals.
127
export const decoupleUpdatePriorityFromScheduler = false;
128
+
129
+export const enableDiscreteEventFlushingChange = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -48,6 +48,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
48
export const enableNewReconciler = false;
49
export const deferRenderPhaseUpdateToNextBatch = true;
50
export const decoupleUpdatePriorityFromScheduler = false;
51
+export const enableDiscreteEventFlushingChange = false;
52
53
// Flow magic to verify the exports of this file match the original version.
54
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
export const enableNewReconciler = false;
48
export const deferRenderPhaseUpdateToNextBatch = true;
49
export const decoupleUpdatePriorityFromScheduler = false;
50
+export const enableDiscreteEventFlushingChange = false;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
export const enableNewReconciler = false;
48
export const deferRenderPhaseUpdateToNextBatch = true;
49
export const decoupleUpdatePriorityFromScheduler = false;
50
+export const enableDiscreteEventFlushingChange = false;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
export const enableNewReconciler = false;
48
export const deferRenderPhaseUpdateToNextBatch = true;
49
export const decoupleUpdatePriorityFromScheduler = false;
50
+export const enableDiscreteEventFlushingChange = false;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
export const enableNewReconciler = false;
48
export const deferRenderPhaseUpdateToNextBatch = true;
49
export const decoupleUpdatePriorityFromScheduler = false;
50
+export const enableDiscreteEventFlushingChange = false;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
export const enableNewReconciler = false;
48
export const deferRenderPhaseUpdateToNextBatch = true;
49
export const decoupleUpdatePriorityFromScheduler = false;
50
+export const enableDiscreteEventFlushingChange = false;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
export const enableNewReconciler = false;
48
export const deferRenderPhaseUpdateToNextBatch = true;
49
export const decoupleUpdatePriorityFromScheduler = false;
50
+export const enableDiscreteEventFlushingChange = true;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.www.js
+2
@@ -75,6 +75,8 @@ export const disableTextareaChildren = __EXPERIMENTAL__;
75
76
export const warnUnstableRenderSubtreeIntoContainer = false;
77
78
+export const enableDiscreteEventFlushingChange = true;
79
+
80
// Enable forked reconciler. Piggy-backing on the "variant" global so that we
81
// don't have to add another test dimension. The build system will compile this
82
// to the correct value.