From decff26e818b71dfd785e11e2b75fdddf2efd1ec Mon Sep 17 00:00:00 2001 From: Toru Kobayashi Date: Thu, 18 Feb 2016 18:41:44 +0900 Subject: [PATCH] Add a warning for adding some properties in the SyntheticEvent object if Proxy is supported --- .../SyntheticClipboardEvent.js | 2 +- .../SyntheticCompositionEvent.js | 2 +- .../syntheticEvents/SyntheticDragEvent.js | 2 +- .../client/syntheticEvents/SyntheticEvent.js | 54 +++++++++++++++++-- .../syntheticEvents/SyntheticFocusEvent.js | 2 +- .../syntheticEvents/SyntheticInputEvent.js | 2 +- .../syntheticEvents/SyntheticKeyboardEvent.js | 2 +- .../syntheticEvents/SyntheticMouseEvent.js | 2 +- .../syntheticEvents/SyntheticTouchEvent.js | 2 +- .../syntheticEvents/SyntheticUIEvent.js | 2 +- .../syntheticEvents/SyntheticWheelEvent.js | 2 +- .../__tests__/SyntheticEvent-test.js | 19 +++++++ 12 files changed, 78 insertions(+), 15 deletions(-) diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticClipboardEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticClipboardEvent.js index f50377187f278..cd0d4b2264352 100644 --- 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 index a277d4bf63a0f..35b7f7ce18b7b 100644 --- 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 index e49a1c3b721f6..f5d97420b33be 100644 --- 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 index f3e300cdbbba6..c9d328d0614ac 100644 --- 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 index c951962d18eb4..e145b8a434ade 100644 --- 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 index c33529d943adc..ba99c7c894c6f 100644 --- 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 index 1baf9c8aa0788..ce71c17ca86fc 100644 --- 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 index 0587daac5daca..e8962b9adecba 100644 --- 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 index 4b4331373f9f6..38078a732c75f 100644 --- 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 index f28f3af5f0371..b0eae7bed3e19 100644 --- 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 index faa7f30ea9756..eea628e33e1e7 100644 --- 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 index e8ffc98a83ec5..2f2f7e27a03c0 100644 --- 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); + } + }); });