Inlined DevTools event emitter impl (#18378)
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.
Brian Vaughn committed
Mar 25, 2020 at 10:26 UTC
bd5781962a930cb7d046148b9d8ee9ccd3e9247f
8 files changed
+206
-21
packages/react-devtools-shared/package.json
-1
@@ -11,7 +11,6 @@
11
"@reach/menu-button": "^0.1.17",
12
"@reach/tooltip": "^0.2.2",
13
"clipboard-js": "^0.3.6",
14
- "events": "^3.0.0",
14
"local-storage-fallback": "^4.1.1",
15
"lodash.throttle": "^4.1.1",
16
"memoize-one": "^3.1.1",
packages/react-devtools-shared/src/__tests__/events-test.js
new
+126
@@ -0,0 +1,126 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @flow
8
+ */
9
+
10
+describe('events', () => {
11
+ let dispatcher;
12
+
13
+ beforeEach(() => {
14
+ const EventEmitter = require('../events').default;
15
+
16
+ dispatcher = new EventEmitter();
17
+ });
18
+
19
+ it('can dispatch an event with no listeners', () => {
20
+ dispatcher.emit('event', 123);
21
+ });
22
+
23
+ it('handles a listener being attached multiple times', () => {
24
+ const callback = jest.fn();
25
+
26
+ dispatcher.addListener('event', callback);
27
+ dispatcher.addListener('event', callback);
28
+
29
+ dispatcher.emit('event', 123);
30
+ expect(callback).toHaveBeenCalledTimes(1);
31
+ expect(callback).toHaveBeenCalledWith(123);
32
+ });
33
+
34
+ it('notifies all attached listeners of events', () => {
35
+ const callback1 = jest.fn();
36
+ const callback2 = jest.fn();
37
+ const callback3 = jest.fn();
38
+
39
+ dispatcher.addListener('event', callback1);
40
+ dispatcher.addListener('event', callback2);
41
+ dispatcher.addListener('other-event', callback3);
42
+ dispatcher.emit('event', 123);
43
+
44
+ expect(callback1).toHaveBeenCalledTimes(1);
45
+ expect(callback1).toHaveBeenCalledWith(123);
46
+ expect(callback2).toHaveBeenCalledTimes(1);
47
+ expect(callback2).toHaveBeenCalledWith(123);
48
+ expect(callback3).not.toHaveBeenCalled();
49
+ });
50
+
51
+ it('calls later listeners before re-throwing if an earlier one throws', () => {
52
+ const callbackThatThrows = jest.fn(() => {
53
+ throw Error('expected');
54
+ });
55
+ const callback = jest.fn();
56
+
57
+ dispatcher.addListener('event', callbackThatThrows);
58
+ dispatcher.addListener('event', callback);
59
+
60
+ expect(() => {
61
+ dispatcher.emit('event', 123);
62
+ }).toThrow('expected');
63
+
64
+ expect(callbackThatThrows).toHaveBeenCalledTimes(1);
65
+ expect(callbackThatThrows).toHaveBeenCalledWith(123);
66
+ expect(callback).toHaveBeenCalledTimes(1);
67
+ expect(callback).toHaveBeenCalledWith(123);
68
+ });
69
+
70
+ it('removes attached listeners', () => {
71
+ const callback1 = jest.fn();
72
+ const callback2 = jest.fn();
73
+
74
+ dispatcher.addListener('event', callback1);
75
+ dispatcher.addListener('other-event', callback2);
76
+ dispatcher.removeListener('event', callback1);
77
+ dispatcher.emit('event', 123);
78
+ expect(callback1).not.toHaveBeenCalled();
79
+ dispatcher.emit('other-event', 123);
80
+ expect(callback2).toHaveBeenCalledTimes(1);
81
+ expect(callback2).toHaveBeenCalledWith(123);
82
+ });
83
+
84
+ it('removes all listeners', () => {
85
+ const callback1 = jest.fn();
86
+ const callback2 = jest.fn();
87
+ const callback3 = jest.fn();
88
+
89
+ dispatcher.addListener('event', callback1);
90
+ dispatcher.addListener('event', callback2);
91
+ dispatcher.addListener('other-event', callback3);
92
+ dispatcher.removeAllListeners();
93
+ dispatcher.emit('event', 123);
94
+ dispatcher.emit('other-event', 123);
95
+
96
+ expect(callback1).not.toHaveBeenCalled();
97
+ expect(callback2).not.toHaveBeenCalled();
98
+ expect(callback3).not.toHaveBeenCalled();
99
+ });
100
+
101
+ it('should call the initial listeners even if others are added or removed during a dispatch', () => {
102
+ const callback1 = jest.fn(() => {
103
+ dispatcher.removeListener('event', callback2);
104
+ dispatcher.addListener('event', callback3);
105
+ });
106
+ const callback2 = jest.fn();
107
+ const callback3 = jest.fn();
108
+
109
+ dispatcher.addListener('event', callback1);
110
+ dispatcher.addListener('event', callback2);
111
+
112
+ dispatcher.emit('event', 123);
113
+ expect(callback1).toHaveBeenCalledTimes(1);
114
+ expect(callback1).toHaveBeenCalledWith(123);
115
+ expect(callback2).toHaveBeenCalledTimes(1);
116
+ expect(callback2).toHaveBeenCalledWith(123);
117
+ expect(callback3).not.toHaveBeenCalled();
118
+
119
+ dispatcher.emit('event', 456);
120
+ expect(callback1).toHaveBeenCalledTimes(2);
121
+ expect(callback1).toHaveBeenCalledWith(456);
122
+ expect(callback2).toHaveBeenCalledTimes(1);
123
+ expect(callback3).toHaveBeenCalledTimes(1);
124
+ expect(callback3).toHaveBeenCalledWith(456);
125
+ });
126
+});
packages/react-devtools-shared/src/backend/agent.js
+1
-1
@@ -7,7 +7,7 @@
7
* @flow
8
*/
9
10
-import EventEmitter from 'events';
10
+import EventEmitter from '../events';
11
import throttle from 'lodash.throttle';
12
import {
13
SESSION_STORAGE_LAST_SELECTION_KEY,
packages/react-devtools-shared/src/bridge.js
+1
-1
@@ -7,7 +7,7 @@
7
* @flow
8
*/
9
10
-import EventEmitter from 'events';
10
+import EventEmitter from './events';
11
12
import type {ComponentFilter, Wall} from './types';
13
import type {
packages/react-devtools-shared/src/devtools/ProfilerStore.js
+1
-1
@@ -7,7 +7,7 @@
7
* @flow
8
*/
9
10
-import EventEmitter from 'events';
10
+import EventEmitter from '../events';
11
import {prepareProfilingDataFrontendFromBackendAndStore} from './views/Profiler/utils';
12
import ProfilingCache from './ProfilingCache';
13
import Store from './store';
packages/react-devtools-shared/src/devtools/store.js
+1
-1
@@ -7,7 +7,7 @@
7
* @flow
8
*/
9
10
-import EventEmitter from 'events';
10
+import EventEmitter from '../events';
11
import {inspect} from 'util';
12
import {
13
TREE_OPERATION_ADD,
packages/react-devtools-shared/src/events.js
new
+75
@@ -0,0 +1,75 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @flow
8
+ */
9
+
10
+export default class EventEmitter<Events: Object> {
11
+ listenersMap: Map<string, Array<Function>> = new Map();
12
+
13
+ addListener<Event: $Keys<Events>>(
14
+ event: Event,
15
+ listener: (...$ElementType<Events, Event>) => any,
16
+ ): void {
17
+ let listeners = this.listenersMap.get(event);
18
+ if (listeners === undefined) {
19
+ this.listenersMap.set(event, [listener]);
20
+ } else {
21
+ const index = listeners.indexOf(listener);
22
+ if (index < 0) {
23
+ listeners.push(listener);
24
+ }
25
+ }
26
+ }
27
+
28
+ emit<Event: $Keys<Events>>(
29
+ event: Event,
30
+ ...args: $ElementType<Events, Event>
31
+ ): void {
32
+ const listeners = this.listenersMap.get(event);
33
+ if (listeners !== undefined) {
34
+ if (listeners.length === 1) {
35
+ // No need to clone or try/catch
36
+ const listener = listeners[0];
37
+ listener.apply(null, args);
38
+ } else {
39
+ let didThrow = false;
40
+ let caughtError = null;
41
+
42
+ const clonedListeners = Array.from(listeners);
43
+ for (let i = 0; i < clonedListeners.length; i++) {
44
+ const listener = clonedListeners[i];
45
+ try {
46
+ listener.apply(null, args);
47
+ } catch (error) {
48
+ if (caughtError === null) {
49
+ didThrow = true;
50
+ caughtError = error;
51
+ }
52
+ }
53
+ }
54
+
55
+ if (didThrow) {
56
+ throw caughtError;
57
+ }
58
+ }
59
+ }
60
+ }
61
+
62
+ removeAllListeners(): void {
63
+ this.listenersMap.clear();
64
+ }
65
+
66
+ removeListener(event: $Keys<Events>, listener: Function): void {
67
+ const listeners = this.listenersMap.get(event);
68
+ if (listeners !== undefined) {
69
+ const index = listeners.indexOf(listener);
70
+ if (index >= 0) {
71
+ listeners.splice(index, 1);
72
+ }
73
+ }
74
+ }
75
+}
scripts/flow/react-devtools.js
+1
-16
@@ -7,19 +7,4 @@
7
* @flow
8
*/
9
10
-declare module 'events' {
11
- declare class EventEmitter<Events: Object> {
12
- addListener<Event: $Keys<Events>>(
13
- event: Event,
14
- listener: (...$ElementType<Events, Event>) => any,
15
- ): void;
16
- emit: <Event: $Keys<Events>>(
17
- event: Event,
18
- ...$ElementType<Events, Event>
19
- ) => void;
20
- removeListener(event: $Keys<Events>, listener: Function): void;
21
- removeAllListeners(event?: $Keys<Events>): void;
22
- }
23
-
24
- declare export default typeof EventEmitter;
25
-}
10
+// No types