Issue 6741 - Use ES2015 classes in lib/events.js

lib/events.js

Index: lib/events.js
===================================================================
--- a/lib/events.js
+++ b/lib/events.js
@@ -15,56 +15,55 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 /**
  * Registers and emits named events.
  *
- * @constructor
+ * @class

[Manish Jethani, 2018/08/15 08:12:56]

The @class keyword is redundant now, as far as I know. We can remove it.

[Jon Sonesen, 2018/08/16 20:19:30]

Acknowledged.

  */
-exports.EventEmitter = function()
+exports.EventEmitter = class EventEmitter

[Manish Jethani, 2018/08/15 08:12:56]

Let's declare it like this:

  class EventEmitter
  {
    ...
  }

  exports.EventEmitter = EventEmitter;

Only because we have following this convention for function as well.

Note that there's no semicolon in this case after class { ... }

[Jon Sonesen, 2018/08/16 20:19:30]

Acknowledged.

 {
-  this._listeners = new Map();
-};
+  constructor()
+  {
+    this._listeners = new Map();
+  }
 
-exports.EventEmitter.prototype = {
   /**
    * Adds a listener for the specified event name.
    *
    * @param {string}   name
    * @param {function} listener
    */
   on(name, listener)
   {
     let listeners = this._listeners.get(name);
     if (listeners)
       listeners.push(listener);
     else
       this._listeners.set(name, [listener]);
-  },
-
+  }

[Manish Jethani, 2018/08/15 08:12:56]

Let's leave a blank line after each method.

[Jon Sonesen, 2018/08/16 20:19:30]

Acknowledged.

   /**
    * Removes a listener for the specified event name.
    *
    * @param {string}   name
    * @param {function} listener
    */
   off(name, listener)
   {
     let listeners = this._listeners.get(name);
     if (listeners)
     {
       let idx = listeners.indexOf(listener);
       if (idx != -1)
         listeners.splice(idx, 1);
     }
-  },
-
+  }
   /**
    * Adds a one time listener and returns a promise that
    * is resolved the next time the specified event is emitted.
    * @param {string} name
    * @return {Promise}

[Manish Jethani, 2018/08/15 08:12:56]

@return -> @returns (see comment below)

[Jon Sonesen, 2018/08/16 20:19:30]

Acknowledged.

    */
   once(name)
   {
@@ -73,30 +72,28 @@
       let listener = () =>
       {
         this.off(name, listener);
         resolve();
       };
 
       this.on(name, listener);
     });
-  },
-
+  }
   /**
    * Returns a copy of the array of listeners for the specified event.
    *
    * @param {string} name
    * @return {function[]}

[Manish Jethani, 2018/08/15 08:12:56]

I'm trying to make the JSDoc comments consistent across core so let's also take
this opportunity to do this (Trac issue #6676).

@return -> @returns

function[] -> Array.<function>

[Jon Sonesen, 2018/08/16 20:19:30]

Acknowledged.

    */
   listeners(name)
   {
     let listeners = this._listeners.get(name);
     return listeners ? listeners.slice() : [];
-  },
-
+  }
   /**
    * Calls all previously added listeners for the given event name.
    *
    * @param {string} name
    * @param {...*}   [arg]

[Manish Jethani, 2018/08/15 08:12:57]

Let's rename the JSDoc name of the parameter to "args" (since that's what it's
called in the code).

[Jon Sonesen, 2018/08/16 20:19:30]

Acknowledged.

[Manish Jethani, 2018/08/17 03:07:20]

The "type" was fine (i.e. {...*}), it's the name of the parameter that I wanted
to change to args. I meant like this:

  @param {...*} [args]

    */
   emit(name, ...args)
   {