Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

throw errors when adding a listener that is not a function #42

Merged
merged 1 commit into from

2 participants

@FugueNation

No description provided.

@hij1nx
Owner

I feel a little like this is an over caution, and for such a hot path, this is a big performance hit... im on the fence about putting this in.

@FugueNation

Is emitter.on a hot path? I could see how emitter.emit is a hot path, but .on is something you call at most a few times (or at least only at certain checkpoints)

Also node's EventEmitter does throw an error

$node

var ee2 = new (require('eventemitter2').EventEmitter2)();
var ee = new (require('events').EventEmitter)();
ee.on('test', null)
Error: addListener only takes instances of Function
at EventEmitter. (events.js:94:11)
at [object Context]:1:4
at Interface. (repl.js:179:22)
at Interface.emit (events.js:64:17)
at Interface._onLine (readline.js:153:10)
at Interface._line (readline.js:408:8)
at Interface._ttyWrite (readline.js:585:14)
at ReadStream. (readline.js:73:12)
at ReadStream.emit (events.js:81:20)
at ReadStream._emitKey (tty_posix.js:307:10)
ee2.on('test', null)

@FugueNation

The fix came because a developer accidentally added a null object to an emitter and we had to track down why on earth were we not receiving and emit even after registering one

var listener;
... some buggy code with an if to set the appropriate listener
ee2.on('myevent', listener)

... some place else
ee2.emit('myevent', {stuff:1})

@hij1nx
Owner

actually. since im not trying to push this into core anymore, might as well pull it in.

@hij1nx hij1nx merged commit d143719 into asyncly:master
@FugueNation

thanks for the merge!

@hij1nx
Owner

np!! =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 1 deletion.
  1. +3 −1 lib/eventemitter2.js
View
4 lib/eventemitter2.js
@@ -1,4 +1,3 @@
-
;!function(exports, undefined) {
var isArray = Array.isArray;
@@ -251,6 +250,9 @@
};
EventEmitter.prototype.on = function(type, listener) {
+ if (typeof listener !== 'function') {
+ throw new Error('on only accepts instances of Function');
+ }
this._events || init.call(this);
// To avoid recursion in the case that type == "newListeners"! Before
Something went wrong with that request. Please try again.