Fix bugs that occur when event responder unmounts during a touch event sequence > This PR adds a test (initially failing) for this case along with a fix for stack and fiber. The stack fix was copied from a Diff submitted by @sema. The fiber fix just required us to stop leaking properties for unmounted views. > Longer term we may want to explicitly invoke a release event listener for a responder just before unmounting it. This PR does not do that.
Fix bugs that occur when event responder unmounts during a touch event sequence > This PR adds a test (initially failing) for this case along with a fix for stack and fiber. The stack fix was copied from a Diff submitted by @sema. The fiber fix just required us to stop leaking properties for unmounted views. > Longer term we may want to explicitly invoke a release event listener for a responder just before unmounting it. This PR does not do that.
Brian Vaughn committed
Feb 22, 2017 at 10:21 UTC
a44a5d68d818782dd097b6d477221494180e60e0
5 files changed
+104
scripts/fiber/tests-passing.txt
+1
@@ -1454,6 +1454,7 @@ src/renderers/native/__tests__/ReactNativeAttributePayload-test.js
1454
1455
src/renderers/native/__tests__/ReactNativeEvents-test.js
1456
* handles events
1457
+* handles when a responder is unmounted while a touch sequence is in progress
1458
1459
src/renderers/native/__tests__/ReactNativeMount-test.js
1460
* should be able to create and render a native component
src/__mocks__/UIManager.js
+2
@@ -12,7 +12,9 @@
12
// Mock of the Native Hooks
13
14
var RCTUIManager = {
15
+ clearJSResponder: jest.fn(),
16
createView: jest.fn(),
17
+ setJSResponder: jest.fn(),
18
setChildren: jest.fn(),
19
manageChildren: jest.fn(),
20
updateView: jest.fn(),
src/renderers/native/ReactNativeComponentTree.js
+1
@@ -53,6 +53,7 @@ function uncacheNode(inst) {
53
54
function uncacheFiberNode(tag) {
55
delete instanceCache[tag];
56
+ delete instanceProps[tag];
57
}
58
59
function getInstanceFromTag(tag) {
src/renderers/native/__tests__/ReactNativeEvents-test.js
+96
@@ -15,6 +15,7 @@ var RCTEventEmitter;
15
var React;
16
var ReactErrorUtils;
17
var ReactNative;
18
+var ResponderEventPlugin;
19
var UIManager;
20
var createReactNativeComponentClass;
21
@@ -25,6 +26,7 @@ beforeEach(() => {
26
React = require('React');
27
ReactErrorUtils = require('ReactErrorUtils');
28
ReactNative = require('ReactNative');
29
+ ResponderEventPlugin = require('ResponderEventPlugin');
30
UIManager = require('UIManager');
31
createReactNativeComponentClass = require('createReactNativeComponentClass');
32
@@ -91,3 +93,97 @@ it('handles events', () => {
93
'outer touchend',
94
]);
95
});
96
+
97
+it('handles when a responder is unmounted while a touch sequence is in progress', () => {
98
+ var EventEmitter = RCTEventEmitter.register.mock.calls[0][0];
99
+ var View = createReactNativeComponentClass({
100
+ validAttributes: { id: true },
101
+ uiViewClassName: 'View',
102
+ });
103
+
104
+ function getViewById(id) {
105
+ return UIManager.createView.mock.calls.find(
106
+ args => args[3] && args[3].id === id
107
+ )[0];
108
+ }
109
+
110
+ function getResponderId() {
111
+ const responder = ResponderEventPlugin._getResponder();
112
+ if (responder === null) {
113
+ return null;
114
+ }
115
+ const props = typeof responder.tag === 'number'
116
+ ? responder.memoizedProps
117
+ : responder._currentElement.props;
118
+ return props ? props.id : null;
119
+ }
120
+
121
+ var log = [];
122
+ ReactNative.render(
123
+ <View id="parent">
124
+ <View key={1}>
125
+ <View
126
+ id="one"
127
+ onResponderEnd={() => log.push('one responder end')}
128
+ onResponderStart={() => log.push('one responder start')}
129
+ onStartShouldSetResponder={() => true}
130
+ />
131
+ </View>
132
+ <View key={2}>
133
+ <View
134
+ id="two"
135
+ onResponderEnd={() => log.push('two responder end')}
136
+ onResponderStart={() => log.push('two responder start')}
137
+ onStartShouldSetResponder={() => true}
138
+ />
139
+ </View>
140
+ </View>,
141
+ 1
142
+ );
143
+
144
+ EventEmitter.receiveTouches(
145
+ 'topTouchStart',
146
+ [{target: getViewById('one'), identifier: 17}],
147
+ [0]
148
+ );
149
+
150
+ expect(getResponderId()).toBe('one');
151
+ expect(log).toEqual(['one responder start']);
152
+ log.splice(0);
153
+
154
+ ReactNative.render(
155
+ <View id="parent">
156
+ <View key={2}>
157
+ <View
158
+ id="two"
159
+ onResponderEnd={() => log.push('two responder end')}
160
+ onResponderStart={() => log.push('two responder start')}
161
+ onStartShouldSetResponder={() => true}
162
+ />
163
+ </View>
164
+ </View>,
165
+ 1
166
+ );
167
+
168
+ // TODO Verify the onResponderEnd listener has been called (before the unmount)
169
+ // expect(log).toEqual(['one responder end']);
170
+ // log.splice(0);
171
+
172
+ EventEmitter.receiveTouches(
173
+ 'topTouchEnd',
174
+ [{target: getViewById('two'), identifier: 17}],
175
+ [0]
176
+ );
177
+
178
+ expect(getResponderId()).toBeNull();
179
+ expect(log).toEqual([]);
180
+
181
+ EventEmitter.receiveTouches(
182
+ 'topTouchStart',
183
+ [{target: getViewById('two'), identifier: 17}],
184
+ [0]
185
+ );
186
+
187
+ expect(getResponderId()).toBe('two');
188
+ expect(log).toEqual(['two responder start']);
189
+});
src/renderers/shared/shared/event/EventPluginHub.js
+4
@@ -142,6 +142,10 @@ var EventPluginHub = {
142
// Text node, let it bubble through.
143
return null;
144
}
145
+ if (!inst._rootNodeID) {
146
+ // If the instance is already unmounted, we have no listeners.
147
+ return null;
148
+ }
149
const props = inst._currentElement.props;
150
listener = props[registrationName];
151
if (shouldPreventMouseEvent(registrationName, inst._currentElement.type, props)) {