Remove the use of proxies for synthetic events in DEV (#13225)

* Revert #5947 and disable the test

* Fix isDefaultPrevented and isPropagationStopped to not get nulled

This was a bug introduced by #5947. It's very confusing that they become nulled while stopPropagation/preventDefault don't.

* Add a comment

* Run Prettier

* Fix grammar

Author: gaearon

packages/events/SyntheticEvent.js

@@ -10,19 +10,8 @@
 import invariant from 'shared/invariant';
 import warningWithoutStack from 'shared/warningWithoutStack';
 
-let didWarnForAddedNewProperty = false;
 const EVENT_POOL_SIZE = 10;
 
-const shouldBeReleasedProperties = [
-  'dispatchConfig',
-  '_targetInst',
-  'nativeEvent',
-  'isDefaultPrevented',
-  'isPropagationStopped',
-  '_dispatchListeners',
-  '_dispatchInstances',
-];
-
 /**
  * @interface Event
  * @see http://www.w3.org/TR/DOM-Level-3-Events/
@@ -81,6 +70,8 @@ function SyntheticEvent(
     delete this.nativeEvent;
     delete this.preventDefault;
     delete this.stopPropagation;
+    delete this.isDefaultPrevented;
+    delete this.isPropagationStopped;
   }
 
   this.dispatchConfig = dispatchConfig;
@@ -188,15 +179,35 @@ Object.assign(SyntheticEvent.prototype, {
         this[propName] = null;
       }
     }
-    for (let i = 0; i < shouldBeReleasedProperties.length; i++) {
-      this[shouldBeReleasedProperties[i]] = null;
-    }
+    this.dispatchConfig = null;
+    this._targetInst = null;
+    this.nativeEvent = null;
+    this.isDefaultPrevented = functionThatReturnsFalse;
+    this.isPropagationStopped = functionThatReturnsFalse;
+    this._dispatchListeners = null;
+    this._dispatchInstances = null;
     if (__DEV__) {
       Object.defineProperty(
         this,
         'nativeEvent',
         getPooledWarningPropertyDefinition('nativeEvent', null),
       );
+      Object.defineProperty(
+        this,
+        'isDefaultPrevented',
+        getPooledWarningPropertyDefinition(
+          'isDefaultPrevented',
+          functionThatReturnsFalse,
+        ),
+      );
+      Object.defineProperty(
+        this,
+        'isPropagationStopped',
+        getPooledWarningPropertyDefinition(
+          'isPropagationStopped',
+          functionThatReturnsFalse,
+        ),
+      );
       Object.defineProperty(
         this,
         'preventDefault',
@@ -237,49 +248,6 @@ SyntheticEvent.extend = function(Interface) {
   return Class;
 };
 
-/** Proxying after everything set on SyntheticEvent
- * to resolve Proxy issue on some WebKit browsers
- * in which some Event properties are set to undefined (GH#10010)
- */
-if (__DEV__) {
-  const isProxySupported =
-    typeof Proxy === 'function' &&
-    // https://github.com/facebook/react/issues/12011
-    !Object.isSealed(new Proxy({}, {}));
-
-  if (isProxySupported) {
-    /*eslint-disable no-func-assign */
-    SyntheticEvent = new Proxy(SyntheticEvent, {
-      construct: function(target, args) {
-        return this.apply(target, Object.create(target.prototype), 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
-            ) {
-              warningWithoutStack(
-                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 */
-  }
-}
-
 addEventPoolingTo(SyntheticEvent);
 
 /**
@@ -354,7 +322,7 @@ function releasePooledEvent(event) {
   const EventConstructor = this;
   invariant(
     event instanceof EventConstructor,
-    'Trying to release an event instance  into a pool of a different type.',
+    'Trying to release an event instance into a pool of a different type.',
   );
   event.destructor();
   if (EventConstructor.eventPool.length < EVENT_POOL_SIZE) {

packages/react-dom/src/events/__tests__/SyntheticEvent-test.js

@@ -249,6 +249,62 @@ describe('SyntheticEvent', () => {
     expect(expectedCount).toBe(1);
   });
 
+  it('should warn when calling `isPropagationStopped` if the synthetic event has not been persisted', () => {
+    let node;
+    let expectedCount = 0;
+    let syntheticEvent;
+
+    const eventHandler = e => {
+      syntheticEvent = e;
+      expectedCount++;
+    };
+    node = ReactDOM.render(<div onClick={eventHandler} />, container);
+
+    const event = document.createEvent('Event');
+    event.initEvent('click', true, true);
+    node.dispatchEvent(event);
+
+    expect(() =>
+      expect(syntheticEvent.isPropagationStopped()).toBe(false),
+    ).toWarnDev(
+      'Warning: This synthetic event is reused for performance reasons. If ' +
+        "you're seeing this, you're accessing the method `isPropagationStopped` on a " +
+        'released/nullified synthetic event. This is a no-op function. If you must ' +
+        'keep the original synthetic event around, use event.persist(). ' +
+        'See https://fb.me/react-event-pooling for more information.',
+      {withoutStack: true},
+    );
+    expect(expectedCount).toBe(1);
+  });
+
+  it('should warn when calling `isDefaultPrevented` if the synthetic event has not been persisted', () => {
+    let node;
+    let expectedCount = 0;
+    let syntheticEvent;
+
+    const eventHandler = e => {
+      syntheticEvent = e;
+      expectedCount++;
+    };
+    node = ReactDOM.render(<div onClick={eventHandler} />, container);
+
+    const event = document.createEvent('Event');
+    event.initEvent('click', true, true);
+    node.dispatchEvent(event);
+
+    expect(() =>
+      expect(syntheticEvent.isDefaultPrevented()).toBe(false),
+    ).toWarnDev(
+      'Warning: This synthetic event is reused for performance reasons. If ' +
+        "you're seeing this, you're accessing the method `isDefaultPrevented` on a " +
+        'released/nullified synthetic event. This is a no-op function. If you must ' +
+        'keep the original synthetic event around, use event.persist(). ' +
+        'See https://fb.me/react-event-pooling for more information.',
+      {withoutStack: true},
+    );
+    expect(expectedCount).toBe(1);
+  });
+
   it('should properly log warnings when events simulated with rendered components', () => {
     let event;
     function assignEvent(e) {
@@ -270,25 +326,25 @@ describe('SyntheticEvent', () => {
     );
   });
 
-  it('should warn if Proxy is supported and the synthetic event is added a property', () => {
+  // TODO: we might want to re-add a warning like this in the future,
+  // but it shouldn't use Proxies because they make debugging difficult.
+  // Or we might disallow this pattern altogether:
+  // https://github.com/facebook/react/issues/13224
+  xit('should warn if a property is added to the synthetic event', () => {
     let node;
     let expectedCount = 0;
     let syntheticEvent;
 
     const eventHandler = e => {
-      if (typeof Proxy === 'function') {
-        expect(() => {
-          e.foo = 'bar';
-        }).toWarnDev(
-          '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.',
-          {withoutStack: true},
-        );
-      } else {
+      expect(() => {
         e.foo = 'bar';
-      }
+      }).toWarnDev(
+        '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.',
+        {withoutStack: true},
+      );
       syntheticEvent = e;
       expectedCount++;
     };