From dbb38793c1049d8d004d33affc5f5b867acff89e Mon Sep 17 00:00:00 2001
From: Joe Lencioni <joe.lencioni@gmail.com>
Date: Tue, 7 Mar 2017 10:29:48 -0800
Subject: [PATCH] Improve `importantify()` performance (#201)

* Avoid unused capture group in IMPORTANT_RE

I was rolling through the code here looking for some opportunities to
speed it up and I noticed that there is a capture group in this regex
that isn't being used. By changing `(` to `(?:` we can avoid capturing
this group while still being able to use the parens for grouping.

* Use string for importantify replace

I was curious about the performance characteristics of various options
here, and using a string in this way seems to have the edge, at least in
Firefox (35% faster) and Safari (12% faster)--Chrome performance seems
pretty similar for all possibilities. I also find this version more
readable, so it seems like we may as well go with this option.

https://jsperf.com/replace-string-vs-function/

* Simplify importantify regex

I don't think we actually need a capture group at all here and can
accomplish the same thing by just anchoring on the end of the string.
This reduces the runtime of this function in my benchmark from ~105ms to
~35ms.

* Scope importantify to just the value

Since we only call this function from one place, and we already have the
CSS property name and value split out, we can further optimize this hot
path by only running importantify on the CSS property value. In my
benchmark, this change speeds up this map by ~22% (~128ms to ~100ms).

* Remove regex from importantify

Now that this has been sufficiently simplified, we can easily remove the
regex entirely.

* Add bracket optimization to importantify

In our microbenchmark, adding this optimization performs a good amount
better. Since it is a simple optimization to add, it seems like we may
as well do it.

https://jsperf.com/slice-vs-bracket-string-check-with-imporant

I considered using bracket notation for the whole check, but I decided
that was too verbose. So instead, I decided to just check for one
character and follow up with a more thorough but concise check
afterward. This optimization favors the case where the style does not
already have `!important` on it, which is by and large likely to be the
general case.

* Move conditional out of loop in generateCSSRuleset

This cleans up the code a bit, and is also faster!

https://jsperf.com/if-vs-extra-function/1
---
 src/generate.js | 12 +++++++-----
 src/util.js     | 20 ++++++++++++--------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/generate.js b/src/generate.js
index 6db7a9a..7d46912 100644
--- a/src/generate.js
+++ b/src/generate.js
@@ -284,11 +284,13 @@ export const generateCSSRuleset = (
         })
     );
 
-    const rules = prefixedRules.map(([key, value]) => {
-        const stringValue = stringifyValue(key, value);
-        const ret = `${kebabifyStyleName(key)}:${stringValue};`;
-        return useImportant === false ? ret : importantify(ret);
-    }).join("");
+    const transformValue = (useImportant === false)
+        ? stringifyValue
+        : (key, value) => importantify(stringifyValue(key, value));
+
+    const rules = prefixedRules
+        .map(([key, value]) => `${kebabifyStyleName(key)}:${transformValue(key, value)};`)
+        .join("");
 
     if (rules) {
         return `${selector}{${rules}}`;
diff --git a/src/util.js b/src/util.js
index ca5ab72..0e24817 100644
--- a/src/util.js
+++ b/src/util.js
@@ -215,11 +215,15 @@ function murmurhash2_32_gc(str) {
 export const hashObject = (object /* : ObjectMap */) /* : string */ => murmurhash2_32_gc(JSON.stringify(object));
 
 
-const IMPORTANT_RE = /^([^:]+:.*?)( !important)?;$/;
-
-// Given a single style rule string like "a: b;", adds !important to generate
-// "a: b !important;".
-export const importantify = (string /* : string */) /* : string */ =>
-    string.replace(
-        IMPORTANT_RE,
-        (_, base) => base + " !important;");
+// Given a single style value string like the "b" from "a: b;", adds !important
+// to generate "b !important".
+export const importantify = (string /* : string */) /* : string */ => (
+    // Bracket string character access is very fast, and in the default case we
+    // normally don't expect there to be "!important" at the end of the string
+    // so we can use this simple check to take an optimized path. If there
+    // happens to be a "!" in this position, we follow up with a more thorough
+    // check.
+    (string[string.length - 10] === '!' && string.slice(-11) === ' !important')
+        ? string
+        : `${string} !important`
+);