Skip to content

Commit

Permalink
warn(SyntheticEvent): Warn when accessing or setting properties on re…
Browse files Browse the repository at this point in the history 
…leased syntheticEvents
  • Loading branch information
Kent C. Dodds committed Jan 29, 2016
1 parent 9c3f595 commit 105c774
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 24 deletions.
84 changes: 64 additions & 20 deletions src/renderers/dom/client/syntheticEvents/SyntheticEvent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ var EventInterface = {
* @param {DOMEventTarget} nativeEventTarget Target node.
*/
function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarget) {
if (__DEV__) {
// these have a getter/setter for warnings
delete this.nativeEvent;
delete this.preventDefault;
delete this.stopPropagation;
}

this.dispatchConfig = dispatchConfig;
this._targetInst = targetInst;
this.nativeEvent = nativeEvent;
Expand All @@ -64,6 +71,9 @@ function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarg
if (!Interface.hasOwnProperty(propName)) {
continue;
}
if (__DEV__) {
delete this[propName]; // this has a getter/setter for warnings
}
var normalize = Interface[propName];
if (normalize) {
this[propName] = normalize(nativeEvent);
Expand Down Expand Up @@ -92,15 +102,6 @@ assign(SyntheticEvent.prototype, {
preventDefault: function() {
this.defaultPrevented = true;
var event = this.nativeEvent;
if (__DEV__) {
warning(
event,
'This synthetic event is reused for performance reasons. If you\'re ' +
'seeing this, you\'re calling `preventDefault` on a ' +
'released/nullified synthetic event. This is a no-op. See ' +
'https://fb.me/react-event-pooling for more information.'
);
}
if (!event) {
return;
}
Expand All @@ -115,15 +116,6 @@ assign(SyntheticEvent.prototype, {

stopPropagation: function() {
var event = this.nativeEvent;
if (__DEV__) {
warning(
event,
'This synthetic event is reused for performance reasons. If you\'re ' +
'seeing this, you\'re calling `stopPropagation` on a ' +
'released/nullified synthetic event. This is a no-op. See ' +
'https://fb.me/react-event-pooling for more information.'
);
}
if (!event) {
return;
}
Expand Down Expand Up @@ -158,11 +150,22 @@ assign(SyntheticEvent.prototype, {
destructor: function() {
var Interface = this.constructor.Interface;
for (var propName in Interface) {
this[propName] = null;
if (__DEV__) {
Object.defineProperty(this, propName, getPooledWarningPropertyDefinition(propName, Interface[propName]));
} else {
this[propName] = null;
}
}
if (__DEV__) {
var noop = require('emptyFunction');
Object.defineProperty(this, 'nativeEvent', getPooledWarningPropertyDefinition('nativeEvent', null));
Object.defineProperty(this, 'preventDefault', getPooledWarningPropertyDefinition('preventDefault', noop));
Object.defineProperty(this, 'stopPropagation', getPooledWarningPropertyDefinition('stopPropagation', noop));
} else {
this.nativeEvent = null;
}
this.dispatchConfig = null;
this._targetInst = null;
this.nativeEvent = null;
},

});
Expand Down Expand Up @@ -195,3 +198,44 @@ SyntheticEvent.augmentClass = function(Class, Interface) {
PooledClass.addPoolingTo(SyntheticEvent, PooledClass.fourArgumentPooler);

module.exports = SyntheticEvent;


// Utility functions

/**
* Helper to nullify syntheticEvent instance properties when destructing
*
* @param {object} SyntheticEvent
* @param {String} propName
*/
function getPooledWarningPropertyDefinition(propName, getVal) {
var setTo = typeof getVal === 'function' ? 'a no-op function' : 'set to null';
return {
configurable: true,
set: function(val) {
var warningCondition = false;
warning(
warningCondition,
'This synthetic event is reused for performance reasons. If you\'re ' +
'seeing this, you\'re setting property `%s` on a ' +
'released/nullified synthetic event. This is effectively a no-op. See ' +
'https://fb.me/react-event-pooling for more information.',
propName
);
return val;
},
get: function() {
var warningCondition = false;
warning(
warningCondition,
'This synthetic event is reused for performance reasons. If you\'re ' +
'seeing this, you\'re accessing property `%s` on a ' +
'released/nullified synthetic event. This is %s. See ' +
'https://fb.me/react-event-pooling for more information.',
propName,
setTo
);
return getVal;
},
};
}
39 changes: 35 additions & 4 deletions src/renderers/dom/client/syntheticEvents/__tests__/SyntheticEvent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ describe('SyntheticEvent', function() {
});

it('should be nullified if the synthetic event has called destructor', function() {
spyOn(console, 'error');
var target = document.createElement('div');
var syntheticEvent = createEvent({srcElement: target});
syntheticEvent.destructor();
Expand All @@ -81,6 +82,36 @@ describe('SyntheticEvent', function() {
expect(syntheticEvent.target).toBe(null);
});

it('should warn when accessing properties of a destructored synthetic event', function() {
spyOn(console, 'error');
var target = document.createElement('div');
var syntheticEvent = createEvent({srcElement: target});
syntheticEvent.destructor();
expect(syntheticEvent.type).toBe(null);
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: This synthetic event is reused for performance reasons. If ' +
'you\'re seeing this, you\'re accessing property `type` on a ' +
'released/nullified synthetic event. This is set to null. See ' +
'https://fb.me/react-event-pooling for more information.'
);
});

it('should warn when setting properties of a destructored synthetic event', function() {
spyOn(console, 'error');
var target = document.createElement('div');
var syntheticEvent = createEvent({srcElement: target});
syntheticEvent.destructor();
expect(syntheticEvent.type = 'MouseEvent').toBe('MouseEvent');
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: This synthetic event is reused for performance reasons. If ' +
'you\'re seeing this, you\'re setting property `type` on a ' +
'released/nullified synthetic event. This is effectively a no-op. See ' +
'https://fb.me/react-event-pooling for more information.'
);
});

it('should warn if the synthetic event has been released when calling `preventDefault`', function() {
spyOn(console, 'error');
var syntheticEvent = createEvent({});
Expand All @@ -89,8 +120,8 @@ describe('SyntheticEvent', function() {
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: This synthetic event is reused for performance reasons. If ' +
'you\'re seeing this, you\'re calling `preventDefault` on a ' +
'released/nullified synthetic event. This is a no-op. See ' +
'you\'re seeing this, you\'re accessing property `preventDefault` on a ' +
'released/nullified synthetic event. This is a no-op function. See ' +
'https://fb.me/react-event-pooling for more information.'
);
});
Expand All @@ -103,8 +134,8 @@ describe('SyntheticEvent', function() {
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: This synthetic event is reused for performance reasons. If ' +
'you\'re seeing this, you\'re calling `stopPropagation` on a ' +
'released/nullified synthetic event. This is a no-op. See ' +
'you\'re seeing this, you\'re accessing property `stopPropagation` on a ' +
'released/nullified synthetic event. This is a no-op function. See ' +
'https://fb.me/react-event-pooling for more information.'
);
});
Expand Down

0 comments on commit 105c774

Please sign in to comment.