warn(SyntheticEvent): Warn when accessing or setting properties on released syntheticEvents
Closes #5939
Kent C. Dodds committed
Jan 29, 2016 at 12:30 UTC
631285268881f405ff4fa0d1934125110628b218
2 files changed
+127
-29
src/renderers/dom/client/syntheticEvents/SyntheticEvent.js
+66
-20
@@ -55,6 +55,13 @@ var EventInterface = {
55
* @param {DOMEventTarget} nativeEventTarget Target node.
56
*/
57
function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarget) {
58
+ if (__DEV__) {
59
+ // these have a getter/setter for warnings
60
+ delete this.nativeEvent;
61
+ delete this.preventDefault;
62
+ delete this.stopPropagation;
63
+ }
64
+
65
this.dispatchConfig = dispatchConfig;
66
this._targetInst = targetInst;
67
this.nativeEvent = nativeEvent;
@@ -64,6 +71,9 @@ function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarg
71
if (!Interface.hasOwnProperty(propName)) {
72
continue;
73
}
74
+ if (__DEV__) {
75
+ delete this[propName]; // this has a getter/setter for warnings
76
+ }
77
var normalize = Interface[propName];
78
if (normalize) {
79
this[propName] = normalize(nativeEvent);
@@ -92,15 +102,6 @@ assign(SyntheticEvent.prototype, {
102
preventDefault: function() {
103
this.defaultPrevented = true;
104
var event = this.nativeEvent;
95
- if (__DEV__) {
96
- warning(
97
- event,
98
- 'This synthetic event is reused for performance reasons. If you\'re ' +
99
- 'seeing this, you\'re calling `preventDefault` on a ' +
100
- 'released/nullified synthetic event. This is a no-op. See ' +
101
- 'https://fb.me/react-event-pooling for more information.'
102
- );
103
- }
105
if (!event) {
106
return;
107
}
@@ -115,15 +116,6 @@ assign(SyntheticEvent.prototype, {
116
117
stopPropagation: function() {
118
var event = this.nativeEvent;
118
- if (__DEV__) {
119
- warning(
120
- event,
121
- 'This synthetic event is reused for performance reasons. If you\'re ' +
122
- 'seeing this, you\'re calling `stopPropagation` on a ' +
123
- 'released/nullified synthetic event. This is a no-op. See ' +
124
- 'https://fb.me/react-event-pooling for more information.'
125
- );
126
- }
119
if (!event) {
120
return;
121
}
@@ -158,11 +150,22 @@ assign(SyntheticEvent.prototype, {
150
destructor: function() {
151
var Interface = this.constructor.Interface;
152
for (var propName in Interface) {
161
- this[propName] = null;
153
+ if (__DEV__) {
154
+ Object.defineProperty(this, propName, getPooledWarningPropertyDefinition(propName, Interface[propName]));
155
+ } else {
156
+ this[propName] = null;
157
+ }
158
+ }
159
+ if (__DEV__) {
160
+ var noop = require('emptyFunction');
161
+ Object.defineProperty(this, 'nativeEvent', getPooledWarningPropertyDefinition('nativeEvent', null));
162
+ Object.defineProperty(this, 'preventDefault', getPooledWarningPropertyDefinition('preventDefault', noop));
163
+ Object.defineProperty(this, 'stopPropagation', getPooledWarningPropertyDefinition('stopPropagation', noop));
164
+ } else {
165
+ this.nativeEvent = null;
166
}
167
this.dispatchConfig = null;
168
this._targetInst = null;
165
- this.nativeEvent = null;
169
},
170
171
});
@@ -195,3 +198,46 @@ SyntheticEvent.augmentClass = function(Class, Interface) {
198
PooledClass.addPoolingTo(SyntheticEvent, PooledClass.fourArgumentPooler);
199
200
module.exports = SyntheticEvent;
201
+
202
+/**
203
+ * Helper to nullify syntheticEvent instance properties when destructing
204
+ *
205
+ * @param {object} SyntheticEvent
206
+ * @param {String} propName
207
+ * @return {object} defineProperty object
208
+ */
209
+function getPooledWarningPropertyDefinition(propName, getVal) {
210
+ var isFunction = typeof getVal === 'function';
211
+ return {
212
+ configurable: true,
213
+ set: set,
214
+ get: get,
215
+ };
216
+
217
+ function set(val) {
218
+ var action = isFunction ? 'setting the method' : 'setting the property';
219
+ warn(action, 'This is effectively a no-op');
220
+ return val;
221
+ }
222
+
223
+ function get() {
224
+ var action = isFunction ? 'accessing the method' : 'accessing the property';
225
+ var result = isFunction ? 'This is a no-op function' : 'This is set to null';
226
+ warn(action, result);
227
+ return getVal;
228
+ }
229
+
230
+ function warn(action, result) {
231
+ var warningCondition = false;
232
+ warning(
233
+ warningCondition,
234
+ 'This synthetic event is reused for performance reasons. If you\'re seeing this,' +
235
+ 'you\'re %s `%s` on a released/nullified synthetic event. %s.' +
236
+ 'If you must keep the original synthetic event around, use event.persist().' +
237
+ 'See https://fb.me/react-event-pooling for more information.',
238
+ action,
239
+ propName,
240
+ result
241
+ );
242
+ }
243
+}
src/renderers/dom/client/syntheticEvents/__tests__/SyntheticEvent-test.js
+61
-9
@@ -12,12 +12,18 @@
12
'use strict';
13
14
var SyntheticEvent;
15
+var React;
16
+var ReactDOM;
17
+var ReactTestUtils;
18
19
describe('SyntheticEvent', function() {
20
var createEvent;
21
22
beforeEach(function() {
23
SyntheticEvent = require('SyntheticEvent');
24
+ React = require('React');
25
+ ReactDOM = require('ReactDOM');
26
+ ReactTestUtils = require('ReactTestUtils');
27
28
createEvent = function(nativeEvent) {
29
var target = require('getEventTarget')(nativeEvent);
@@ -72,13 +78,36 @@ describe('SyntheticEvent', function() {
78
expect(syntheticEvent.isPersistent()).toBe(true);
79
});
80
75
- it('should be nullified if the synthetic event has called destructor', function() {
81
+ it('should be nullified if the synthetic event has called destructor and log warnings', function() {
82
+ spyOn(console, 'error');
83
var target = document.createElement('div');
84
var syntheticEvent = createEvent({srcElement: target});
85
syntheticEvent.destructor();
86
expect(syntheticEvent.type).toBe(null);
87
expect(syntheticEvent.nativeEvent).toBe(null);
88
expect(syntheticEvent.target).toBe(null);
89
+ expect(console.error.calls.length).toBe(3); // once for each property accessed
90
+ expect(console.error.argsForCall[0][0]).toBe( // assert the first warning for accessing `type`
91
+ 'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
92
+ 'you\'re accessing the property `type` on a released/nullified synthetic event. This is set to null.' +
93
+ 'If you must keep the original synthetic event around, use event.persist().' +
94
+ 'See https://fb.me/react-event-pooling for more information.'
95
+ );
96
+ });
97
+
98
+ it('should warn when setting properties of a destructored synthetic event', function() {
99
+ spyOn(console, 'error');
100
+ var target = document.createElement('div');
101
+ var syntheticEvent = createEvent({srcElement: target});
102
+ syntheticEvent.destructor();
103
+ expect(syntheticEvent.type = 'MouseEvent').toBe('MouseEvent');
104
+ expect(console.error.calls.length).toBe(1);
105
+ expect(console.error.argsForCall[0][0]).toBe(
106
+ 'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
107
+ 'you\'re setting the property `type` on a released/nullified synthetic event. This is effectively a no-op.' +
108
+ 'If you must keep the original synthetic event around, use event.persist().' +
109
+ 'See https://fb.me/react-event-pooling for more information.'
110
+ );
111
});
112
113
it('should warn if the synthetic event has been released when calling `preventDefault`', function() {
@@ -88,10 +117,10 @@ describe('SyntheticEvent', function() {
117
syntheticEvent.preventDefault();
118
expect(console.error.calls.length).toBe(1);
119
expect(console.error.argsForCall[0][0]).toBe(
91
- 'Warning: This synthetic event is reused for performance reasons. If ' +
92
- 'you\'re seeing this, you\'re calling `preventDefault` on a ' +
93
- 'released/nullified synthetic event. This is a no-op. See ' +
94
- 'https://fb.me/react-event-pooling for more information.'
120
+ 'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
121
+ 'you\'re accessing the method `preventDefault` on a released/nullified synthetic event. This is a no-op function.' +
122
+ 'If you must keep the original synthetic event around, use event.persist().' +
123
+ 'See https://fb.me/react-event-pooling for more information.'
124
);
125
});
126
@@ -102,10 +131,33 @@ describe('SyntheticEvent', function() {
131
syntheticEvent.stopPropagation();
132
expect(console.error.calls.length).toBe(1);
133
expect(console.error.argsForCall[0][0]).toBe(
105
- 'Warning: This synthetic event is reused for performance reasons. If ' +
106
- 'you\'re seeing this, you\'re calling `stopPropagation` on a ' +
107
- 'released/nullified synthetic event. This is a no-op. See ' +
108
- 'https://fb.me/react-event-pooling for more information.'
134
+ 'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
135
+ 'you\'re accessing the method `stopPropagation` on a released/nullified synthetic event. This is a no-op function.' +
136
+ 'If you must keep the original synthetic event around, use event.persist().' +
137
+ 'See https://fb.me/react-event-pooling for more information.'
138
+ );
139
+ });
140
+
141
+ it('should properly log warnings when events simulated with rendered components', function() {
142
+ spyOn(console, 'error');
143
+ var event;
144
+ var element = document.createElement('div');
145
+ function assignEvent(e) {
146
+ event = e;
147
+ }
148
+ var instance = ReactDOM.render(<div onClick={assignEvent} />, element);
149
+ ReactTestUtils.Simulate.click(ReactDOM.findDOMNode(instance));
150
+ expect(console.error.calls.length).toBe(0);
151
+
152
+ // access a property to cause the warning
153
+ event.nativeEvent; // eslint-disable-line no-unused-expressions
154
+
155
+ expect(console.error.calls.length).toBe(1);
156
+ expect(console.error.argsForCall[0][0]).toBe(
157
+ 'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
158
+ 'you\'re accessing the property `nativeEvent` on a released/nullified synthetic event. This is set to null.' +
159
+ 'If you must keep the original synthetic event around, use event.persist().' +
160
+ 'See https://fb.me/react-event-pooling for more information.'
161
);
162
});
163
});