From a94687fb23f90f1072d1b9b0c6f68e00d5ddd0f6 Mon Sep 17 00:00:00 2001
From: Louis-Dominique Dubeau <ldd@lddubeau.com>
Date: Mon, 2 Jul 2018 15:09:36 -0400
Subject: [PATCH] refactor: provide default no-op implementation for events

BREAKING CHANGE:

By default parsers now have a default no-op implementation for each
event it supports. This would break code that determines whether a
custom handler was added by checking whether there's any handler at
all. This removes the necessity for the parser implementation to check
whether there is a handler before calling it.

In the process of making this change, we've removed support for the
``on...`` properties on streams objects. Their existence was not
warranted by any standard API provided by Node. (``EventEmitter`` does
not have ``on...`` properties for events it supports, nor does
``Stream``.) Their existence was also undocumented. And their
functioning was awkward. For instance, with sax, this:

```
const s = sax.createStream();
const handler = () => console.log("moo");
s.on("cdata", handler);
console.log(s.oncdata === handler);
```

would print ``false``. If you examine ``s.oncdata`` you see it is glue
code instead of the handler assigned. This is just bizarre, so we
removed it.
---
 lib/saxes.js | 81 +++++++++++++++++++++++-----------------------------
 1 file changed, 35 insertions(+), 46 deletions(-)

diff --git a/lib/saxes.js b/lib/saxes.js
index 95255ada..92b51ad2 100644
--- a/lib/saxes.js
+++ b/lib/saxes.js
@@ -99,8 +99,6 @@ catch (ex) {
   Stream = function FakeStream() {};
 }
 
-const streamWraps = exports.EVENTS.filter(ev => ev !== "error" && ev !== "end");
-
 function isWhitespace(c) {
   return c === " " || c === "\n" || c === "\r" || c === "\t";
 }
@@ -164,9 +162,29 @@ class SAXParser {
     if (this.trackPosition) {
       this.position = this.line = this.column = 0;
     }
-    this.emit("onready");
+    this.onready();
   }
 
+  // We provide default no-op handlers.
+  /* eslint-disable class-methods-use-this */
+  ontext() {}
+  onprocessinginstruction() {}
+  ondoctype() {}
+  oncomment() {}
+  onopentagstart() {}
+  onattribute() {}
+  onopentag() {}
+  onclosetag() {}
+  onopencdata() {}
+  oncdata() {}
+  onclosecdata() {}
+  onerror() {}
+  onend() {}
+  onready() {}
+  onopennamespace() {}
+  onclosenamespace() {}
+  /* eslint-enable class-methods-use-this */
+
   end() {
     if (this.sawRoot && !this.closedRoot) {
       this.fail("Unclosed root tag");
@@ -179,7 +197,7 @@ class SAXParser {
     this.closeText();
     this.c = "";
     this.closed = true;
-    this.emit("onend");
+    this.onend();
     this._init(this.opt);
     return this;
   }
@@ -193,7 +211,7 @@ class SAXParser {
     }
     er = new Error(er);
     this.error = er;
-    this.emit("onerror", er);
+    this.onerror(er);
     return this;
   }
 
@@ -797,15 +815,9 @@ class SAXParser {
     }
   }
 
-  emit(event, data) {
-    if (this[event]) {
-      this[event](data);
-    }
-  }
-
   closeText() {
     if (this.textNode) {
-      this.emit("ontext", this.textNode);
+      this.ontext(this.textNode);
     }
     this.textNode = "";
   }
@@ -814,7 +826,7 @@ class SAXParser {
     if (this.textNode) {
       this.closeText();
     }
-    this.emit(nodeType, data);
+    this[nodeType](data);
   }
 
   attrib() {
@@ -1035,6 +1047,7 @@ class SAXParser {
   }
 }
 
+const streamWraps = exports.EVENTS.filter(ev => ev !== "error");
 class SAXStream extends Stream {
   constructor(opt) {
     super();
@@ -1043,9 +1056,15 @@ class SAXStream extends Stream {
     this.writable = true;
     this.readable = true;
 
-    this._parser.onend = () => {
-      this.emit("end");
-    };
+    this._decoder = null;
+
+    for (const ev of streamWraps) {
+      // Override the no-op defaults with handlers that emit on the
+      // stream.
+      this._parser[`on${ev}`] = (...args) => {
+        this.emit(ev, ...args);
+      };
+    }
 
     this._parser.onerror = (er) => {
       this.emit("error", er);
@@ -1054,26 +1073,6 @@ class SAXStream extends Stream {
       // go ahead and clear error, so we can write again.
       this._parser.error = null;
     };
-
-    this._decoder = null;
-
-    for (const ev of streamWraps) {
-      Object.defineProperty(this, `on${ev}`, {
-        get() {
-          return this._parser[`on${ev}`];
-        },
-        set(h) {
-          if (!h) {
-            this.removeAllListeners(ev);
-            this._parser[`on${ev}`] = h;
-            return;
-          }
-          this.on(ev, h);
-        },
-        enumerable: true,
-        configurable: false,
-      });
-    }
   }
 
   write(data) {
@@ -1100,16 +1099,6 @@ class SAXStream extends Stream {
     this._parser.end();
     return true;
   }
-
-  on(ev, handler) {
-    if (!this._parser[`on${ev}`] && streamWraps.indexOf(ev) !== -1) {
-      this._parser[`on${ev}`] = (...args) => {
-        this.emit(ev, ...args);
-      };
-    }
-
-    return Stream.prototype.on.call(this, ev, handler);
-  }
 }
 
 function createStream(opt) {