Skip to content

Commit decab71

Browse files
committed
events: remove reaches into _events internals
Refactor lib & src code to eliminate all deep reaches into the internal _events dictionary object, instead use available APIs and add an extra method to EventEmitter: rawListeners. PR-URL: #17440 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent e24ad97 commit decab71

File tree

7 files changed

+69
-22
lines changed

7 files changed

+69
-22
lines changed

doc/api/events.md

+33
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,39 @@ to indicate an unlimited number of listeners.
574574

575575
Returns a reference to the `EventEmitter`, so that calls can be chained.
576576

577+
### emitter.rawListeners(eventName)
578+
<!-- YAML
579+
added: REPLACEME
580+
-->
581+
- `eventName` {any}
582+
583+
Returns a copy of the array of listeners for the event named `eventName`,
584+
including any wrappers (such as those created by `.once`).
585+
586+
```js
587+
const emitter = new EventEmitter();
588+
emitter.once('log', () => console.log('log once'));
589+
590+
// Returns a new Array with a function `onceWrapper` which has a property
591+
// `listener` which contains the original listener bound above
592+
const listeners = emitter.rawListeners('log');
593+
const logFnWrapper = listeners[0];
594+
595+
// logs "log once" to the console and does not unbind the `once` event
596+
logFnWrapper.listener();
597+
598+
// logs "log once" to the console and removes the listener
599+
logFnWrapper();
600+
601+
emitter.on('log', () => console.log('log persistently'));
602+
// will return a new Array with a single function bound by `on` above
603+
const newListeners = emitter.rawListeners('log');
604+
605+
// logs "log persistently" twice
606+
newListeners[0]();
607+
emitter.emit('log');
608+
```
609+
577610
[`--trace-warnings`]: cli.html#cli_trace_warnings
578611
[`EventEmitter.defaultMaxListeners`]: #events_eventemitter_defaultmaxlisteners
579612
[`domain`]: domain.html

lib/events.js

+14-4
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ module.exports = EventEmitter;
3232
EventEmitter.EventEmitter = EventEmitter;
3333

3434
EventEmitter.prototype._events = undefined;
35+
EventEmitter.prototype._eventsCount = 0;
3536
EventEmitter.prototype._maxListeners = undefined;
3637

3738
// By default EventEmitters will print a warning if more than 10 listeners are
@@ -357,8 +358,8 @@ EventEmitter.prototype.removeAllListeners =
357358
return this;
358359
};
359360

360-
EventEmitter.prototype.listeners = function listeners(type) {
361-
const events = this._events;
361+
function _listeners(target, type, unwrap) {
362+
const events = target._events;
362363

363364
if (events === undefined)
364365
return [];
@@ -368,9 +369,18 @@ EventEmitter.prototype.listeners = function listeners(type) {
368369
return [];
369370

370371
if (typeof evlistener === 'function')
371-
return [evlistener.listener || evlistener];
372+
return unwrap ? [evlistener.listener || evlistener] : [evlistener];
373+
374+
return unwrap ?
375+
unwrapListeners(evlistener) : arrayClone(evlistener, evlistener.length);
376+
}
377+
378+
EventEmitter.prototype.listeners = function listeners(type) {
379+
return _listeners(this, type, true);
380+
};
372381

373-
return unwrapListeners(evlistener);
382+
EventEmitter.prototype.rawListeners = function rawListeners(type) {
383+
return _listeners(this, type, false);
374384
};
375385

376386
EventEmitter.listenerCount = function(emitter, type) {

lib/internal/bootstrap_node.js

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
function startup() {
1515
const EventEmitter = NativeModule.require('events');
16-
process._eventsCount = 0;
1716

1817
const origProcProto = Object.getPrototypeOf(process);
1918
Object.setPrototypeOf(origProcProto, EventEmitter.prototype);

lib/vm.js

+3-10
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,15 @@ const realRunInThisContext = Script.prototype.runInThisContext;
5656
const realRunInContext = Script.prototype.runInContext;
5757

5858
Script.prototype.runInThisContext = function(options) {
59-
if (options && options.breakOnSigint && process._events.SIGINT) {
59+
if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
6060
return sigintHandlersWrap(realRunInThisContext, this, [options]);
6161
} else {
6262
return realRunInThisContext.call(this, options);
6363
}
6464
};
6565

6666
Script.prototype.runInContext = function(contextifiedSandbox, options) {
67-
if (options && options.breakOnSigint && process._events.SIGINT) {
67+
if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
6868
return sigintHandlersWrap(realRunInContext, this,
6969
[contextifiedSandbox, options]);
7070
} else {
@@ -95,14 +95,7 @@ function createScript(code, options) {
9595
// Remove all SIGINT listeners and re-attach them after the wrapped function
9696
// has executed, so that caught SIGINT are handled by the listeners again.
9797
function sigintHandlersWrap(fn, thisArg, argsArray) {
98-
// Using the internal list here to make sure `.once()` wrappers are used,
99-
// not the original ones.
100-
let sigintListeners = process._events.SIGINT;
101-
102-
if (Array.isArray(sigintListeners))
103-
sigintListeners = sigintListeners.slice();
104-
else
105-
sigintListeners = [sigintListeners];
98+
const sigintListeners = process.rawListeners('SIGINT');
10699

107100
process.removeAllListeners('SIGINT');
108101

src/env.h

-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ class ModuleWrap;
145145
V(env_pairs_string, "envPairs") \
146146
V(errno_string, "errno") \
147147
V(error_string, "error") \
148-
V(events_string, "_events") \
149148
V(exiting_string, "_exiting") \
150149
V(exit_code_string, "exitCode") \
151150
V(exit_string, "exit") \

src/node.cc

-6
Original file line numberDiff line numberDiff line change
@@ -3354,12 +3354,6 @@ void SetupProcessObject(Environment* env,
33543354
env->SetMethod(process, "_setupNextTick", SetupNextTick);
33553355
env->SetMethod(process, "_setupPromises", SetupPromises);
33563356
env->SetMethod(process, "_setupDomainUse", SetupDomainUse);
3357-
3358-
// pre-set _events object for faster emit checks
3359-
Local<Object> events_obj = Object::New(env->isolate());
3360-
CHECK(events_obj->SetPrototype(env->context(),
3361-
Null(env->isolate())).FromJust());
3362-
process->Set(env->events_string(), events_obj);
33633357
}
33643358

33653359

test/parallel/test-event-emitter-listeners.js

+19
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,22 @@ function listener2() {}
8282
const s = new TestStream();
8383
assert.deepStrictEqual(s.listeners('foo'), []);
8484
}
85+
86+
{
87+
const ee = new events.EventEmitter();
88+
ee.on('foo', listener);
89+
const wrappedListener = ee.rawListeners('foo');
90+
assert.strictEqual(wrappedListener.length, 1);
91+
assert.strictEqual(wrappedListener[0], listener);
92+
assert.notStrictEqual(wrappedListener, ee.rawListeners('foo'));
93+
ee.once('foo', listener);
94+
const wrappedListeners = ee.rawListeners('foo');
95+
assert.strictEqual(wrappedListeners.length, 2);
96+
assert.strictEqual(wrappedListeners[0], listener);
97+
assert.notStrictEqual(wrappedListeners[1], listener);
98+
assert.strictEqual(wrappedListeners[1].listener, listener);
99+
assert.notStrictEqual(wrappedListeners, ee.rawListeners('foo'));
100+
ee.emit('foo');
101+
assert.strictEqual(wrappedListeners.length, 2);
102+
assert.strictEqual(wrappedListeners[1].listener, listener);
103+
}

0 commit comments

Comments
 (0)