Refactor the library

[DigitalBrainJS]

The library has strange, almost dead conditional branches with checking _events and _all props, since _events is always truly, moreover it is initialized with object even in wildcard mode, but never used.

EventEmitter2/lib/eventemitter2.js

Line 222 in 9f8a837

this._events = {};

The _all array stay initialized for always after being first time used.
So in fact these branches are dead (never executed at all):

EventEmitter2/lib/eventemitter2.js

Line 459 in 9f8a837

if (!this._events && !this._all) {

EventEmitter2/lib/eventemitter2.js

Line 567 in 9f8a837

if (!this._events && !this._all) {

These checks are useless:

EventEmitter2/lib/eventemitter2.js

Line 712 in 9f8a837

this._events || init.call(this);

EventEmitter2/lib/eventemitter2.js

Line 463 in 9f8a837

this._events || init.call(this);

EventEmitter2/lib/eventemitter2.js

Line 876 in 9f8a837

if (type === undefined) {

EventEmitter2/lib/eventemitter2.js

Line 904 in 9f8a837

this._events || init.call(this);

And so on.
It seems we should set _events and _all to null if there are no any listeners, or use internal flag to determine if there are any listeners to avoid object reinitiazlization.

emitter.emitAsync has bad logic and returns very strange values.
If _all and _events was not initialized yet it returns false (but fortunately is never executed) not even Promise.resolve(false).
For newListener event and if the vent has no listeners it returns [false]
If _all and _events is initialized and there are no any listeners - []
Moreover it's resolves promises from all listeners array and array of specific event type so there is no ability to determine is value returned from all listener or from a listener of specific event type.
I bellive this should be refactored in major version:

• The method should return resolved Promise with an empty array or false if there are no methods listening the specific event.
• The method should not collect the values from all listeners, just wait while they will be resolved.

[RangerMauve]

Yeah, I don't think I was around when that was introduced. I think emitAsync was a PR which I didn't look too deeply into.

I'm down for a major bump. 😁

[DigitalBrainJS]

Yeap. This lib has a bit more issues that just emitAsync and currently i have not an idea how to fix that and keep 100% compability.
Some internal props isn't marked as private for ex. this.listenerTree, this.delimiter, this.wildcard so i can't refactor that and be sure nobody uses them.
this.listenerTree should be refactored to this._events to simplify code logic
this.delimiter should be refactored to this._delimiter
this.wildcard should be refactored to this._wildcard and so on. In the future their keys could be replaced with private or symbol properties.
this.emit always returns true, even if all the listeners have been removed...

[DigitalBrainJS]

It doesn’t look so good :) We have to fix this 😄

[DigitalBrainJS]

@RangerMauve I just found out that wildcard emitter benchmark is totally fake 😄

EventEmitter2/test/perf/benchmark.js

Line 11 in f3f642f

var emitter2 = new EventEmitter2;

EventEmitter2/test/perf/benchmark.js

Lines 38 to 44 in 799d2f8

	.add('EventEmitter2 (wild)', function() {
	emitter2.on('test2.foo', function () { 1==1; });
	emitter2.emit('test2.foo');
	emitter2.removeAllListeners('test2.foo');
	})

Previously i wonder how that heavy code has a performance only three times less than highly optimized plain emitter. Even plain namespace.split('.') has performance only around 3M op/sec))
So, the secret is revealed. That benchmark tests an emitter not in wildcard mode and the real performance is near 300k operations/sec, not 3.5 millions :D It's 40-50 times slower than simply emitter has.

EventEmitterHeatUp x 3,265,667 ops/sec ±4.18% (64 runs sampled)
EventEmitter x 3,306,892 ops/sec ±3.70% (61 runs sampled)
EventEmitter2 x 11,297,163 ops/sec ±4.96% (62 runs sampled)
EventEmitter2 (wild) x 278,127 ops/sec ±11.46% (64 runs sampled)
EventEmitter3 x 9,987,862 ops/sec ±3.12% (66 runs sampled)

[RangerMauve]

Ah dang! I wonder how long it's been that way. 😂

[DigitalBrainJS]

@RangerMauve not for long, It looks like just about 9 years :)
It seems searchListenerTree is too slow and over complicated for emit purpose.
And it's hard to refactor it, since I don’t know fully how it should work with double ** wildcard and README has almost no info about that. Is unlikely that someone will use wildcard if it is 50 times slower than simple emitter... On the other hand this library is used by 1300 packages, including pm2 :)

[RangerMauve]

When I was using this library actively at work (years ago) I exclusively used it for the wildcards. 😂

[DigitalBrainJS]

@RangerMauve Hm. I never needed to use wildcards emitters, at least something harder than basic one level wildcard e.g. event.* . This lib supports multilevel * and ** and in fact all logic depends on one complex function with heavy recursion that has a pretty low performance. I can't image a challange where we need namespace like event.**.foo.bar.
The previos benchmark was a benchmark for a wildcard emitter from git branch with some optimizations which affected the results. The reality a bit worse (origin master branch):

EventEmitter2 x 12,089,852 ops/sec ±4.00% (62 runs sampled)
EventEmitter2 (wild) x 167,847 ops/sec ±4.90% (63 runs sampled)
EventEmitter2 (wild) using plain events x 190,736 ops/sec ±3.08% (62 runs sampled)
EventEmitter2 (wild) emitting ns x 676,378 ops/sec ±4.07% (67 runs sampled)
EventEmitter2 (wild) emitting a plain event x 764,142 ops/sec ±3.87% (71 runs sampled)
EventEmitter3 x 10,223,124 ops/sec ±3.51% (70 runs sampled)

Even in a case of emitting a simple wildcard event (test.*) it's 75 times slower than a general emitter.