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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,17 @@ describe('ReactDOMFiber', () => {
10311031
const handlerA = () => ops.push('A');
10321032
const handlerB = () => ops.push('B');
10331033

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+
10341045
class Example extends React.Component {
10351046
state = {flip: false, count: 0};
10361047
flip() {
@@ -1064,23 +1075,23 @@ describe('ReactDOMFiber', () => {
10641075
const node = 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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 27 additions & 3 deletions
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-
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+
}
93117
}
94118
}
95119

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

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

277277
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);
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
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);
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
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);
427442
}
428443

429444
// Click the button a single time

‎packages/shared/ReactFeatureFlags.js

Lines changed: 2 additions & 0 deletions
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
export const decoupleUpdatePriorityFromScheduler = false;
128+
129+
export const enableDiscreteEventFlushingChange = false;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4848
export const enableNewReconciler = false;
4949
export const deferRenderPhaseUpdateToNextBatch = true;
5050
export const decoupleUpdatePriorityFromScheduler = false;
51+
export const enableDiscreteEventFlushingChange = 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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
export const enableNewReconciler = false;
4848
export const deferRenderPhaseUpdateToNextBatch = true;
4949
export const decoupleUpdatePriorityFromScheduler = false;
50+
export const enableDiscreteEventFlushingChange = 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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
export const enableNewReconciler = false;
4848
export const deferRenderPhaseUpdateToNextBatch = true;
4949
export const decoupleUpdatePriorityFromScheduler = false;
50+
export const enableDiscreteEventFlushingChange = 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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
export const enableNewReconciler = false;
4848
export const deferRenderPhaseUpdateToNextBatch = true;
4949
export const decoupleUpdatePriorityFromScheduler = false;
50+
export const enableDiscreteEventFlushingChange = 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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
4747
export const enableNewReconciler = false;
4848
export const deferRenderPhaseUpdateToNextBatch = true;
4949
export const decoupleUpdatePriorityFromScheduler = false;
50+
export const enableDiscreteEventFlushingChange = false;
5051

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

0 commit comments

Comments
 (0)