Skip to content

Commit

Permalink
Removed optimization for events without target in ReactNativeEventEmi…
Browse files Browse the repository at this point in the history
…tter (#9219)

* Removed optimization for events without target in ReactNativeEventEmitter

This PR fixes the problem originally introduced in #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: facebook/react-native#12976 .

The test also illustrates this problem.

* Prettier
  • Loading branch information
shergin authored and sophiebits committed Mar 21, 2017
1 parent 7f665da commit 97ab3f5
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 5 deletions.
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,7 @@ src/renderers/native/__tests__/ReactNativeEvents-test.js
* handles events
* handles events on text nodes
* handles when a responder is unmounted while a touch sequence is in progress
* handles events without target

src/renderers/native/__tests__/ReactNativeMount-test.js
* should be able to create and render a native component
Expand Down
5 changes: 0 additions & 5 deletions src/renderers/native/ReactNativeEventEmitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ var ReactNativeEventEmitter = {
) {
var nativeEvent = nativeEventParam || EMPTY_NATIVE_EVENT;
var inst = ReactNativeComponentTree.getInstanceFromNode(rootNodeID);
if (!inst) {
// If the original instance is already gone, we don't have to dispatch
// any events.
return;
}
ReactGenericBatching.batchedUpdates(function() {
ReactNativeEventEmitter.handleTopLevel(
topLevelType,
Expand Down
94 changes: 94 additions & 0 deletions src/renderers/native/__tests__/ReactNativeEvents-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,97 @@ it('handles when a responder is unmounted while a touch sequence is in progress'
expect(getResponderId()).toBe('two');
expect(log).toEqual(['two responder start']);
});

it('handles events without target', () => {
var EventEmitter = RCTEventEmitter.register.mock.calls[0][0];
var View = createReactNativeComponentClass({
validAttributes: {id: true},
uiViewClassName: 'View',
});

function getViewById(id) {
return UIManager.createView.mock.calls.find(
args => args[3] && args[3].id === id,
)[0];
}

function getResponderId() {
const responder = ResponderEventPlugin._getResponder();
if (responder === null) {
return null;
}
const props = typeof responder.tag === 'number'
? responder.memoizedProps
: responder._currentElement.props;
return props ? props.id : null;
}

var log = [];

function render(renderFirstComponent) {
ReactNative.render(
<View id="parent">
<View key={1}>
{renderFirstComponent
? <View
id="one"
onResponderEnd={() => log.push('one responder end')}
onResponderStart={() => log.push('one responder start')}
onStartShouldSetResponder={() => true}
/>
: null}
</View>
<View key={2}>
<View
id="two"
onResponderEnd={() => log.push('two responder end')}
onResponderStart={() => log.push('two responder start')}
onStartShouldSetResponder={() => true}
/>
</View>
</View>,
1,
);
}

render(true);

EventEmitter.receiveTouches(
'topTouchStart',
[{target: getViewById('one'), identifier: 17}],
[0],
);

// Unmounting component 'one'.
render(false);

EventEmitter.receiveTouches(
'topTouchEnd',
[{target: getViewById('one'), identifier: 17}],
[0],
);

expect(getResponderId()).toBe(null);

EventEmitter.receiveTouches(
'topTouchStart',
[{target: getViewById('two'), identifier: 18}],
[0],
);

expect(getResponderId()).toBe('two');

EventEmitter.receiveTouches(
'topTouchEnd',
[{target: getViewById('two'), identifier: 18}],
[0],
);

expect(getResponderId()).toBe(null);

expect(log).toEqual([
'one responder start',
'two responder start',
'two responder end',
]);
});

0 comments on commit 97ab3f5

Please sign in to comment.