warn(SyntheticEvent): Warn when accessing or setting properties on re…

…leased syntheticEvents
facebook · Jan 29, 2016 · fa5582e · fa5582e
1 parent 9c3f595
commit fa5582e
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 23 deletions.
diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js
@@ -35,6 +35,11 @@ var EventInterface = {
   defaultPrevented: null,
   isTrusted: null,
 };
+/**
+ * These are additional properties not in the EventInterface
+ * which are nulled when destructoring a syntheticEvent instance
+ */
+var otherNullableProps = ['dispatchConfig', '_targetInst', 'nativeEvent'];
 
 /**
  * Synthetic events are dispatched by event plugins, typically in response to a
@@ -55,15 +60,20 @@ var EventInterface = {
  * @param {DOMEventTarget} nativeEventTarget Target node.
  */
 function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarget) {
+  for (var i = 0; i < otherNullableProps.length; i++) {
+    delete this[otherNullableProps[i]];
+  }
   this.dispatchConfig = dispatchConfig;
   this._targetInst = targetInst;
   this.nativeEvent = nativeEvent;
+  this._destructored = false;
 
   var Interface = this.constructor.Interface;
   for (var propName in Interface) {
     if (!Interface.hasOwnProperty(propName)) {
       continue;
     }
+    delete this[propName];
     var normalize = Interface[propName];
     if (normalize) {
       this[propName] = normalize(nativeEvent);
@@ -90,17 +100,21 @@ function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarg
 assign(SyntheticEvent.prototype, {
 
   preventDefault: function() {
+    if (this._destructored) {
+      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.'
+        );
+      }
+      return;
+    }
+
     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;
     }
@@ -114,16 +128,19 @@ 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 (this._destructored) {
+      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.'
+        );
+      }
+      return;
     }
+    var event = this.nativeEvent;
     if (!event) {
       return;
     }
@@ -156,13 +173,45 @@ assign(SyntheticEvent.prototype, {
    * `PooledClass` looks for `destructor` on each instance it releases.
    */
   destructor: function() {
+    var setNull = setToNullOrWarning.bind(this);
     var Interface = this.constructor.Interface;
-    for (var propName in Interface) {
-      this[propName] = null;
+    for (var interfacePropName in Interface) {
+      setNull(interfacePropName);
+    }
+    otherNullableProps.forEach(setNull);
+    this._destructored = true;
+
+    function setToNullOrWarning(propName) {
+      if (__DEV__) {
+        Object.defineProperty(this, propName, {
+          set: function(val) {
+            // no-op
+            var warningCondition = false;
+            warning(
+              warningCondition,
+              'This synthetic event is reused for performance reasons. If you\'re ' +
+              'seeing this, you\'re setting property `' + propName + '` on a ' +
+              'released/nullified synthetic event. This is effectively a no-op. See ' +
+              'https://fb.me/react-event-pooling for more information.'
+            );
+            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 `' + propName + '` on a ' +
+              'released/nullified synthetic event. This is set to null. See ' +
+              'https://fb.me/react-event-pooling for more information.'
+            );
+            return null;
+          },
+        });
+      } else {
+        this[propName] = null;
+      }
     }
-    this.dispatchConfig = null;
-    this._targetInst = null;
-    this.nativeEvent = null;
   },
 
 });

diff --git a/src/renderers/dom/client/syntheticEvents/__tests__/SyntheticEvent-test.js b/src/renderers/dom/client/syntheticEvents/__tests__/SyntheticEvent-test.js
@@ -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();
@@ -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({});