Inlined DevTools event emitter impl (facebook#18378)

Brian Vaughn · web-flow · commit bd5781962a93 · 2020-03-25T10:26:40.000-07:00
DevTools previously used the NPM events package for dispatching events. This package has an unfortunate flaw though- if a listener throws during event dispatch, no subsequent listeners are called. I've replaced that event dispatcher with my own implementation that ensures all listeners are called before it re-throws an error.

This commit replaces that event emitter with a custom implementation that calls all listeners before re-throwing an error.
diff --git a/packages/react-devtools-shared/package.json b/packages/react-devtools-shared/package.json
@@ -11,7 +11,6 @@
     "@reach/menu-button": "^0.1.17",
     "@reach/tooltip": "^0.2.2",
     "clipboard-js": "^0.3.6",
-    "events": "^3.0.0",
     "local-storage-fallback": "^4.1.1",
     "lodash.throttle": "^4.1.1",
     "memoize-one": "^3.1.1",
diff --git a/packages/react-devtools-shared/src/__tests__/events-test.js b/packages/react-devtools-shared/src/__tests__/events-test.js
@@ -0,0 +1,126 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+describe('events', () => {
+  let dispatcher;
+
+  beforeEach(() => {
+    const EventEmitter = require('../events').default;
+
+    dispatcher = new EventEmitter();
+  });
+
+  it('can dispatch an event with no listeners', () => {
+    dispatcher.emit('event', 123);
+  });
+
+  it('handles a listener being attached multiple times', () => {
+    const callback = jest.fn();
+
+    dispatcher.addListener('event', callback);
+    dispatcher.addListener('event', callback);
+
+    dispatcher.emit('event', 123);
+    expect(callback).toHaveBeenCalledTimes(1);
+    expect(callback).toHaveBeenCalledWith(123);
+  });
+
+  it('notifies all attached listeners of events', () => {
+    const callback1 = jest.fn();
+    const callback2 = jest.fn();
+    const callback3 = jest.fn();
+
+    dispatcher.addListener('event', callback1);
+    dispatcher.addListener('event', callback2);
+    dispatcher.addListener('other-event', callback3);
+    dispatcher.emit('event', 123);
+
+    expect(callback1).toHaveBeenCalledTimes(1);
+    expect(callback1).toHaveBeenCalledWith(123);
+    expect(callback2).toHaveBeenCalledTimes(1);
+    expect(callback2).toHaveBeenCalledWith(123);
+    expect(callback3).not.toHaveBeenCalled();
+  });
+
+  it('calls later listeners before re-throwing if an earlier one throws', () => {
+    const callbackThatThrows = jest.fn(() => {
+      throw Error('expected');
+    });
+    const callback = jest.fn();
+
+    dispatcher.addListener('event', callbackThatThrows);
+    dispatcher.addListener('event', callback);
+
+    expect(() => {
+      dispatcher.emit('event', 123);
+    }).toThrow('expected');
+
+    expect(callbackThatThrows).toHaveBeenCalledTimes(1);
+    expect(callbackThatThrows).toHaveBeenCalledWith(123);
+    expect(callback).toHaveBeenCalledTimes(1);
+    expect(callback).toHaveBeenCalledWith(123);
+  });
+
+  it('removes attached listeners', () => {
+    const callback1 = jest.fn();
+    const callback2 = jest.fn();
+
+    dispatcher.addListener('event', callback1);
+    dispatcher.addListener('other-event', callback2);
+    dispatcher.removeListener('event', callback1);
+    dispatcher.emit('event', 123);
+    expect(callback1).not.toHaveBeenCalled();
+    dispatcher.emit('other-event', 123);
+    expect(callback2).toHaveBeenCalledTimes(1);
+    expect(callback2).toHaveBeenCalledWith(123);
+  });
+
+  it('removes all listeners', () => {
+    const callback1 = jest.fn();
+    const callback2 = jest.fn();
+    const callback3 = jest.fn();
+
+    dispatcher.addListener('event', callback1);
+    dispatcher.addListener('event', callback2);
+    dispatcher.addListener('other-event', callback3);
+    dispatcher.removeAllListeners();
+    dispatcher.emit('event', 123);
+    dispatcher.emit('other-event', 123);
+
+    expect(callback1).not.toHaveBeenCalled();
+    expect(callback2).not.toHaveBeenCalled();
+    expect(callback3).not.toHaveBeenCalled();
+  });
+
+  it('should call the initial listeners even if others are added or removed during a dispatch', () => {
+    const callback1 = jest.fn(() => {
+      dispatcher.removeListener('event', callback2);
+      dispatcher.addListener('event', callback3);
+    });
+    const callback2 = jest.fn();
+    const callback3 = jest.fn();
+
+    dispatcher.addListener('event', callback1);
+    dispatcher.addListener('event', callback2);
+
+    dispatcher.emit('event', 123);
+    expect(callback1).toHaveBeenCalledTimes(1);
+    expect(callback1).toHaveBeenCalledWith(123);
+    expect(callback2).toHaveBeenCalledTimes(1);
+    expect(callback2).toHaveBeenCalledWith(123);
+    expect(callback3).not.toHaveBeenCalled();
+
+    dispatcher.emit('event', 456);
+    expect(callback1).toHaveBeenCalledTimes(2);
+    expect(callback1).toHaveBeenCalledWith(456);
+    expect(callback2).toHaveBeenCalledTimes(1);
+    expect(callback3).toHaveBeenCalledTimes(1);
+    expect(callback3).toHaveBeenCalledWith(456);
+  });
+});
diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js
@@ -7,7 +7,7 @@
  * @flow
  */
 
-import EventEmitter from 'events';
+import EventEmitter from '../events';
 import throttle from 'lodash.throttle';
 import {
   SESSION_STORAGE_LAST_SELECTION_KEY,
diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js
@@ -7,7 +7,7 @@
  * @flow
  */
 
-import EventEmitter from 'events';
+import EventEmitter from './events';
 
 import type {ComponentFilter, Wall} from './types';
 import type {
diff --git a/packages/react-devtools-shared/src/devtools/ProfilerStore.js b/packages/react-devtools-shared/src/devtools/ProfilerStore.js
@@ -7,7 +7,7 @@
  * @flow
  */
 
-import EventEmitter from 'events';
+import EventEmitter from '../events';
 import {prepareProfilingDataFrontendFromBackendAndStore} from './views/Profiler/utils';
 import ProfilingCache from './ProfilingCache';
 import Store from './store';
diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js
@@ -7,7 +7,7 @@
  * @flow
  */
 
-import EventEmitter from 'events';
+import EventEmitter from '../events';
 import {inspect} from 'util';
 import {
   TREE_OPERATION_ADD,
diff --git a/packages/react-devtools-shared/src/events.js b/packages/react-devtools-shared/src/events.js
@@ -0,0 +1,75 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+export default class EventEmitter<Events: Object> {
+  listenersMap: Map<string, Array<Function>> = new Map();
+
+  addListener<Event: $Keys<Events>>(
+    event: Event,
+    listener: (...$ElementType<Events, Event>) => any,
+  ): void {
+    let listeners = this.listenersMap.get(event);
+    if (listeners === undefined) {
+      this.listenersMap.set(event, [listener]);
+    } else {
+      const index = listeners.indexOf(listener);
+      if (index < 0) {
+        listeners.push(listener);
+      }
+    }
+  }
+
+  emit<Event: $Keys<Events>>(
+    event: Event,
+    ...args: $ElementType<Events, Event>
+  ): void {
+    const listeners = this.listenersMap.get(event);
+    if (listeners !== undefined) {
+      if (listeners.length === 1) {
+        // No need to clone or try/catch
+        const listener = listeners[0];
+        listener.apply(null, args);
+      } else {
+        let didThrow = false;
+        let caughtError = null;
+
+        const clonedListeners = Array.from(listeners);
+        for (let i = 0; i < clonedListeners.length; i++) {
+          const listener = clonedListeners[i];
+          try {
+            listener.apply(null, args);
+          } catch (error) {
+            if (caughtError === null) {
+              didThrow = true;
+              caughtError = error;
+            }
+          }
+        }
+
+        if (didThrow) {
+          throw caughtError;
+        }
+      }
+    }
+  }
+
+  removeAllListeners(): void {
+    this.listenersMap.clear();
+  }
+
+  removeListener(event: $Keys<Events>, listener: Function): void {
+    const listeners = this.listenersMap.get(event);
+    if (listeners !== undefined) {
+      const index = listeners.indexOf(listener);
+      if (index >= 0) {
+        listeners.splice(index, 1);
+      }
+    }
+  }
+}
diff --git a/scripts/flow/react-devtools.js b/scripts/flow/react-devtools.js
@@ -7,19 +7,4 @@
  * @flow
  */
 
-declare module 'events' {
-  declare class EventEmitter<Events: Object> {
-    addListener<Event: $Keys<Events>>(
-      event: Event,
-      listener: (...$ElementType<Events, Event>) => any,
-    ): void;
-    emit: <Event: $Keys<Events>>(
-      event: Event,
-      ...$ElementType<Events, Event>
-    ) => void;
-    removeListener(event: $Keys<Events>, listener: Function): void;
-    removeAllListeners(event?: $Keys<Events>): void;
-  }
-
-  declare export default typeof EventEmitter;
-}
+// No types
