Merge pull request #5947 from koba04/add-warning-for-adding-synthetic…

…-event-property Add a warning for adding a property in the SyntheticEvent object
facebook · Feb 24, 2016 · dff05be · dff05be
2 parents 7a3009c + decff26
commit dff05be
Show file tree

Hide file tree

Showing 12 changed files with 78 additions and 15 deletions.
diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticClipboardEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticClipboardEvent.js
@@ -34,7 +34,7 @@ var ClipboardEventInterface = {
  * @extends {SyntheticUIEvent}
  */
 function SyntheticClipboardEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticEvent.augmentClass(SyntheticClipboardEvent, ClipboardEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticCompositionEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticCompositionEvent.js
@@ -33,7 +33,7 @@ function SyntheticCompositionEvent(
   nativeEvent,
   nativeEventTarget
 ) {
-  SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticEvent.augmentClass(

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticDragEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticDragEvent.js
@@ -28,7 +28,7 @@ var DragEventInterface = {
  * @extends {SyntheticUIEvent}
  */
 function SyntheticDragEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticMouseEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticMouseEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticMouseEvent.augmentClass(SyntheticDragEvent, DragEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js
@@ -17,6 +17,19 @@ var assign = require('Object.assign');
 var emptyFunction = require('emptyFunction');
 var warning = require('warning');
 
+var didWarnForAddedNewProperty = false;
+var isProxySupported = typeof Proxy === 'function';
+
+var shouldBeReleasedProperties = [
+  'dispatchConfig',
+  '_targetInst',
+  'nativeEvent',
+  'isDefaultPrevented',
+  'isPropagationStopped',
+  '_dispatchListeners',
+  '_dispatchInstances',
+];
+
 /**
  * @interface Event
  * @see http://www.w3.org/TR/DOM-Level-3-Events/
@@ -57,7 +70,7 @@ var EventInterface = {
 function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarget) {
   if (__DEV__) {
     // these have a getter/setter for warnings
-    delete this.nativeEvent; 
+    delete this.nativeEvent;
     delete this.preventDefault;
     delete this.stopPropagation;
   }
@@ -95,6 +108,7 @@ function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarg
     this.isDefaultPrevented = emptyFunction.thatReturnsFalse;
   }
   this.isPropagationStopped = emptyFunction.thatReturnsFalse;
+  return this;
 }
 
 assign(SyntheticEvent.prototype, {
@@ -156,22 +170,52 @@ assign(SyntheticEvent.prototype, {
         this[propName] = null;
       }
     }
+    for (var i = 0; i < shouldBeReleasedProperties.length; i++) {
+      this[shouldBeReleasedProperties[i]] = 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;
   },
 
 });
 
 SyntheticEvent.Interface = EventInterface;
 
+if (__DEV__) {
+  if (isProxySupported) {
+    /*eslint-disable no-func-assign */
+    SyntheticEvent = new Proxy(SyntheticEvent, {
+      construct: function(target, args) {
+        return this.apply(target, {}, args);
+      },
+      apply: function(constructor, that, args) {
+        return new Proxy(constructor.apply(that, args), {
+          set: function(target, prop, value) {
+            if (prop !== 'isPersistent' &&
+                !target.constructor.Interface.hasOwnProperty(prop) &&
+                shouldBeReleasedProperties.indexOf(prop) === -1) {
+              warning(
+                didWarnForAddedNewProperty || target.isPersistent(),
+                'This synthetic event is reused for performance reasons. If you\'re ' +
+                'seeing this, you\'re adding a new property in the synthetic event object. ' +
+                'The property is never released. See ' +
+                'https://fb.me/react-event-pooling for more information.'
+              );
+              didWarnForAddedNewProperty = true;
+            }
+            target[prop] = value;
+            return true;
+          },
+        });
+      },
+    });
+    /*eslint-enable no-func-assign */
+  }
+}
 /**
  * Helper to reduce boilerplate when creating subclasses.
  *

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticFocusEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticFocusEvent.js
@@ -28,7 +28,7 @@ var FocusEventInterface = {
  * @extends {SyntheticUIEvent}
  */
 function SyntheticFocusEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticUIEvent.augmentClass(SyntheticFocusEvent, FocusEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticInputEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticInputEvent.js
@@ -34,7 +34,7 @@ function SyntheticInputEvent(
   nativeEvent,
   nativeEventTarget
 ) {
-  SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticEvent.augmentClass(

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticKeyboardEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticKeyboardEvent.js
@@ -76,7 +76,7 @@ var KeyboardEventInterface = {
  * @extends {SyntheticUIEvent}
  */
 function SyntheticKeyboardEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticUIEvent.augmentClass(SyntheticKeyboardEvent, KeyboardEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticMouseEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticMouseEvent.js
@@ -72,7 +72,7 @@ var MouseEventInterface = {
  * @extends {SyntheticUIEvent}
  */
 function SyntheticMouseEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticUIEvent.augmentClass(SyntheticMouseEvent, MouseEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticTouchEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticTouchEvent.js
@@ -37,7 +37,7 @@ var TouchEventInterface = {
  * @extends {SyntheticUIEvent}
  */
 function SyntheticTouchEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticUIEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticUIEvent.augmentClass(SyntheticTouchEvent, TouchEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticUIEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticUIEvent.js
@@ -51,7 +51,7 @@ var UIEventInterface = {
  * @extends {SyntheticEvent}
  */
 function SyntheticUIEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticEvent.augmentClass(SyntheticUIEvent, UIEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticWheelEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticWheelEvent.js
@@ -50,7 +50,7 @@ var WheelEventInterface = {
  * @extends {SyntheticMouseEvent}
  */
 function SyntheticWheelEvent(dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget) {
-  SyntheticMouseEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
+  return SyntheticMouseEvent.call(this, dispatchConfig, dispatchMarker, nativeEvent, nativeEventTarget);
 }
 
 SyntheticMouseEvent.augmentClass(SyntheticWheelEvent, WheelEventInterface);

diff --git a/src/renderers/dom/client/syntheticEvents/__tests__/SyntheticEvent-test.js b/src/renderers/dom/client/syntheticEvents/__tests__/SyntheticEvent-test.js
@@ -160,4 +160,23 @@ describe('SyntheticEvent', function() {
       'See https://fb.me/react-event-pooling for more information.'
     );
   });
+
+  it('should warn if Proxy is supported and the synthetic event is added a property', function() {
+    spyOn(console, 'error');
+    var syntheticEvent = createEvent({});
+    syntheticEvent.foo = 'bar';
+    SyntheticEvent.release(syntheticEvent);
+    expect(syntheticEvent.foo).toBe('bar');
+    if (typeof Proxy === '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 adding a new property in the synthetic event object. ' +
+        'The property is never released. See ' +
+        'https://fb.me/react-event-pooling for more information.'
+      );
+    } else {
+      expect(console.error.calls.length).toBe(0);
+    }
+  });
 });