Skip to content

Commit 105c774

Browse files
author
Kent C. Dodds
committed
warn(SyntheticEvent): Warn when accessing or setting properties on released syntheticEvents
1 parent 9c3f595 commit 105c774

File tree

2 files changed

+99
-24
lines changed

2 files changed

+99
-24
lines changed

Diff for: src/renderers/dom/client/syntheticEvents/SyntheticEvent.js

+64-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,44 @@ SyntheticEvent.augmentClass = function(Class, Interface) {
195198
PooledClass.addPoolingTo(SyntheticEvent, PooledClass.fourArgumentPooler);
196199

197200
module.exports = SyntheticEvent;
201+
202+
203+
// Utility functions
204+
205+
/**
206+
* Helper to nullify syntheticEvent instance properties when destructing
207+
*
208+
* @param {object} SyntheticEvent
209+
* @param {String} propName
210+
*/
211+
function getPooledWarningPropertyDefinition(propName, getVal) {
212+
var setTo = typeof getVal === 'function' ? 'a no-op function' : 'set to null';
213+
return {
214+
configurable: true,
215+
set: function(val) {
216+
var warningCondition = false;
217+
warning(
218+
warningCondition,
219+
'This synthetic event is reused for performance reasons. If you\'re ' +
220+
'seeing this, you\'re setting property `%s` on a ' +
221+
'released/nullified synthetic event. This is effectively a no-op. See ' +
222+
'https://fb.me/react-event-pooling for more information.',
223+
propName
224+
);
225+
return val;
226+
},
227+
get: function() {
228+
var warningCondition = false;
229+
warning(
230+
warningCondition,
231+
'This synthetic event is reused for performance reasons. If you\'re ' +
232+
'seeing this, you\'re accessing property `%s` on a ' +
233+
'released/nullified synthetic event. This is %s. See ' +
234+
'https://fb.me/react-event-pooling for more information.',
235+
propName,
236+
setTo
237+
);
238+
return getVal;
239+
},
240+
};
241+
}

Diff for: src/renderers/dom/client/syntheticEvents/__tests__/SyntheticEvent-test.js

+35-4
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');
7677
var target = document.createElement('div');
7778
var syntheticEvent = createEvent({srcElement: target});
7879
syntheticEvent.destructor();
@@ -81,6 +82,36 @@ 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 ' +
94+
'you\'re seeing this, you\'re accessing property `type` on a ' +
95+
'released/nullified synthetic event. This is set to null. See ' +
96+
'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 ' +
109+
'you\'re seeing this, you\'re setting property `type` on a ' +
110+
'released/nullified synthetic event. This is effectively a no-op. See ' +
111+
'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({});
@@ -89,8 +120,8 @@ describe('SyntheticEvent', function() {
89120
expect(console.error.calls.length).toBe(1);
90121
expect(console.error.argsForCall[0][0]).toBe(
91122
'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 ' +
123+
'you\'re seeing this, you\'re accessing property `preventDefault` on a ' +
124+
'released/nullified synthetic event. This is a no-op function. See ' +
94125
'https://fb.me/react-event-pooling for more information.'
95126
);
96127
});
@@ -103,8 +134,8 @@ describe('SyntheticEvent', function() {
103134
expect(console.error.calls.length).toBe(1);
104135
expect(console.error.argsForCall[0][0]).toBe(
105136
'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 ' +
137+
'you\'re seeing this, you\'re accessing property `stopPropagation` on a ' +
138+
'released/nullified synthetic event. This is a no-op function. See ' +
108139
'https://fb.me/react-event-pooling for more information.'
109140
);
110141
});

0 commit comments

Comments
 (0)