Interactive updates shouldn't flush until the end of the outermost batch
Accounts for the case where an event is dispatched synchronously from inside another event, like `el.focus`. I've added a test, but in general we need more coverage around this area.
Andrew Clark committed
Jan 30, 2018 at 23:13 UTC
27fe752eea6f3c358ab696d5725dee7de719c94b
3 files changed
+70
-11
packages/react-dom/src/events/ReactDOMEventListener.js
+1
-10
@@ -5,11 +5,7 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
-import {
9
- batchedUpdates,
10
- flushInteractiveUpdates,
11
- interactiveUpdates,
12
-} from 'events/ReactGenericBatching';
8
+import {batchedUpdates, interactiveUpdates} from 'events/ReactGenericBatching';
9
import {runExtractedEventsInBatch} from 'events/EventPluginHub';
10
import {isFiberMounted} from 'react-reconciler/reflection';
11
import {HostRoot} from 'shared/ReactTypeOfWork';
@@ -166,11 +162,6 @@ export function trapCapturedEvent(topLevelType, handlerBaseName, element) {
162
}
163
164
function dispatchInteractiveEvent(topLevelType, nativeEvent) {
169
- // If there are any pending interactive updates, synchronously flush them.
170
- // This needs to happen before we read any handlers, because the effect of the
171
- // previous event may affect which handlers are called during this event.
172
- flushInteractiveUpdates();
173
- // Increase the priority of updates inside this event.
165
interactiveUpdates(dispatchEvent, topLevelType, nativeEvent);
166
}
167
packages/react-dom/src/events/__tests__/SimpleEventPlugin-test.internal.js
+50
@@ -167,6 +167,56 @@ describe('SimpleEventPlugin', function() {
167
});
168
});
169
170
+ it('batches updates that occur as a result of a nested event dispatch', () => {
171
+ const container = document.createElement('div');
172
+ document.body.appendChild(container);
173
+
174
+ let ops = [];
175
+ let button;
176
+ class Button extends React.Component {
177
+ state = {count: 0};
178
+ increment = () =>
179
+ this.setState(state => ({
180
+ count: state.count + 1,
181
+ }));
182
+ componentDidUpdate() {
183
+ ops.push(`didUpdate - Count: ${this.state.count}`);
184
+ }
185
+ render() {
186
+ return (
187
+ <button
188
+ ref={el => (button = el)}
189
+ onFocus={this.increment}
190
+ onClick={() => {
191
+ // The focus call synchronously dispatches a nested event. All of
192
+ // the updates in this handler should be batched together.
193
+ this.increment();
194
+ button.focus();
195
+ this.increment();
196
+ }}>
197
+ Count: {this.state.count}
198
+ </button>
199
+ );
200
+ }
201
+ }
202
+
203
+ function click() {
204
+ button.dispatchEvent(
205
+ new MouseEvent('click', {bubbles: true, cancelable: true}),
206
+ );
207
+ }
208
+
209
+ ReactDOM.render(<Button />, container);
210
+ expect(button.textContent).toEqual('Count: 0');
211
+ expect(ops).toEqual([]);
212
+
213
+ click();
214
+
215
+ // There should be exactly one update.
216
+ expect(ops).toEqual(['didUpdate - Count: 3']);
217
+ expect(button.textContent).toEqual('Count: 3');
218
+ });
219
+
220
describe('interactive events, in async mode', () => {
221
beforeEach(() => {
222
jest.resetModules();
packages/react-reconciler/src/ReactFiberScheduler.js
+19
-1
@@ -1807,13 +1807,31 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
1807
if (isBatchingInteractiveUpdates) {
1808
return fn(a, b);
1809
}
1810
-
1810
+ // If there are any pending interactive updates, synchronously flush them.
1811
+ // This needs to happen before we read any handlers, because the effect of
1812
+ // the previous event may influence which handlers are called during
1813
+ // this event.
1814
+ if (
1815
+ !isBatchingUpdates &&
1816
+ !isRendering &&
1817
+ lowestPendingInteractiveExpirationTime !== NoWork
1818
+ ) {
1819
+ // Synchronously flush pending interactive updates.
1820
+ performWork(lowestPendingInteractiveExpirationTime, false, null);
1821
+ lowestPendingInteractiveExpirationTime = NoWork;
1822
+ }
1823
const previousIsBatchingInteractiveUpdates = isBatchingInteractiveUpdates;
1824
+ const previousIsBatchingUpdates = isBatchingUpdates;
1825
isBatchingInteractiveUpdates = true;
1826
+ isBatchingUpdates = true;
1827
try {
1828
return fn(a, b);
1829
} finally {
1830
isBatchingInteractiveUpdates = previousIsBatchingInteractiveUpdates;
1831
+ isBatchingUpdates = previousIsBatchingUpdates;
1832
+ if (!isBatchingUpdates && !isRendering) {
1833
+ performSyncWork();
1834
+ }
1835
}
1836
}
1837