JS Custom Event System

Question

I've written the following custom event system module and I'm planning to use it in production in the future. Before I do that, I'd like to ask for general suggestions on how to improve it. Thanks a lot.

const event = (function() {

  let registry = {};

  function on(event, callback) {
    if(!registry[event]) registry[event] = [];
    registry[event].push(callback);
  }

  function once(event, callback) {
    if(!registry[event]) registry[event] = [];
    registry[event].push(function(message) {
      callback(message);
      off(event, callback);
    });
  }

  function off(event, callback) {
    if(registry[event]) {
      registry[event].splice(registry[event].indexOf(callback), 1);
      if(!registry[event].length) delete registry[event];
    }
  }

  function emit(event, message) {
    if(registry[event]) {
      registry[event].forEach(callback => {
        callback(message);
      });
    }
  }

  function getListeners(event) {
    return registry[event] ? registry[event] : null;
  }

  return {
    on: on,
    once: once,
    off: off,
    emit: emit,
    getListeners: getListeners
  }
})();

export { event };

Usage:

Add event listener:

event.on('someEvent', someEventhandler);

Remove event listener:

event.off('someEvent', someEventhandler);

Add event listener and remove it after first use:

event.once('someEvent', someEventhandler);

Fire the event:

event.emit('someEvent');

Get all eventhandlers for an event:

event.getListeners('someEvent');

Answer 1

While there are examples of how the code can be used, it would be helpful for anyone reading the code (including your future self) to have a comment block above each method to describe the purpose, parameters, return value and any possible error that could be thrown.

---

The question is tagged with performance- are you trying to optimize the code for speed? If so, it would be better to use a for loop instead of functional methods that use iterators - e.g. Array.prototype.forEach(), Array.prototype.indexOf().

---

Has the code been tested? Are there any unit tests for the module? If not, it would be wise to create tests to cover all aspects of the code.

Correct me if I am wrong but looking at how the off() method is implemented my presumption is that the call from once() will not actually remove any callback, given that a wrapped function is registered instead of the callback parameter.

---

There appears to be no validation that the callback is a function. This could lead to an error if some other type is passed in that parameter.

---

The shorthand property definition notation can be used to simplify the following lines:

 return {
    on: on,
    once: once,
    off: off,
    emit: emit,
    getListeners: getListeners
  }

To this:

return {
    on,
    once,
    off,
    emit,
    getListeners
  }

---

You may be interested in reading the responses I received on similar code I wrote- see Event emitter npm module

Answer 2

getListeners should not return null but rather an empty array []. Usually this is easier to handle and less error prone for clients. And so you need not delete fields with empty arrays in function off.

Function once can be defined by using function on.