[fix] setNativeProps inline styles

Inline styles are preserved when using 'setNativeProps'. Adds unit tests
for the resolution logic required by 'setNativeProps'/'resolveStateful'
in a DOM context.

Fix #439

performance/webpack.config.js

@@ -49,7 +49,7 @@ module.exports = {
   ],
   resolve: {
     alias: {
-      'react-native': path.join(__dirname, '../src/module')
+      'react-native': path.join(__dirname, '../src')
     }
   }
 };

src/apis/StyleSheet/StyleRegistry.js

@@ -11,6 +11,8 @@ import prefixInlineStyles from './prefixInlineStyles';
 import ReactNativePropRegistry from '../../modules/ReactNativePropRegistry';
 import StyleManager from './StyleManager';
 
+const vendorPrefixPattern = /^(webkit|moz|ms)/;
+
 const createCacheKey = id => {
   const prefix = I18nManager.isRTL ? 'rtl' : 'ltr';
   return `${prefix}-${id}`;
@@ -81,36 +83,59 @@ class StyleRegistry {
 
   /**
    * Resolves a React Native style object to DOM attributes, accounting for
-   * the existing styles applied to the DOM node
+   * the existing styles applied to the DOM node.
+   *
+   * To determine the next style, some of the existing DOM state must be
+   * converted back into React Native styles.
    */
-  resolveStateful(reactNativeStyle, domClassList) {
-    const previousReactNativeStyle = {};
-    const preservedClassNames = [];
+  resolveStateful(rnStyleNext, domStyleProps) {
+    // Convert the DOM classList back into a React Native form
+    // Preserves unrecognized class names.
+    const rnStyleProps = domStyleProps.classList.reduce(
+      (styleProps, className) => {
+        const { prop, value } = this.styleManager.getDeclaration(className);
+        if (prop) {
+          styleProps.style[prop] = value;
+        } else {
+          styleProps.classList.push(className);
+        }
+        return styleProps;
+      },
+      { classList: [], style: {} }
+    );
 
-    // Convert the existing classList to a React Native style and preserve any
-    // unrecognized classNames.
-    domClassList.forEach(className => {
-      const { prop, value } = this.styleManager.getDeclaration(className);
-      if (prop) {
-        previousReactNativeStyle[prop] = value;
-      } else {
-        preservedClassNames.push(className);
+    // DOM style may include vendor prefixes and properties set by other libraries.
+    // Preserve it but transform back into React DOM style.
+    const rdomStyle = Object.keys(domStyleProps.style).reduce((acc, styleName) => {
+      const value = domStyleProps.style[styleName];
+      if (value !== '') {
+        const reactStyleName = vendorPrefixPattern.test(styleName)
+          ? styleName.charAt(0).toUpperCase() + styleName.slice(1)
+          : styleName;
+        acc[reactStyleName] = value;
+      }
+      return acc;
+    }, {});
+
+    // Create next DOM style props from current and next RN styles
+    const { classList: rdomClassListNext, style: rdomStyleNext } = this.resolve([
+      rnStyleProps.style,
+      rnStyleNext
+    ]);
+    // Add the current class names not managed by React Native
+    rdomClassListNext.push(...rnStyleProps.classList);
+    // Next class names take priority over current inline styles
+    const style = { ...rdomStyle };
+    rdomClassListNext.forEach(className => {
+      const { prop } = this.styleManager.getDeclaration(className);
+      if (style[prop]) {
+        style[prop] = '';
       }
     });
+    // Next inline styles take priority over current inline styles
+    Object.assign(style, rdomStyleNext);
+    const className = classListToString(rdomClassListNext);
 
-    // Resolve the two React Native styles.
-    const { classList, style = {} } = this.resolve([previousReactNativeStyle, reactNativeStyle]);
-
-    // Because this is used in stateful operations we need to remove any
-    // existing inline styles that would override the classNames.
-    classList.forEach(className => {
-      const { prop } = this.styleManager.getDeclaration(className);
-      style[prop] = null;
-    });
-
-    classList.push(preservedClassNames);
-
-    const className = classListToString(classList);
     return { className, style };
   }
 

src/apis/StyleSheet/__tests__/StyleRegistry-test.js

@@ -58,19 +58,37 @@ describe('apis/StyleSheet/StyleRegistry', () => {
     });
   });
 
-  test('resolveStateful', () => {
-    // generate a classList to act as pre-existing DOM state
-    const mockStyle = styleRegistry.register({
-      borderWidth: 0,
-      borderColor: 'red',
-      width: 100
+  describe('resolveStateful', () => {
+    test('preserves unrelated class names', () => {
+      const domStyleProps = { classList: ['unknown-class-1', 'unknown-class-2'], style: {} };
+      const domStyleNextProps = styleRegistry.resolveStateful({}, domStyleProps);
+      expect(domStyleNextProps).toMatchSnapshot();
     });
-    const { classList: domClassList } = styleRegistry.resolve(mockStyle);
-    domClassList.unshift('external-className');
-    expect(domClassList).toMatchSnapshot();
 
-    // test result
-    const result = styleRegistry.resolveStateful({ borderWidth: 1, opacity: 1 }, domClassList);
-    expect(result).toMatchSnapshot();
+    test('preserves unrelated inline styles', () => {
+      const domStyleProps = { classList: [], style: { fontSize: '20px' } };
+      const domStyleNextProps = styleRegistry.resolveStateful({ opacity: 1 }, domStyleProps);
+      expect(domStyleNextProps).toMatchSnapshot();
+    });
+
+    test('next class names have priority over current inline styles', () => {
+      const domStyleProps = { classList: [], style: { opacity: 0.5 } };
+      const nextStyle = styleRegistry.register({ opacity: 1 });
+      const domStyleNextProps = styleRegistry.resolveStateful(nextStyle, domStyleProps);
+      expect(domStyleNextProps).toMatchSnapshot();
+    });
+
+    test('next inline styles have priority over current inline styles', () => {
+      // note: this also checks for correctly uppercasing the first letter of DOM vendor prefixes
+      const domStyleProps = {
+        classList: [],
+        style: { opacity: 0.5, webkitTransform: 'scale(1)', transform: 'scale(1)' }
+      };
+      const domStyleNextProps = styleRegistry.resolveStateful(
+        { opacity: 1, transform: [{ scale: 2 }] },
+        domStyleProps
+      );
+      expect(domStyleNextProps).toMatchSnapshot();
+    });
   });
 });

src/apis/StyleSheet/__tests__/__snapshots__/StyleRegistry-test.js.snap

@@ -192,35 +192,39 @@ Object {
 }
 `;
 
-exports[`apis/StyleSheet/StyleRegistry resolveStateful 1`] = `
-Array [
-  "external-className",
-  "rn-borderTopColor-1gxhl28",
-  "rn-borderRightColor-knoah9",
-  "rn-borderBottomColor-1ani3fp",
-  "rn-borderLeftColor-ribj9x",
-  "rn-borderTopWidth-13yce4e",
-  "rn-borderRightWidth-fnigne",
-  "rn-borderBottomWidth-ndvcnb",
-  "rn-borderLeftWidth-gxnn5r",
-  "rn-width-b8lwoo",
-]
-`;
-
-exports[`apis/StyleSheet/StyleRegistry resolveStateful 2`] = `
+exports[`apis/StyleSheet/StyleRegistry resolveStateful next class names have priority over current inline styles 1`] = `
 Object {
-  "className": "rn-borderBottomColor-1ani3fp rn-borderBottomWidth-ndvcnb rn-borderLeftColor-ribj9x rn-borderLeftWidth-gxnn5r rn-borderRightColor-knoah9 rn-borderRightWidth-fnigne rn-borderTopColor-1gxhl28 rn-borderTopWidth-13yce4e rn-width-b8lwoo external-className",
+  "className": "rn-opacity-6dt33c",
   "style": Object {
-    "borderBottomColor": null,
-    "borderBottomWidth": null,
-    "borderLeftColor": null,
-    "borderLeftWidth": null,
-    "borderRightColor": null,
-    "borderRightWidth": null,
-    "borderTopColor": null,
-    "borderTopWidth": null,
-    "opacity": 1,
-    "width": null,
+    "opacity": "",
+  },
+}
+`;
+
+exports[`apis/StyleSheet/StyleRegistry resolveStateful next inline styles have priority over current inline styles 1`] = `
+Object {
+  "className": "",
+  "style": Object {
+    "WebkitTransform": "scale(2)",
+    "opacity": 1,
+    "transform": "scale(2)",
+  },
+}
+`;
+
+exports[`apis/StyleSheet/StyleRegistry resolveStateful preserves unrelated class names 1`] = `
+Object {
+  "className": "unknown-class-1 unknown-class-2",
+  "style": Object {},
+}
+`;
+
+exports[`apis/StyleSheet/StyleRegistry resolveStateful preserves unrelated inline styles 1`] = `
+Object {
+  "className": "",
+  "style": Object {
+    "fontSize": "20px",
+    "opacity": 1,
   },
 }
 `;

src/modules/NativeMethodsMixin/index.js

@@ -67,12 +67,15 @@ const NativeMethodsMixin = {
    * the initial styles from the DOM node and merge them with incoming props.
    */
   setNativeProps(nativeProps: Object) {
-    // DOM state
+    // Copy of existing DOM state
     const node = findNodeHandle(this);
     const classList = Array.prototype.slice.call(node.classList);
+    const style = { ...node.style };
+    const domStyleProps = { classList, style };
 
+    // Next DOM state
     const domProps = createDOMProps(nativeProps, style =>
-      StyleRegistry.resolveStateful(style, classList)
+      StyleRegistry.resolveStateful(style, domStyleProps)
     );
     UIManager.updateView(node, domProps, this);
   }