@samitouri / QOS-React / commits / 97ab3f5ef8

Removed optimization for events without target in ReactNativeEventEmitter (#9219)

* Removed optimization for events without target in ReactNativeEventEmitter This PR fixes the problem originally introduced in https://github.com/facebook/react/pull/6590 The problem is that `ResponderEventPlugin` (and `ResponderTouchHistoryStore`) relies on that fact that touch events are balanced. So if one `startish` event happened, should be coming `endish` event. Otherwise there is no way to maintain internal `trackedTouchCount` counter. So, if we drop some events, we break this logic. Moreover, that optimization clearly contradict with this statement from `ResponderEventPlugin`: ``` We must be resilient to `targetInst` being `null` on `touchMove` or `touchEnd`. On certain platforms, this means that a native scroll has assumed control and the original touch targets are destroyed. ``` This issue causes several major problems in React Native, and one of them (finally!) is easy to reproduce: https://github.com/facebook/react-native/issues/12976 . The test also illustrates this problem. * Prettier

Valentin Shergin committed Mar 20, 2017 at 17:07 UTC 97ab3f5ef87e53b212979868de97ae3b7d3fad82
3 files changed +95 -5
scripts/fiber/tests-passing.txt
+1
@@ -1439,6 +1439,7 @@ src/renderers/native/__tests__/ReactNativeEvents-test.js
1439 * handles events
1440 * handles events on text nodes
1441 * handles when a responder is unmounted while a touch sequence is in progress
1442 +* handles events without target
1443
1444 src/renderers/native/__tests__/ReactNativeMount-test.js
1445 * should be able to create and render a native component
src/renderers/native/ReactNativeEventEmitter.js
-5
@@ -102,11 +102,6 @@ var ReactNativeEventEmitter = {
102 ) {
103 var nativeEvent = nativeEventParam || EMPTY_NATIVE_EVENT;
104 var inst = ReactNativeComponentTree.getInstanceFromNode(rootNodeID);
105 - if (!inst) {
106 - // If the original instance is already gone, we don't have to dispatch
107 - // any events.
108 - return;
109 - }
105 ReactGenericBatching.batchedUpdates(function() {
106 ReactNativeEventEmitter.handleTopLevel(
107 topLevelType,
src/renderers/native/__tests__/ReactNativeEvents-test.js
+94
@@ -267,3 +267,97 @@ it('handles when a responder is unmounted while a touch sequence is in progress'
267 expect(getResponderId()).toBe('two');
268 expect(log).toEqual(['two responder start']);
269 });
270 +
271 +it('handles events without target', () => {
272 + var EventEmitter = RCTEventEmitter.register.mock.calls[0][0];
273 + var View = createReactNativeComponentClass({
274 + validAttributes: {id: true},
275 + uiViewClassName: 'View',
276 + });
277 +
278 + function getViewById(id) {
279 + return UIManager.createView.mock.calls.find(
280 + args => args[3] && args[3].id === id,
281 + )[0];
282 + }
283 +
284 + function getResponderId() {
285 + const responder = ResponderEventPlugin._getResponder();
286 + if (responder === null) {
287 + return null;
288 + }
289 + const props = typeof responder.tag === 'number'
290 + ? responder.memoizedProps
291 + : responder._currentElement.props;
292 + return props ? props.id : null;
293 + }
294 +
295 + var log = [];
296 +
297 + function render(renderFirstComponent) {
298 + ReactNative.render(
299 + <View id="parent">
300 + <View key={1}>
301 + {renderFirstComponent
302 + ? <View
303 + id="one"
304 + onResponderEnd={() => log.push('one responder end')}
305 + onResponderStart={() => log.push('one responder start')}
306 + onStartShouldSetResponder={() => true}
307 + />
308 + : null}
309 + </View>
310 + <View key={2}>
311 + <View
312 + id="two"
313 + onResponderEnd={() => log.push('two responder end')}
314 + onResponderStart={() => log.push('two responder start')}
315 + onStartShouldSetResponder={() => true}
316 + />
317 + </View>
318 + </View>,
319 + 1,
320 + );
321 + }
322 +
323 + render(true);
324 +
325 + EventEmitter.receiveTouches(
326 + 'topTouchStart',
327 + [{target: getViewById('one'), identifier: 17}],
328 + [0],
329 + );
330 +
331 + // Unmounting component 'one'.
332 + render(false);
333 +
334 + EventEmitter.receiveTouches(
335 + 'topTouchEnd',
336 + [{target: getViewById('one'), identifier: 17}],
337 + [0],
338 + );
339 +
340 + expect(getResponderId()).toBe(null);
341 +
342 + EventEmitter.receiveTouches(
343 + 'topTouchStart',
344 + [{target: getViewById('two'), identifier: 18}],
345 + [0],
346 + );
347 +
348 + expect(getResponderId()).toBe('two');
349 +
350 + EventEmitter.receiveTouches(
351 + 'topTouchEnd',
352 + [{target: getViewById('two'), identifier: 18}],
353 + [0],
354 + );
355 +
356 + expect(getResponderId()).toBe(null);
357 +
358 + expect(log).toEqual([
359 + 'one responder start',
360 + 'two responder start',
361 + 'two responder end',
362 + ]);
363 +});