From 792aa6a7b702baba0599524cb30f7ca52e277dae Mon Sep 17 00:00:00 2001
From: Dylan Piercey <dpiercey@ebay.com>
Date: Fri, 3 Apr 2020 12:43:00 -0700
Subject: [PATCH] perf: optimize merge html attrs (#1538)

(cherry picked from commit 17f0335503d30c46ef4f44090d8931c7e6915470)
---
 .../src/runtime/html/helpers/_dynamic-attr.js | 35 ++++++++++++
 .../marko/src/runtime/html/helpers/attr.js    | 29 +++++++---
 .../marko/src/runtime/html/helpers/attrs.js   | 57 ++++---------------
 .../src/runtime/html/helpers/merge-attrs.js   | 28 +++------
 packages/marko/test/render/index.test.js      | 15 ++++-
 5 files changed, 89 insertions(+), 75 deletions(-)
 create mode 100644 packages/marko/src/runtime/html/helpers/_dynamic-attr.js

diff --git a/packages/marko/src/runtime/html/helpers/_dynamic-attr.js b/packages/marko/src/runtime/html/helpers/_dynamic-attr.js
new file mode 100644
index 000000000..87ecbcd74
--- /dev/null
+++ b/packages/marko/src/runtime/html/helpers/_dynamic-attr.js
@@ -0,0 +1,35 @@
+"use strict";
+var attrHelper = require("./attr");
+var notEmptyAttr = attrHelper.___notEmptyAttr;
+var isEmptyAttrValue = attrHelper.___isEmptyAttrValue;
+var classHelper = require("./class-attr");
+var styleHelper = require("./style-attr");
+
+module.exports = function dynamicAttr(name, value) {
+  switch (name) {
+    case "class":
+      return classHelper(value);
+    case "style":
+      return styleHelper(value);
+    case "renderBody":
+      return "";
+    default:
+      return isEmptyAttrValue(value) || isInvalidAttrName(name)
+        ? ""
+        : notEmptyAttr(name, value);
+  }
+};
+
+// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
+// Technically the above includes more invalid characters for attributes.
+// In practice however the only character that does not become an attribute name
+// is when there is a >.
+function isInvalidAttrName(name) {
+  for (let i = name.length; i--; ) {
+    if (name[i] === ">") {
+      return true;
+    }
+  }
+
+  return false;
+}
diff --git a/packages/marko/src/runtime/html/helpers/attr.js b/packages/marko/src/runtime/html/helpers/attr.js
index a8c41f49f..47de9cc9e 100644
--- a/packages/marko/src/runtime/html/helpers/attr.js
+++ b/packages/marko/src/runtime/html/helpers/attr.js
@@ -4,28 +4,39 @@ var escapeQuoteHelpers = require("./escape-quotes");
 var escapeDoubleQuotes = escapeQuoteHelpers.___escapeDoubleQuotes;
 var escapeSingleQuotes = escapeQuoteHelpers.___escapeSingleQuotes;
 
-module.exports = function attr(name, value) {
+module.exports = maybeEmptyAttr;
+
+maybeEmptyAttr.___notEmptyAttr = notEmptyAttr;
+maybeEmptyAttr.___isEmptyAttrValue = isEmpty;
+
+function maybeEmptyAttr(name, value) {
+  if (isEmpty(value)) {
+    return "";
+  }
+
+  return notEmptyAttr(name, value);
+}
+
+function notEmptyAttr(name, value) {
   switch (typeof value) {
     case "string":
       return " " + name + guessQuotes(value);
     case "boolean":
-      return value ? " " + name : "";
+      return " " + name;
     case "number":
       return " " + name + "=" + value;
-    case "undefined":
-      return "";
     case "object":
-      if (value === null) {
-        return "";
-      }
-
       if (value instanceof RegExp) {
         return " " + name + doubleQuote(value.source);
       }
   }
 
   return " " + name + guessQuotes(value + "");
-};
+}
+
+function isEmpty(value) {
+  return value == null || value === false;
+}
 
 function doubleQuote(value) {
   return '="' + escapeDoubleQuotes(value) + '"';
diff --git a/packages/marko/src/runtime/html/helpers/attrs.js b/packages/marko/src/runtime/html/helpers/attrs.js
index 654715489..e0a9e8e77 100644
--- a/packages/marko/src/runtime/html/helpers/attrs.js
+++ b/packages/marko/src/runtime/html/helpers/attrs.js
@@ -1,51 +1,18 @@
 "use strict";
 
-var attrHelper = require("./attr");
-var classAttrHelper = require("./class-attr");
-var styleAttrHelper = require("./style-attr");
+var dynamicAttrHelper = require("./_dynamic-attr");
 
-// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
-var invalidAttrNameCharacters = /[\s'"</=\\]/u;
-var validAttrs = new Set();
-var invalidAttrs = new Set();
-
-module.exports = function attrs(attributes) {
-  if (attributes != null) {
-    // eslint-disable-next-line no-constant-condition
-    if ("MARKO_DEBUG") {
-      if (typeof attributes !== "object") {
-        throw new Error(
-          "A non object was passed as a dynamic attributes value."
-        );
+module.exports = function attrs(arg) {
+  switch (typeof arg) {
+    case "object":
+      var result = "";
+      for (var attrName in arg) {
+        result += dynamicAttrHelper(attrName, arg[attrName]);
       }
-    }
-
-    var result = "";
-
-    for (var attrName in attributes) {
-      if (attrName === "style") {
-        result += styleAttrHelper(attributes[attrName]);
-      } else if (attrName === "class") {
-        result += classAttrHelper(attributes[attrName]);
-      } else if (attrName !== "renderBody" && isValidAttrName(attrName)) {
-        result += attrHelper(attrName, attributes[attrName]);
-      }
-    }
-    return result;
+      return result;
+    case "string":
+      return arg;
+    default:
+      return "";
   }
-
-  return "";
 };
-
-function isValidAttrName(attrName) {
-  if (validAttrs.has(attrName)) return true;
-  if (invalidAttrs.has(attrName)) return false;
-
-  if (invalidAttrNameCharacters.test(attrName)) {
-    invalidAttrs.add(attrName);
-    return false;
-  }
-
-  validAttrs.add(attrName);
-  return true;
-}
diff --git a/packages/marko/src/runtime/html/helpers/merge-attrs.js b/packages/marko/src/runtime/html/helpers/merge-attrs.js
index 3d1dbd903..469aedb18 100644
--- a/packages/marko/src/runtime/html/helpers/merge-attrs.js
+++ b/packages/marko/src/runtime/html/helpers/merge-attrs.js
@@ -1,33 +1,23 @@
 "use strict";
 
-var attrsHelper = require("./attrs");
-var hasOwnProperty = Object.prototype.hasOwnProperty;
+var dynamicAttrHelper = require("./_dynamic-attr");
 
 /**
  * Merges attribute objects into a string.
  */
 module.exports = function mergeAttrs() {
   var result = "";
-  var finalAttributes = {};
-  for (var i = 0; i < arguments.length; i++) {
-    var attributes = arguments[i];
-    if (attributes != null) {
-      // eslint-disable-next-line no-constant-condition
-      if ("MARKO_DEBUG") {
-        if (typeof attributes !== "object") {
-          throw new Error(
-            "A non object was passed as a dynamic attributes value."
-          );
-        }
-      }
+  var seen = new Set();
 
-      for (var k in attributes) {
-        if (hasOwnProperty.call(attributes, k)) {
-          finalAttributes[k] = attributes[k];
-        }
+  for (var i = arguments.length, last = i - 1; i--; ) {
+    var source = arguments[i];
+    for (var k in source) {
+      if (i === last || !seen.has(k)) {
+        result += dynamicAttrHelper(k, source[k]);
+        seen.add(k);
       }
     }
   }
 
-  return result + attrsHelper(finalAttributes);
+  return result;
 };
diff --git a/packages/marko/test/render/index.test.js b/packages/marko/test/render/index.test.js
index ca1c5cdd8..175dd8c0b 100644
--- a/packages/marko/test/render/index.test.js
+++ b/packages/marko/test/render/index.test.js
@@ -190,8 +190,19 @@ function normalizeHtml(htmlOrNode) {
       isClientReorderFragment(node)
     ) {
       nodesToRemove.push(node);
-    } else if (node.tagName === "TEXTAREA") {
-      node.textContent = node.value;
+    }
+    if (node.nodeType === 1) {
+      if (node.tagName === "TEXTAREA") {
+        node.textContent = node.value;
+      }
+
+      // sort attrs by name.
+      Array.from(node.attributes)
+        .sort((a, b) => a.name.localeCompare(b.name))
+        .forEach(attr => {
+          node.removeAttributeNode(attr);
+          node.setAttributeNode(attr);
+        });
     }
   }