Remove event pooling in the modern system

Author: gaearon

packages/legacy-events/EventBatching.js

@@ -7,6 +7,7 @@
  */
 
 import invariant from 'shared/invariant';
+import {enableModernEventSystem} from 'shared/ReactFeatureFlags';
 import {rethrowCaughtError} from 'shared/ReactErrorUtils';
 
 import type {ReactSyntheticEvent} from './ReactSyntheticEventType';
@@ -30,8 +31,11 @@ const executeDispatchesAndRelease = function(event: ReactSyntheticEvent) {
   if (event) {
     executeDispatchesInOrder(event);
 
-    if (!event.isPersistent()) {
-      event.constructor.release(event);
+    // Modern event system doesn't use pooling.
+    if (!enableModernEventSystem) {
+      if (!event.isPersistent()) {
+        event.constructor.release(event);
+      }
     }
   }
 };

packages/legacy-events/ResponderEventPlugin.js

@@ -5,6 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import {enableModernEventSystem} from 'shared/ReactFeatureFlags';
 import {getLowestCommonAncestor, isAncestor} from 'shared/ReactTreeTraversal';
 
 import {
@@ -378,8 +379,11 @@ function setResponderAndExtractTransfer(
     accumulateTwoPhaseDispatches(shouldSetEvent);
   }
   const wantsResponderInst = executeDispatchesInOrderStopAtTrue(shouldSetEvent);
-  if (!shouldSetEvent.isPersistent()) {
-    shouldSetEvent.constructor.release(shouldSetEvent);
+  // Modern event system doesn't use pooling.
+  if (!enableModernEventSystem) {
+    if (!shouldSetEvent.isPersistent()) {
+      shouldSetEvent.constructor.release(shouldSetEvent);
+    }
   }
 
   if (!wantsResponderInst || wantsResponderInst === responderInst) {
@@ -409,8 +413,12 @@ function setResponderAndExtractTransfer(
     const shouldSwitch =
       !hasDispatches(terminationRequestEvent) ||
       executeDirectDispatch(terminationRequestEvent);
-    if (!terminationRequestEvent.isPersistent()) {
-      terminationRequestEvent.constructor.release(terminationRequestEvent);
+
+    // Modern event system doesn't use pooling.
+    if (!enableModernEventSystem) {
+      if (!terminationRequestEvent.isPersistent()) {
+        terminationRequestEvent.constructor.release(terminationRequestEvent);
+      }
     }
 
     if (shouldSwitch) {

packages/legacy-events/SyntheticEvent.js

@@ -9,6 +9,8 @@
 
 import invariant from 'shared/invariant';
 
+import {enableModernEventSystem} from 'shared/ReactFeatureFlags';
+
 const EVENT_POOL_SIZE = 10;
 
 /**
@@ -152,71 +154,79 @@ Object.assign(SyntheticEvent.prototype, {
    * won't be added back into the pool.
    */
   persist: function() {
-    this.isPersistent = functionThatReturnsTrue;
+    // Modern event system doesn't use pooling.
+    if (!enableModernEventSystem) {
+      this.isPersistent = functionThatReturnsTrue;
+    }
   },
 
   /**
    * Checks if this event should be released back into the pool.
    *
    * @return {boolean} True if this should not be released, false otherwise.
    */
-  isPersistent: functionThatReturnsFalse,
+  isPersistent: enableModernEventSystem
+    ? functionThatReturnsTrue
+    : functionThatReturnsFalse,
 
   /**
    * `PooledClass` looks for `destructor` on each instance it releases.
    */
   destructor: function() {
-    const Interface = this.constructor.Interface;
-    for (const propName in Interface) {
+    // Modern event system doesn't use pooling.
+    if (!enableModernEventSystem) {
+      const Interface = this.constructor.Interface;
+      for (const propName in Interface) {
+        if (__DEV__) {
+          Object.defineProperty(
+            this,
+            propName,
+            getPooledWarningPropertyDefinition(propName, Interface[propName]),
+          );
+        } else {
+          this[propName] = 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,
-          propName,
-          getPooledWarningPropertyDefinition(propName, Interface[propName]),
+          'nativeEvent',
+          getPooledWarningPropertyDefinition('nativeEvent', null),
         );
-      } else {
-        this[propName] = 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(
+        Object.defineProperty(
+          this,
           'isDefaultPrevented',
-          functionThatReturnsFalse,
-        ),
-      );
-      Object.defineProperty(
-        this,
-        'isPropagationStopped',
-        getPooledWarningPropertyDefinition(
+          getPooledWarningPropertyDefinition(
+            'isDefaultPrevented',
+            functionThatReturnsFalse,
+          ),
+        );
+        Object.defineProperty(
+          this,
           'isPropagationStopped',
-          functionThatReturnsFalse,
-        ),
-      );
-      Object.defineProperty(
-        this,
-        'preventDefault',
-        getPooledWarningPropertyDefinition('preventDefault', () => {}),
-      );
-      Object.defineProperty(
-        this,
-        'stopPropagation',
-        getPooledWarningPropertyDefinition('stopPropagation', () => {}),
-      );
+          getPooledWarningPropertyDefinition(
+            'isPropagationStopped',
+            functionThatReturnsFalse,
+          ),
+        );
+        Object.defineProperty(
+          this,
+          'preventDefault',
+          getPooledWarningPropertyDefinition('preventDefault', () => {}),
+        );
+        Object.defineProperty(
+          this,
+          'stopPropagation',
+          getPooledWarningPropertyDefinition('stopPropagation', () => {}),
+        );
+      }
     }
   },
 });
@@ -296,18 +306,26 @@ function getPooledWarningPropertyDefinition(propName, getVal) {
   }
 }
 
-function getPooledEvent(dispatchConfig, targetInst, nativeEvent, nativeInst) {
+function createOrGetPooledEvent(
+  dispatchConfig,
+  targetInst,
+  nativeEvent,
+  nativeInst,
+) {
   const EventConstructor = this;
-  if (EventConstructor.eventPool.length) {
-    const instance = EventConstructor.eventPool.pop();
-    EventConstructor.call(
-      instance,
-      dispatchConfig,
-      targetInst,
-      nativeEvent,
-      nativeInst,
-    );
-    return instance;
+  // Modern event system doesn't use pooling.
+  if (!enableModernEventSystem) {
+    if (EventConstructor.eventPool.length) {
+      const instance = EventConstructor.eventPool.pop();
+      EventConstructor.call(
+        instance,
+        dispatchConfig,
+        targetInst,
+        nativeEvent,
+        nativeInst,
+      );
+      return instance;
+    }
   }
   return new EventConstructor(
     dispatchConfig,
@@ -318,21 +336,28 @@ function getPooledEvent(dispatchConfig, targetInst, nativeEvent, nativeInst) {
 }
 
 function releasePooledEvent(event) {
-  const EventConstructor = this;
-  invariant(
-    event instanceof EventConstructor,
-    'Trying to release an event instance into a pool of a different type.',
-  );
-  event.destructor();
-  if (EventConstructor.eventPool.length < EVENT_POOL_SIZE) {
-    EventConstructor.eventPool.push(event);
+  // Modern event system doesn't use pooling.
+  if (!enableModernEventSystem) {
+    const EventConstructor = this;
+    invariant(
+      event instanceof EventConstructor,
+      'Trying to release an event instance into a pool of a different type.',
+    );
+    event.destructor();
+    if (EventConstructor.eventPool.length < EVENT_POOL_SIZE) {
+      EventConstructor.eventPool.push(event);
+    }
   }
 }
 
 function addEventPoolingTo(EventConstructor) {
-  EventConstructor.eventPool = [];
-  EventConstructor.getPooled = getPooledEvent;
-  EventConstructor.release = releasePooledEvent;
+  EventConstructor.getPooled = createOrGetPooledEvent;
+
+  // Modern event system doesn't use pooling.
+  if (!enableModernEventSystem) {
+    EventConstructor.eventPool = [];
+    EventConstructor.release = releasePooledEvent;
+  }
 }
 
 export default SyntheticEvent;

packages/react-dom/src/events/DOMModernPluginEventSystem.js

@@ -132,10 +132,7 @@ function dispatchEventsForPlugins(
   for (let i = 0; i < syntheticEvents.length; i++) {
     const syntheticEvent = syntheticEvents[i];
     executeDispatchesInOrder(syntheticEvent);
-    // Release the event from the pool if needed
-    if (!syntheticEvent.isPersistent()) {
-      syntheticEvent.constructor.release(syntheticEvent);
-    }
+    // This doesn't call release because modern system doesn't use pooling.
   }
 }
 

packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js

@@ -169,4 +169,25 @@ describe('DOMModernPluginEventSystem', () => {
     expect(log).toEqual([]);
     expect(onDivClick).toHaveBeenCalledTimes(0);
   });
+
+  it('does not pool events', () => {
+    const buttonRef = React.createRef();
+    const log = [];
+    const onClick = jest.fn(e => log.push(e));
+
+    function Test() {
+      return <button ref={buttonRef} onClick={onClick} />;
+    }
+
+    ReactDOM.render(<Test />, container);
+
+    let buttonElement = buttonRef.current;
+    dispatchClickEvent(buttonElement);
+    expect(onClick).toHaveBeenCalledTimes(1);
+    dispatchClickEvent(buttonElement);
+    expect(onClick).toHaveBeenCalledTimes(2);
+    expect(log[0]).not.toBe(log[1]);
+    expect(log[0].type).toBe('click');
+    expect(log[1].type).toBe('click');
+  });
 });