Question: why do not use events.forEach in emit() function?

[browserify]

Question: why do not use events.forEach in emit() function?

[Daymannovaes]

Possible reasons that I could think of:

Performance

It's common sense that for statement is more performatic than forEach. But I ran some benchmarks test, and I got a surprising result. Apparently modern browsers are optimized for the second case now:

Run at https://jsbench.me/

Run at https://jsben.ch/

Compatibility

ForEach is widely implemented. It has support in 95% of all environments, against 96% of the for statement.

[goto-bus-stop]

For this package the answer is "because node.js does it", and this is a straight port of the Node.js built in module. Changes are made exclusively to broaden browser support.

Your benchmark uses a single listener array of 1000 listeners which is quite unusual in the real world. 99% of the time, there will only be a small amount of listeners for an event, usually just 1. The benchmark might look different if you have less than 10 listeners vs. 1000. Though either way, this module values compatibility over performance :)

e; forEach() also doesn't have the desired behaviour on its own.

function a () { console.log('a') }
function b () { console.log('b') }
function c () { console.log('c') }
function removeB () { listeners.splice(1, 1) }
var listeners = [
  a,
  b,
  removeB,
  c
]
listeners.forEach(listener => listener())

This removes the b() handler during the iteration. But it then never calls the c listener because the indices shifted. For events it is essential that c does still get called. So the proper comparison would be to use arrayClone() and .forEach() instead.

[Daymannovaes]

Right. So, you have any clue on why node choose this pattern?

…

On Tue, Feb 23, 2021, 9:59 AM Renée Kooi ***@***.***> wrote: For this package the answer is "because node.js does it", and this is a straight port of the Node.js built in module. Changes are made exclusively to broaden browser support. Your benchmark uses a single listener array of 1000 listeners which is quite unusual in the real world. 99% of the time, there will only be a small amount of listeners for an event, usually just 1. The benchmark might look different if you have less than 10 listeners vs. 1000. Though either way, this module values compatibility over performance :) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#81 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA32ZGNVQATYLB6RK27J233TAORDBANCNFSM4YCMEKUQ> .

[goto-bus-stop]

Maybe my edit didn't come through email—if you use forEach() you still need to clone first.

That + the performance characteristics possibly being different for small arrays could mean it's not faster to use forEach in the typical case.

[Daymannovaes]

You're correct.

Although, if we clone the array first, we could still use the forEach() approach.

For example:

listeners.slice().forEach(listener => listener());

[goto-bus-stop]

You could, but it's not what Node.js does, probably because it's slower.

I get this:

I used a short array of 3 listeners, which is probably already more than the median.

[Daymannovaes]

Ok, thanks for helping me.

I'll close the issue since my questions are solved.