Preserve style order in prefixProperty

While attempting to update Aphrodite to inline-style-prefixer 3.0.0, I
ran into an issue with prefixAll putting the prefixes in the wrong
order. Specifically, they came after the un-prefixed style, which is not
what we want because we want the standard style to have precedence in
browsers that have it implemented.

  Khan/aphrodite#205

After a little bit of digging, I found that this was caused by the way
prefixProperty was designed. To ensure that the prefixed styles are
injected in the correct spot, we need to build a new object key-by-key
and return it instead of mutating the style object that was passed in.

This is likely to cause a performance hit if there are multiple styles
to be prefixed in the same object. It might be worth making a pass to
optimize this so all of the styles can be prefixed in one pass, but I'm
going to leave that for another time.

Although Object ordering is not guaranteed, it is generally sticky in
most browsers so this seems like an improvement but not a total fix.
This reliance on Object ordering will likely cause issues (different
style order) when used on the server on older versions of Node (e.g.
Node 4). There should probably be some effort similar to
Khan/aphrodite#200 to ensure that the order is
properly preserved throughout this package.

Author: lencioni

modules/static/createPrefixer.js

@@ -43,7 +43,7 @@ StaticData
           style[property] = processedValue
         }
 
-        prefixProperty(prefixMap, property, style)
+        style = prefixProperty(prefixMap, property, style)
       }
     }
 

modules/utils/prefixProperty.js

@@ -5,11 +5,27 @@ export default function prefixProperty(
   prefixProperties: Object,
   property: string,
   style: Object
-): void {
-  if (prefixProperties.hasOwnProperty(property)) {
-    const requiredPrefixes = prefixProperties[property]
-    for (let i = 0, len = requiredPrefixes.length; i < len; ++i) {
-      style[requiredPrefixes[i] + capitalizeString(property)] = style[property]
-    }
+): Object {
+  if (!prefixProperties.hasOwnProperty(property)) {
+    return style
   }
+
+  // We need to preserve the order of the styles while inserting new prefixed
+  // styles. Object order is not guaranteed, but this is better than nothing.
+  // Note that this is brittle and is likely to break in older versions of
+  // Node (e.g. Node 4).
+  const newStyle = {}
+  Object.keys(style).forEach((styleProperty) => {
+    if (styleProperty === property) {
+      // We've found the style we need to prefix.
+      const requiredPrefixes = prefixProperties[property]
+      for (let i = 0, len = requiredPrefixes.length; i < len; ++i) {
+        newStyle[requiredPrefixes[i] + capitalizeString(property)] = style[property]
+      }
+    }
+
+    newStyle[styleProperty] = style[styleProperty]
+  })
+
+  return newStyle
 }

test/dynamic/createPrefixer-test.js

@@ -386,5 +386,33 @@ describe('Dynamic Prefixer', () => {
       }
       expect(Prefixer.prefixAll(input)).to.eql(output)
     })
+
+    it('puts the prefixes in the correct order', () => {
+      const input = { userSelect: 'none' }
+      const order = [
+        'WebkitUserSelect',
+        'MozUserSelect',
+        'msUserSelect',
+        'userSelect'
+      ]
+      expect(Object.keys(Prefixer.prefixAll(input))).to.eql(order)
+    });
+
+    it('does not mess up the order of other styles', () => {
+      const input = {
+        color: 'red',
+        userSelect: 'none',
+        border: 0
+      }
+      const order = [
+        'color',
+        'WebkitUserSelect',
+        'MozUserSelect',
+        'msUserSelect',
+        'userSelect',
+        'border'
+      ]
+      expect(Object.keys(Prefixer.prefixAll(input))).to.eql(order)
+    });
   })
 })

test/static/createPrefixer-test.js

@@ -21,6 +21,34 @@ describe('Static Prefixer', () => {
       expect(prefixAll(input)).to.eql(output)
     })
 
+    it('puts the prefixes in the correct order', () => {
+      const input = { userSelect: 'none' }
+      const order = [
+        'WebkitUserSelect',
+        'MozUserSelect',
+        'msUserSelect',
+        'userSelect'
+      ]
+      expect(Object.keys(prefixAll(input))).to.eql(order)
+    });
+
+    it('does not mess up the order of other styles', () => {
+      const input = {
+        color: 'red',
+        userSelect: 'none',
+        border: 0
+      }
+      const order = [
+        'color',
+        'WebkitUserSelect',
+        'MozUserSelect',
+        'msUserSelect',
+        'userSelect',
+        'border'
+      ]
+      expect(Object.keys(prefixAll(input))).to.eql(order)
+    });
+
     it('should use dash-cased alternative values in array', () => {
       const input = { marginLeft: 'calc(30deg)' }
       const output = { marginLeft: ['-webkit-calc(30deg)', '-moz-calc(30deg)', 'calc(30deg)'] }