Event emitter npm module

Question

I was asked to complete the task described below:

Event Emitters are objects that serve as the core building block in event-driven architectures. They simplify the process of handling asynchronous events and enable clean and decoupled code.

Create an Event Emitter module in JavaScript (as modern of a version as you prefer) with documentation and tests. Your implementation should allow for:

• Emitting named events with any number of arguments.
• Registering handler functions for named events that are passed the appropriate arguments on emission.
• Registering a "one-time" handler that will be called at most one time.
• Removing specific previously-registered event handlers and/or all previously-registered event handlers.

This module should be suitable for publishing to npm, though it is not necessary for you to do so.

Do not subclass or otherwise require an existing Event Emitter module, and do not include any dependencies apart from testing or development dependencies.

Below is the module I created, as well as the unit tests I developed as I created the module. It is similar to the NodeJS Event Emitter API but obviously not as complex.

How does the module and test code look? What would you recommend be done differently?

Emitter module:

"use strict";

function Emitter() {
  //constructor
  this.eventHandlers = {};
}
/**
 *  Emit an event
 * @param event string
 * @param arg1...argN - any arguments to be sent to callback functions
 */
Emitter.prototype.emit = function(event) {
  const args = Array.from(arguments).slice(1);
  if (this.eventHandlers.hasOwnProperty(event)) {
    let indexesToRemove = [];
    for (const index in this.eventHandlers[event]) {
      const handler = this.eventHandlers[event][index];
      handler.callback.apply(null, args);
      if (handler.hasOwnProperty('once')) {
        indexesToRemove.push(index);
      }
    }
    if (indexesToRemove.length) {
      for(const index in indexesToRemove) {
        this.eventHandlers[event].splice(index, 1);
      }
    }
  }
};
/**
 * Register a callback for an event
 * @param event
 * @param callback
 */
Emitter.prototype.on = function(event, callback) {
  addHandler.call(this, event, {callback: callback});
};

/**
 * Register a callback for an event to be called only once
 * @param event
 * @param callback
 */
Emitter.prototype.once = function(event, callback) {
  addHandler.call(this, event, {callback: callback, once: true});
};
/**
* Un-register a single or all callbacks for a given event
* @param event
* @param callback optional
*/
Emitter.prototype.off = function(event, callback) {
  if (this.eventHandlers.hasOwnProperty(event)) {
    if (callback !== undefined) {
      for (const index in this.eventHandlers[event]) {
        if (callback.toString() == this.eventHandlers[event][index].callback.toString()) {
          this.eventHandlers[event].splice(index, 1);
        }
      }
    }
    else {
      delete this.eventHandlers[event];
    }
  }
}

module.exports = Emitter;


/** 
* Helper function to add an event handler
* @param event
* @param handlerObject
*/
function addHandler(event, handlerObject) {
  if (!(event in this.eventHandlers)) {
    this.eventHandlers[event] = [];
  }
  this.eventHandlers[event].push(handlerObject);
}

Unit tests

const EventEmitter = require("../src/");
const chai = require("chai");
const sinon = require('sinon');
const sinonChai = require("sinon-chai");
chai.should();
chai.use(sinonChai);

describe("Event Emitter", function() {
  let emitter;
  before(function() {
    emitter = new EventEmitter();
  });
  it("Allows registering handler functions for named events that are passed the appropriate arguments on emission.", function() {
    const callback = sinon.fake();
    emitter.on('scrape', callback);
    emitter.emit('scrape');
    callback.should.have.been.called;

    const testValue1 = 'testValue1';
    emitter.emit('scrape', testValue1);
    callback.should.have.been.calledWith(testValue1);

    const multiArgs = ['a', 'scraped', 'value'];
    emitter.emit('scrape', ...multiArgs);
    callback.should.have.been.calledWith(...multiArgs);
  });    
  it("Allows Registering a \"one-time\" handler that will be called at most one time.", function() {
    const callback = sinon.fake();
    const callback2 = sinon.fake();
    emitter.once('pull', callback);
    emitter.on('pull', callback2);

    emitter.emit('pull');
    emitter.emit('pull');

    callback.should.have.been.calledOnce;
    callback2.should.have.been.calledTwice;
  });
  it("Allows Removing specific previously-registered event handlers and/or all previously-registered event handlers.", function() {
    const callback = sinon.fake();
    const callback2 = sinon.fake();
    const callback3 = sinon.fake();
    const callback4 = sinon.fake();

    emitter.on('push', callback);
    emitter.on('push', callback2);
    emitter.off('push', callback);
    emitter.emit('push');

    callback.should.not.have.been.called;
    callback2.should.have.been.called;

    emitter.on('push', callback3);
    emitter.off('push');
    emitter.emit('push');

    emitter.on('unshift', callback4);
    emitter.emit('unshift');

    callback3.should.not.have.been.called;
    callback4.should.have.been.called;    
  });
});

Answer 1

sineemore already covered almost everything I wanted to say.

1. Make up your mind about what version of JS you are targeting.You use let, which was introduced in ES2015, but opt for Array.from(arguments).slice(1) instead of the more readable ...args which was also introduced in ES2015. I recommend going for a later version as it makes it possible to write more readable code.

2. Since you store events in an object, users are forced to use string keys. This is probably what they will do anyways, but since the specification does not provide this assurance, your implementation should correctly handle more complex event keys. I recommend using a Map instead of Object.create(null).

3. The specification mentions removing all event handlers. I took this to mean all event handlers, not just all event handlers registered for the passed event. If I call emitter.off(), I would expect the eventHandlers object to be reset.

4. Don't bother checking if an array contains content before iterating over it. It just creates more noise and provides next to no performance benefit.

5. Most event emitter libraries allow passing in a context argument to the on which this will be bound to. I'd recommend adding this simple feature as it can greatly improve usability. With arrow functions, I don't use this feature in any implementation I consume, and I don't include it anymore in implementations I write.

6. If I pass the same function to on multiple times, what should happen? Should it drop the second subscription? What if the first subscription is with once and the second with on? The reverse? What your library does should be documented and is worth some thought.

7. Consider failing early if passed invalid arguments. I'd much rather be notified that a handler is not a function when calling on than when calling emit.

8. Don't repeat yourself! emit can and should use off to remove once events instead of doing it within the function.

---

I wanted to give this a shot myself, so I added a few tests to address sineemore's points and my own.

it('Works with toString', function() {
  const callback = sinon.fake();
  emitter.on('toString', callback);
  emitter.emit('toString');
  callback.should.have.been.called;
})

it('Works with object references', function() {
  const callback = sinon.fake();
  emitter.on({}, callback);
  emitter.emit({});
  callback.should.not.have.been.called;
});

it('Does not apply `off` calls in the middle of emitting', function() {
  const callback = sinon.fake();
  emitter.once('event', () => {
    emitter.off('event', callback);
  })
  emitter.on('event', callback);
  emitter.emit('event');
  callback.should.have.been.called;
});

Implementation:

// The assert* functions really belong in their own module.

/**
 * Throws an error if fn is not a function.
 * @param {function?} fn
 * @param {string} name
 */
function assertFunction(fn, name) {
  if (typeof fn !== 'function') {
    throw new Error(`Expected ${name} to be a function. Got ${typeof fn}.`);
  }
}

/**
 * Throws an error if arg is not defined.
 * @param {*} arg
 * @param {string} name
 */
function assertDefined(arg, name) {
  if (arg === undefined) {
    throw new Error(`Expected ${name} to be defined.`);
  }
}

/**
 * Factory function to create an event emitter.
 */
function Emitter() {
  const listeners = new Map();
  const getHandlers = event => listeners.get(event) || listeners.set(event, []).get(event);

  /**
   * Attaches a listener to the emitter.
   * If on is called multiple times with the same callback, the callback will be subscribed multiple times.
   * @param {*} event may not be undefined.
   * @param {function} callback
   * @param {*} once if this callback should only be called once.
   */
  function on(event, callback, once = false) {
    assertDefined(event, 'event');
    assertFunction(callback, 'callback');
    const handlers = getHandlers(event);
    handlers.push({ callback, once });
  }

  /**
   * Attaches a listener to the emitter that will be called at most once.
   * @see on
   * @param {*} event may not be undefined
   * @param {function} callback
   */
  function once(event, callback) {
    on(event, callback, true);
  }

  /**
   * Removes listeners from an event, or from all events.
   * If a callback is subscribed multiple times, all subscriptions will be cancelled.
   * @param {*} event the event to remove listeners from, if not specified, all listeners on all events will be removed.
   * @param {function?} callback the listener to remove, if not specified all listeners on event will be removed.
   */
  function off(event, callback) {
    if (event === undefined) {
      listeners.clear();
    } else if (callback === undefined) {
      listeners.delete(event);
    } else {
      const handlers = getHandlers(event).filter(({ callback: cb }) => cb !== callback);
      listeners.set(event, handlers);
    }
  }

  /**
   * Fires an event, calling all subscribed listeners in their subscription order.
   * If a listener throws, listeners subscribed after that listener will not be called and this function will throw.
   * @param {*} event
   * @param {*[]} args arguments to pass into the subscribed listeners.
   */
  function emit(event, ...args) {
    assertDefined(event, 'event');
    getHandlers(event).slice().forEach(handler => {
      if (handler.once) {
        off(event, handler.callback);
      }
      handler.callback(...args);
    });
  }

  return { on, once, off, emit }
}

const emitter = Emitter()
emitter.on('test', console.log)
emitter.once('test', msg => console.log('>Once<', msg))
emitter.emit('test', 'It works!')
emitter.emit('test', 'Again!')

Answer 2

Caveats of {}

In JavaScript {} is often used as a map with user provided values being object property names. This can be a recipe for disaster. See this particular piece of code, that results in error:

var ev = new Emitter();

ev.once("toString", function () {
    console.log("Oops!");
});

ev.emit("toString");

The error we'll get comes from addHandler function that uses in operator.

The in operator returns true if the specified property is in the specified object or its prototype chain.

When addHandler is called with event "toString" it checks if "toString" in this.eventHandlers. It finds one in Object.prototype since {} inherits from it. From now on we are in trouble.

If we replace the in with hasOwnProperty call we'll still be in trouble when someone desides that hasOwnProperty is a good name for an event:

var ev = new Emitter();

ev.once("hasOwnProperty", function () {
    console.log("Still in trouble!");
});

ev.emit("hasOwnProperty");

After that all this.eventHandlers.hasOwnProperty expressions will evaluate to array of handlers:

var a = { hasOwnProperty: [] };
a.hasOwnProperty();

To safely call hasOwnProperty on object with user provided property names we should use function from Object.prototype directly:

// JavaScript best practices, so sweet
Object.prototype.hasOwnProperty.call(this.eventHandlers, event);

Actually same stands for any standard method we want to call.

Instead of this I suggest you to use Object.create(null). It will create an empty object without prototype so it will be safe to use bracket lookup notation evading all hasOwnProperty mess.

See it in action:

var a = {};
console.log("a = {}");
console.log("constructor" in a, "toString" in a, "hasOwnProperty" in a);

var a = Object.create(null);
console.log("a = Object.create(null)");
console.log("constructor" in a, "toString" in a, "hasOwnProperty" in a);

See more about this on StackOverflow answer.

once

once method has one particular fail case when same event is emitted directly from the event handler:

var ev = new Emitter();

ev.once("foo", function () {
    console.log("Hey, I was called!");
    ev.emit("foo");
});

ev.emit("foo");

I suggest you to remove the once handler right before it being called.

off

The off method removes registered event handler. But what if we call it directly from an event handler:

var ev = new Emitter();

ev.on("foo", function () {
    console.log("First");
    ev.off("foo");
});

ev.on("foo", function () {
    console.log("Second");
});

ev.emit("foo");

Here we have two handlers for the same event "foo". When the first handler is called it removes all "foo" handlers with delete this.eventHandlers[event]. But when it returns we'll still be in the very for loop trying to access next event handler in this.eventHandlers[event] which was recently deleted:

var foo = { bar: [1, 2] };

for (var i = 0; i < foo.bar.length; i++) {
  console.log(foo.bar[i]);
  delete foo.bar;
}

Callbacks comparsion

The off method allows the following:

ev.on("foo", function() { /* handler code */ });
ev.off("foo", function() { /* handler code */ });

The right way to remove event handler involves storing the handler function and using it later with off method:

var handler = function() { /* handler code */ };
ev.on("foo", handler);
ev.off("foo", handler);

You are comparing functions by their string representation:

callback.toString() == this.eventHandlers[event][index].callback.toString()

Actually, you can compare functions directly:

callback === this.eventHandlers[event][index].callback

Example:

var a = function() {
  console.log(Math.PI);
}

var b = function() {
  console.log(Math.PI);
}

var c = a;
   
console.log(function(){} !== function(){}); // different functions
console.log(a !== b);                       // different functions
console.log(a === c);                       // same function
console.log(a.toString() === b.toString()); // different functions,
                                            // but same body

Overall behaviour

A quote from Node.js Events documentation:

Note that once an event has been emitted, all listeners attached to it at the time of emitting will be called in order. This implies that any removeListener() or removeAllListeners() calls after emitting and before the last listener finishes execution will not remove them from emit() in progress.

So in short nothing should mutate a pending array of triggered event handlers.

To accomplish this you can slice this.eventHandlers[event] and iterate on it.

let handlers = this.eventHandlers[event].slice(0);
for (var i = 0; i < handlers.length; i++) {
    const handler = handlers[i];
    // ...
}

This will handle the off error described above.

Other notes

• I don't see point in if (handler.hasOwnProperty('once')), consider if (handler.once) instead.
• index is a long name for a loop variable, how about plain old i?
• for (var i in arr) form is for iteration over object property names where iteration order is not guranteed. To iterate over arrays, use for (var i = 0; i < arr.length; i++) instead.