Allow specifying styles as Maps to guarantee ordering

Summary: Key ordering in objects can be different in different environments.
Sometimes, this causes problems, like where styles generated on the server are
not in the same order as the same styles generated on the client. This
manifests in problems such as #199

This change lets users manually fix instances where the ordering of elements
changes by specifying their styles in an ES6 `Map`, which has defined value
ordering.

In order to accomplish this, an `OrderedElements` class was created, which is
sorta like a `Map` but can only store string keys and lacks most of the
features. Internally, `Map`s and objects are converted into this and then these
are merged together to preserve the ordering.

Fixes #199

Test Plan:
 - `npm test`

@lencioni @ljharb

package.json

@@ -44,6 +44,7 @@
     "babel-core": "^5.8.25",
     "babel-loader": "^5.3.2",
     "chai": "^3.3.0",
+    "core-js": "^2.4.1",
     "eslint": "^3.7.1",
     "eslint-config-standard-react": "^4.2.0",
     "eslint-plugin-react": "^6.3.0",

src/generate.js

@@ -1,6 +1,7 @@
 /* @flow */
 import prefixAll from 'inline-style-prefixer/static';
 
+import OrderedElements from './ordered-elements';
 import {
     objectToPairs, kebabifyStyleName, recursiveMerge, stringifyValue,
     importantify, flatten
@@ -148,18 +149,20 @@ export const generateCSS = (
     stringHandlers /* : StringHandlers */ = {},
     useImportant /* : boolean */ = true
 ) /* : string */ => {
-    const merged = styleTypes.reduce(recursiveMerge);
+    const merged /* : OrderedElements */ = styleTypes.reduce(
+        recursiveMerge,
+        new OrderedElements());
 
-    const plainDeclarations = {};
+    const plainDeclarations = new OrderedElements();
     let generatedStyles = "";
 
-    Object.keys(merged).forEach(key => {
+    merged.forEach((key, val) => {
         // For each key, see if one of the selector handlers will handle these
         // styles.
         const foundHandler = selectorHandlers.some(handler => {
             const result = handler(key, selector, (newSelector) => {
                 return generateCSS(
-                    newSelector, [merged[key]], selectorHandlers,
+                    newSelector, [val], selectorHandlers,
                     stringHandlers, useImportant);
             });
             if (result != null) {
@@ -172,7 +175,7 @@ export const generateCSS = (
         // If none of the handlers handled it, add it to the list of plain
         // style declarations.
         if (!foundHandler) {
-            plainDeclarations[key] = merged[key];
+            plainDeclarations.set(key, val);
         }
     });
 
@@ -191,13 +194,11 @@ export const generateCSS = (
  * See generateCSSRuleset for usage and documentation of paramater types.
  */
 const runStringHandlers = (
-    declarations /* : SheetDefinition */,
+    declarations /* : OrderedElements */,
     stringHandlers /* : StringHandlers */,
     selectorHandlers /* : SelectorHandler[] */
 ) /* */ => {
-    const result = {};
-
-    Object.keys(declarations).forEach(key => {
+    return declarations.map((key, val) => {
         // If a handler exists for this particular key, let it interpret
         // that value first before continuing
         if (stringHandlers && stringHandlers.hasOwnProperty(key)) {
@@ -206,14 +207,11 @@ const runStringHandlers = (
             // `selectorHandlers` and have them make calls to `generateCSS`
             // themselves. Right now, this is impractical because our string
             // handlers are very specialized and do complex things.
-            result[key] = stringHandlers[key](
-                declarations[key], selectorHandlers);
+            return stringHandlers[key](val, selectorHandlers);
         } else {
-            result[key] = declarations[key];
+            return val;
         }
     });
-
-    return result;
 };
 
 /**
@@ -249,44 +247,33 @@ const runStringHandlers = (
  */
 export const generateCSSRuleset = (
     selector /* : string */,
-    declarations /* : SheetDefinition */,
+    declarations /* : OrderedElements */,
     stringHandlers /* : StringHandlers */,
     useImportant /* : boolean */,
     selectorHandlers /* : SelectorHandler[] */
 ) /* : string */ => {
-    const handledDeclarations = runStringHandlers(
+    const handledDeclarations /* : OrderedElements */ = runStringHandlers(
         declarations, stringHandlers, selectorHandlers);
 
-    const prefixedDeclarations = prefixAll(handledDeclarations);
+    const prefixedDeclarations = prefixAll(handledDeclarations.elements);
 
     const prefixedRules = flatten(
         objectToPairs(prefixedDeclarations).map(([key, value]) => {
             if (Array.isArray(value)) {
                 // inline-style-prefix-all returns an array when there should be
                 // multiple rules, we will flatten to single rules
-
-                const prefixedValues = [];
-                const unprefixedValues = [];
-
-                value.forEach(v => {
-                  if (v.indexOf('-') === 0) {
-                    prefixedValues.push(v);
-                  } else {
-                    unprefixedValues.push(v);
-                  }
-                });
-
-                prefixedValues.sort();
-                unprefixedValues.sort();
-
-                return prefixedValues
-                  .concat(unprefixedValues)
-                  .map(v => [key, v]);
+                return value.map(v => [key, v]);
             }
             return [[key, value]];
         })
     );
 
+    prefixedRules.sort((a, b) => {
+        const aIndex = handledDeclarations.keyOrder.indexOf(a);
+        const bIndex = handledDeclarations.keyOrder.indexOf(b);
+        return aIndex - bIndex;
+    });
+
     const rules = prefixedRules.map(([key, value]) => {
         const stringValue = stringifyValue(key, value);
         const ret = `${kebabifyStyleName(key)}:${stringValue};`;

src/ordered-elements.js

@@ -0,0 +1,77 @@
+/* @flow */
+/* global Map */
+
+export default class OrderedElements {
+    /* ::
+    elements: {[string]: any};
+    keyOrder: string[];
+
+    static fromObject: ({[string]: any}) => OrderedElements;
+    static fromMap: (Map<string,any>) => OrderedElements;
+    static from: (Map<string,any> | {[string]: any} | OrderedElements) =>
+        OrderedElements;
+    */
+
+    constructor(
+        elements /* : {[string]: any} */ = {},
+        keyOrder /* : string[] */ = []
+    ) {
+        this.elements = elements;
+        this.keyOrder = keyOrder;
+    }
+
+    forEach(callback /* : (string, any) => void */) {
+        this.keyOrder.forEach(key => callback(key, this.elements[key]));
+    }
+
+    map(callback /* : (string, any) => any */) /* : OrderedElements */ {
+        const ret = new OrderedElements();
+        this.forEach((key, value) => {
+            ret.set(key, callback(key, value));
+        });
+        return ret;
+    }
+
+    set(key /* : string */, value /* : any */) {
+        if (!this.elements.hasOwnProperty(key)) {
+            this.keyOrder.push(key);
+        }
+        this.elements[key] = value;
+    }
+
+    get(key /* : string */) /* : any */ {
+        return this.elements[key];
+    }
+
+    has(key /* : string */) /* : boolean */ {
+        return this.elements.hasOwnProperty(key);
+    }
+}
+
+OrderedElements.fromObject = (obj) => {
+    return new OrderedElements(obj, Object.keys(obj));
+};
+
+OrderedElements.fromMap = (map) => {
+    const ret = new OrderedElements();
+    map.forEach((val, key) => {
+        ret.set(key, val);
+    });
+    return ret;
+};
+
+OrderedElements.from = (obj) => {
+    if (obj instanceof OrderedElements) {
+        return obj;
+    } else if (
+        // For some reason, flow complains about a plain
+        // `typeof Map !== "undefined"` check. Casting `Map` to `any` solves
+        // the problem.
+        typeof /*::(*/ Map /*: any)*/ !== "undefined" &&
+        obj instanceof Map
+    ) {
+        return OrderedElements.fromMap(obj);
+    } else {
+        return OrderedElements.fromObject(obj);
+    }
+};

src/util.js

@@ -1,4 +1,7 @@
 /* @flow */
+/* global Map */
+
+import OrderedElements from './ordered-elements';
 
 /* ::
 type Pair = [ string, any ];
@@ -37,26 +40,43 @@ const MS_RE = /^ms-/;
 const kebabify = (string /* : string */) /* : string */ => string.replace(UPPERCASE_RE, '-$1').toLowerCase();
 export const kebabifyStyleName = (string /* : string */) /* : string */ => kebabify(string).replace(MS_RE, '-ms-');
 
+const hasMap = typeof Map !== "undefined";
+const isMap = (x /* : any */) /* : boolean */ => hasMap && x instanceof Map;
+
 export const recursiveMerge = (
-    a /* : ObjectMap | any */,
+    a /* : OrderedElements | ObjectMap | any */,
     b /* : ObjectMap */
-) /* : ObjectMap */ => {
+) /* : OrderedElements */ => {
     // TODO(jlfwong): Handle malformed input where a and b are not the same
     // type.
 
     if (typeof a !== 'object') {
-        return b;
+        if (typeof b === 'object' && !Array.isArray(b)) {
+            return OrderedElements.from(b);
+        } else {
+            return b;
+        }
     }
 
-    const ret = {...a};
+    const ret = OrderedElements.from(a);
 
-    Object.keys(b).forEach(key => {
-        if (ret.hasOwnProperty(key)) {
-            ret[key] = recursiveMerge(a[key], b[key]);
-        } else {
-            ret[key] = b[key];
-        }
-    });
+    if (isMap(b)) {
+        b.forEach((val, key) => {
+            if (ret.has(key)) {
+                ret.set(key, recursiveMerge(ret.get(key), val));
+            } else {
+                ret.set(key, val)
+            }
+        });
+    } else {
+        Object.keys(b).forEach(key => {
+            if (ret.has(key)) {
+                ret.set(key, recursiveMerge(ret.get(key), b[key]));
+            } else {
+                ret.set(key, b[key])
+            }
+        });
+    }
 
     return ret;
 };

tests/generate_test.js

@@ -1,12 +1,15 @@
 import {assert} from 'chai';
 
+import OrderedElements from '../src/ordered-elements';
 import {
     generateCSSRuleset, generateCSS, defaultSelectorHandlers
 } from '../src/generate';
 
 describe('generateCSSRuleset', () => {
     const assertCSSRuleset = (selector, declarations, expected) => {
-        const actual = generateCSSRuleset(selector, declarations);
+        const actual = generateCSSRuleset(
+            selector,
+            OrderedElements.from(declarations));
         assert.equal(actual, expected.split('\n').map(x => x.trim()).join(''));
     };
     it('returns a CSS string for a single property', () => {
@@ -153,8 +156,14 @@ describe('generateCSS', () => {
     it('adds browser prefixes', () => {
         assertCSS('.foo', [{
             display: 'flex',
-        }], '.foo{display:-moz-box !important;display:-ms-flexbox !important;display:-webkit-box !important;display:-webkit-flex !important;display:flex !important;}',
-            defaultSelectorHandlers);
+        }], '.foo{' +
+            'display:-webkit-box !important;' +
+            'display:-moz-box !important;' +
+            'display:-ms-flexbox !important;' +
+            'display:-webkit-flex !important;' +
+            'display:flex !important;' +
+        '}',
+        defaultSelectorHandlers);
     });
 
     it('supports other selector handlers', () => {