FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

events: fix EventTarget support · nodejs/node@aa1cb3f · GitHub

/ node Public

Commit aa1cb3f

Browse files
benjamingr
authored and
codebytere
committed
events: fix EventTarget support
Remove support for multiple arguments (which don't actually work for EventTarget). Use our EventTarget implementation rather than a mock. Refactor `once` code in preparation of `on` using shared code for supporting `on` with `EventTarget`s. Support EventTarget in the `events.on` static method PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 0f0f4e0 commit aa1cb3f

3 files changed

Lines changed: 121 additions & 111 deletions

File tree

‎lib/events.js‎

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -623,41 +623,20 @@ function unwrapListeners(arr) {
623623

624624
function once(emitter, name) {
625625
return new Promise((resolve, reject) => {
626-
if (typeof emitter.addEventListener === 'function') {
627-
// EventTarget does not have `error` event semantics like Node
628-
// EventEmitters, we do not listen to `error` events here.
629-
emitter.addEventListener(
630-
name,
631-
(...args) => { resolve(args); },
632-
{ once: true }
633-
);
634-
return;
635-
}
636-
637-
const eventListener = (...args) => {
638-
if (errorListener !== undefined) {
626+
const errorListener = (err) => {
627+
emitter.removeListener(name, resolver);
628+
reject(err);
629+
};
630+
const resolver = (...args) => {
631+
if (typeof emitter.removeListener === 'function') {
639632
emitter.removeListener('error', errorListener);
640633
}
641634
resolve(args);
642635
};
643-
let errorListener;
644-
645-
// Adding an error listener is not optional because
646-
// if an error is thrown on an event emitter we cannot
647-
// guarantee that the actual event we are waiting will
648-
// be fired. The result could be a silent way to create
649-
// memory or file descriptor leaks, which is something
650-
// we should avoid.
636+
eventTargetAgnosticAddListener(emitter, name, resolver, { once: true });
651637
if (name !== 'error') {
652-
errorListener = (err) => {
653-
emitter.removeListener(name, eventListener);
654-
reject(err);
655-
};
656-
657-
emitter.once('error', errorListener);
638+
addErrorHandlerIfEventEmitter(emitter, errorListener, { once: true });
658639
}
659-
660-
emitter.once(name, eventListener);
661640
});
662641
}
663642

@@ -668,6 +647,38 @@ function createIterResult(value, done) {
668647
return { value, done };
669648
}
670649

650+
function addErrorHandlerIfEventEmitter(emitter, handler, flags) {
651+
if (typeof emitter.on === 'function') {
652+
eventTargetAgnosticAddListener(emitter, 'error', handler, flags);
653+
}
654+
}
655+
656+
function eventTargetAgnosticRemoveListener(emitter, name, listener, flags) {
657+
if (typeof emitter.removeListener === 'function') {
658+
emitter.removeListener(name, listener);
659+
} else if (typeof emitter.removeEventListener === 'function') {
660+
emitter.removeEventListener(name, listener, flags);
661+
} else {
662+
throw new ERR_INVALID_ARG_TYPE('emitter', 'EventEmitter', emitter);
663+
}
664+
}
665+
666+
function eventTargetAgnosticAddListener(emitter, name, listener, flags) {
667+
if (typeof emitter.on === 'function') {
668+
if (flags && flags.once) {
669+
emitter.once(name, listener);
670+
} else {
671+
emitter.on(name, listener);
672+
}
673+
} else if (typeof emitter.addEventListener === 'function') {
674+
// EventTarget does not have `error` event semantics like Node
675+
// EventEmitters, we do not listen to `error` events here.
676+
emitter.addEventListener(name, (arg) => { listener(arg); }, flags);
677+
} else {
678+
throw new ERR_INVALID_ARG_TYPE('emitter', 'EventEmitter', emitter);
679+
}
680+
}
681+
671682
function on(emitter, event) {
672683
const unconsumedEvents = [];
673684
const unconsumedPromises = [];
@@ -704,8 +715,8 @@ function on(emitter, event) {
704715
},
705716

706717
return() {
707-
emitter.removeListener(event, eventHandler);
708-
emitter.removeListener('error', errorHandler);
718+
eventTargetAgnosticRemoveListener(emitter, event, eventHandler);
719+
eventTargetAgnosticRemoveListener(emitter, 'error', errorHandler);
709720
finished = true;
710721

711722
for (const promise of unconsumedPromises) {
@@ -721,17 +732,20 @@ function on(emitter, event) {
721732
'Error', err);
722733
}
723734
error = err;
724-
emitter.removeListener(event, eventHandler);
725-
emitter.removeListener('error', errorHandler);
735+
eventTargetAgnosticRemoveListener(emitter, event, eventHandler);
736+
eventTargetAgnosticRemoveListener(emitter, 'error', errorHandler);
726737
},
727738

728739
[SymbolAsyncIterator]() {
729740
return this;
730741
}
731742
}, AsyncIteratorPrototype);
732743

733-
emitter.on(event, eventHandler);
734-
emitter.on('error', errorHandler);
744+
eventTargetAgnosticAddListener(emitter, event, eventHandler);
745+
if (event !== 'error') {
746+
addErrorHandlerIfEventEmitter(emitter, errorHandler);
747+
}
748+
735749

736750
return iterator;
737751

‎test/parallel/test-event-on-async-iterator.js‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1+
// Flags: --expose-internals
12
'use strict';
23

34
const common = require('../common');
45
const assert = require('assert');
56
const { on, EventEmitter } = require('events');
7+
const {
8+
EventTarget,
9+
NodeEventTarget,
10+
Event
11+
} = require('internal/event_target');
612

713
async function basic() {
814
const ee = new EventEmitter();
@@ -204,6 +210,45 @@ async function iterableThrow() {
204210
assert.strictEqual(ee.listenerCount('error'), 0);
205211
}
206212

213+
async function eventTarget() {
214+
const et = new EventTarget();
215+
const tick = () => et.dispatchEvent(new Event('tick'));
216+
const interval = setInterval(tick, 0);
217+
let count = 0;
218+
for await (const [ event ] of on(et, 'tick')) {
219+
count++;
220+
assert.strictEqual(event.type, 'tick');
221+
if (count >= 5) {
222+
break;
223+
}
224+
}
225+
assert.strictEqual(count, 5);
226+
clearInterval(interval);
227+
}
228+
229+
async function errorListenerCount() {
230+
const et = new EventEmitter();
231+
on(et, 'foo');
232+
assert.strictEqual(et.listenerCount('error'), 1);
233+
}
234+
235+
async function nodeEventTarget() {
236+
const et = new NodeEventTarget();
237+
const tick = () => et.dispatchEvent(new Event('tick'));
238+
const interval = setInterval(tick, 0);
239+
let count = 0;
240+
for await (const [ event] of on(et, 'tick')) {
241+
count++;
242+
assert.strictEqual(event.type, 'tick');
243+
if (count >= 5) {
244+
break;
245+
}
246+
}
247+
assert.strictEqual(count, 5);
248+
clearInterval(interval);
249+
}
250+
251+
207252
async function run() {
208253
const funcs = [
209254
basic,
@@ -212,7 +257,10 @@ async function run() {
212257
throwInLoop,
213258
next,
214259
nextError,
215-
iterableThrow
260+
iterableThrow,
261+
eventTarget,
262+
errorListenerCount,
263+
nodeEventTarget
216264
];
217265

218266
for (const fn of funcs) {

‎test/parallel/test-events-once.js‎

Lines changed: 23 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,10 @@
11
'use strict';
2+
// Flags: --expose-internals
23

34
const common = require('../common');
45
const { once, EventEmitter } = require('events');
5-
const { strictEqual, deepStrictEqual } = require('assert');
6-
7-
class EventTargetMock {
8-
constructor() {
9-
this.events = {};
10-
}
11-
12-
addEventListener = common.mustCall(function(name, listener, options) {
13-
if (!(name in this.events)) {
14-
this.events[name] = { listeners: [], options };
15-
}
16-
this.events[name].listeners.push(listener);
17-
});
18-
19-
removeEventListener = common.mustCall(function(name, callback) {
20-
if (!(name in this.events)) {
21-
return;
22-
}
23-
const event = this.events[name];
24-
const stack = event.listeners;
25-
26-
for (let i = 0, l = stack.length; i < l; i++) {
27-
if (stack[i] === callback) {
28-
stack.splice(i, 1);
29-
if (stack.length === 0) {
30-
Reflect.deleteProperty(this.events, name);
31-
}
32-
return;
33-
}
34-
}
35-
});
36-
37-
dispatchEvent = function(name, ...arg) {
38-
if (!(name in this.events)) {
39-
return true;
40-
}
41-
const event = this.events[name];
42-
const stack = event.listeners.slice();
43-
44-
for (let i = 0, l = stack.length; i < l; i++) {
45-
stack[i].apply(this, arg);
46-
if (event.options.once) {
47-
this.removeEventListener(name, stack[i]);
48-
}
49-
}
50-
return !name.defaultPrevented;
51-
};
52-
}
6+
const { strictEqual, deepStrictEqual, fail } = require('assert');
7+
const { EventTarget, Event } = require('internal/event_target');
538

549
async function onceAnEvent() {
5510
const ee = new EventEmitter();
@@ -104,8 +59,6 @@ async function stopListeningAfterCatchingError() {
10459
ee.emit('myevent', 42, 24);
10560
});
10661

107-
process.on('multipleResolves', common.mustNotCall());
108-
10962
try {
11063
await once(ee, 'myevent');
11164
} catch (_e) {
@@ -125,54 +78,49 @@ async function onceError() {
12578
ee.emit('error', expected);
12679
});
12780

128-
const [err] = await once(ee, 'error');
81+
const promise = once(ee, 'error');
82+
strictEqual(ee.listenerCount('error'), 1);
83+
const [ err ] = await promise;
12984
strictEqual(err, expected);
13085
strictEqual(ee.listenerCount('error'), 0);
13186
strictEqual(ee.listenerCount('myevent'), 0);
13287
}
13388

13489
async function onceWithEventTarget() {
135-
const et = new EventTargetMock();
136-
90+
const et = new EventTarget();
91+
const event = new Event('myevent');
13792
process.nextTick(() => {
138-
et.dispatchEvent('myevent', 42);
93+
et.dispatchEvent(event);
13994
});
14095
const [ value ] = await once(et, 'myevent');
141-
strictEqual(value, 42);
142-
strictEqual(Reflect.has(et.events, 'myevent'), false);
143-
}
144-
145-
async function onceWithEventTargetTwoArgs() {
146-
const et = new EventTargetMock();
147-
148-
process.nextTick(() => {
149-
et.dispatchEvent('myevent', 42, 24);
150-
});
151-
152-
const value = await once(et, 'myevent');
153-
deepStrictEqual(value, [42, 24]);
96+
strictEqual(value, event);
15497
}
15598

15699
async function onceWithEventTargetError() {
157-
const et = new EventTargetMock();
158-
159-
const expected = new Error('kaboom');
100+
const et = new EventTarget();
101+
const error = new Event('error');
160102
process.nextTick(() => {
161-
et.dispatchEvent('error', expected);
103+
et.dispatchEvent(error);
162104
});
163105

164-
const [err] = await once(et, 'error');
165-
strictEqual(err, expected);
166-
strictEqual(Reflect.has(et.events, 'error'), false);
106+
const [ err ] = await once(et, 'error');
107+
strictEqual(err, error);
167108
}
168109

110+
async function prioritizesEventEmitter() {
111+
const ee = new EventEmitter();
112+
ee.addEventListener = fail;
113+
ee.removeAllListeners = fail;
114+
process.nextTick(() => ee.emit('foo'));
115+
await once(ee, 'foo');
116+
}
169117
Promise.all([
170118
onceAnEvent(),
171119
onceAnEventWithTwoArgs(),
172120
catchesErrors(),
173121
stopListeningAfterCatchingError(),
174122
onceError(),
175123
onceWithEventTarget(),
176-
onceWithEventTargetTwoArgs(),
177124
onceWithEventTargetError(),
125+
prioritizesEventEmitter(),
178126
]).then(common.mustCall());

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL