Skip to content

Commit 113facd

Browse files
author
Kent C. Dodds
committed
warn(SyntheticEvent): Warn when accessing or setting properties on released syntheticEvents
Closes #5939
1 parent 9c3f595 commit 113facd

File tree

2 files changed

+105
-28
lines changed

2 files changed

+105
-28
lines changed

src/renderers/dom/client/syntheticEvents/SyntheticEvent.js

+66-20
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ var EventInterface = {
5555
* @param {DOMEventTarget} nativeEventTarget Target node.
5656
*/
5757
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+
5865
this.dispatchConfig = dispatchConfig;
5966
this._targetInst = targetInst;
6067
this.nativeEvent = nativeEvent;
@@ -64,6 +71,9 @@ function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarg
6471
if (!Interface.hasOwnProperty(propName)) {
6572
continue;
6673
}
74+
if (__DEV__) {
75+
delete this[propName]; // this has a getter/setter for warnings
76+
}
6777
var normalize = Interface[propName];
6878
if (normalize) {
6979
this[propName] = normalize(nativeEvent);
@@ -92,15 +102,6 @@ assign(SyntheticEvent.prototype, {
92102
preventDefault: function() {
93103
this.defaultPrevented = true;
94104
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-
}
104105
if (!event) {
105106
return;
106107
}
@@ -115,15 +116,6 @@ assign(SyntheticEvent.prototype, {
115116

116117
stopPropagation: function() {
117118
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-
}
127119
if (!event) {
128120
return;
129121
}
@@ -158,11 +150,22 @@ assign(SyntheticEvent.prototype, {
158150
destructor: function() {
159151
var Interface = this.constructor.Interface;
160152
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;
162166
}
163167
this.dispatchConfig = null;
164168
this._targetInst = null;
165-
this.nativeEvent = null;
166169
},
167170

168171
});
@@ -195,3 +198,46 @@ SyntheticEvent.augmentClass = function(Class, Interface) {
195198
PooledClass.addPoolingTo(SyntheticEvent, PooledClass.fourArgumentPooler);
196199

197200
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

+39-8
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ describe('SyntheticEvent', function() {
7373
});
7474

7575
it('should be nullified if the synthetic event has called destructor', function() {
76+
spyOn(console, 'error'); // accessing properties on destructored events logs warnings (tested elsewhere)
7677
var target = document.createElement('div');
7778
var syntheticEvent = createEvent({srcElement: target});
7879
syntheticEvent.destructor();
@@ -81,17 +82,47 @@ describe('SyntheticEvent', function() {
8182
expect(syntheticEvent.target).toBe(null);
8283
});
8384

85+
it('should warn when accessing properties of a destructored synthetic event', function() {
86+
spyOn(console, 'error');
87+
var target = document.createElement('div');
88+
var syntheticEvent = createEvent({srcElement: target});
89+
syntheticEvent.destructor();
90+
expect(syntheticEvent.type).toBe(null);
91+
expect(console.error.calls.length).toBe(1);
92+
expect(console.error.argsForCall[0][0]).toBe(
93+
'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
94+
'you\'re accessing the property `type` on a released/nullified synthetic event. This is set to null.' +
95+
'If you must keep the original synthetic event around use event.persist().' +
96+
'See https://fb.me/react-event-pooling for more information.'
97+
);
98+
});
99+
100+
it('should warn when setting properties of a destructored synthetic event', function() {
101+
spyOn(console, 'error');
102+
var target = document.createElement('div');
103+
var syntheticEvent = createEvent({srcElement: target});
104+
syntheticEvent.destructor();
105+
expect(syntheticEvent.type = 'MouseEvent').toBe('MouseEvent');
106+
expect(console.error.calls.length).toBe(1);
107+
expect(console.error.argsForCall[0][0]).toBe(
108+
'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
109+
'you\'re setting the property `type` on a released/nullified synthetic event. This is effectively a no-op.' +
110+
'If you must keep the original synthetic event around use event.persist().' +
111+
'See https://fb.me/react-event-pooling for more information.'
112+
);
113+
});
114+
84115
it('should warn if the synthetic event has been released when calling `preventDefault`', function() {
85116
spyOn(console, 'error');
86117
var syntheticEvent = createEvent({});
87118
SyntheticEvent.release(syntheticEvent);
88119
syntheticEvent.preventDefault();
89120
expect(console.error.calls.length).toBe(1);
90121
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.'
122+
'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
123+
'you\'re accessing the method `preventDefault` on a released/nullified synthetic event. This is a no-op function.' +
124+
'If you must keep the original synthetic event around use event.persist().' +
125+
'See https://fb.me/react-event-pooling for more information.'
95126
);
96127
});
97128

@@ -102,10 +133,10 @@ describe('SyntheticEvent', function() {
102133
syntheticEvent.stopPropagation();
103134
expect(console.error.calls.length).toBe(1);
104135
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.'
136+
'Warning: This synthetic event is reused for performance reasons. If you\'re seeing this,' +
137+
'you\'re accessing the method `stopPropagation` on a released/nullified synthetic event. This is a no-op function.' +
138+
'If you must keep the original synthetic event around use event.persist().' +
139+
'See https://fb.me/react-event-pooling for more information.'
109140
);
110141
});
111142
});

0 commit comments

Comments
 (0)