Revert [React Native] Refactor Attribute Processing (Step 3)

Reviewed By: @kmagiera, @mikearmstrong001

Differential Revision: D2512548

fb-gh-sync-id: bcae3a8c85bfcfa468f2128288a25f406d78cf08

Libraries/Components/View/ReactNativeStyleAttributes.js

@@ -19,7 +19,6 @@ var ViewStylePropTypes = require('ViewStylePropTypes');
 var keyMirror = require('keyMirror');
 var matricesDiffer = require('matricesDiffer');
 var processColor = require('processColor');
-var processTransform = require('processTransform');
 var sizesDiffer = require('sizesDiffer');
 
 var ReactNativeStyleAttributes = {
@@ -28,7 +27,6 @@ var ReactNativeStyleAttributes = {
   ...keyMirror(ImageStylePropTypes),
 };
 
-ReactNativeStyleAttributes.transform = { process: processTransform };
 ReactNativeStyleAttributes.transformMatrix = { diff: matricesDiffer };
 ReactNativeStyleAttributes.shadowOffset = { diff: sizesDiffer };
 

Libraries/ReactIOS/NativeMethodsMixin.js

@@ -17,7 +17,10 @@ var ReactNativeAttributePayload = require('ReactNativeAttributePayload');
 var TextInputState = require('TextInputState');
 
 var findNodeHandle = require('findNodeHandle');
+var flattenStyle = require('flattenStyle');
 var invariant = require('invariant');
+var mergeFast = require('mergeFast');
+var precomputeStyle = require('precomputeStyle');
 
 type MeasureOnSuccessCallback = (
   x: number,

Libraries/ReactNative/ReactNativeAttributePayload.js

@@ -11,11 +11,10 @@
  */
 'use strict';
 
-var Platform = require('Platform');
-
 var deepDiffer = require('deepDiffer');
 var styleDiffer = require('styleDiffer');
 var flattenStyle = require('flattenStyle');
+var precomputeStyle = require('precomputeStyle');
 
 type AttributeDiffer = (prevProp : mixed, nextProp : mixed) => boolean;
 type AttributePreprocessor = (nextProp: mixed) => mixed;
@@ -30,21 +29,6 @@ type AttributeConfiguration =
     CustomAttributeConfiguration | AttributeConfiguration /*| boolean*/
   ) };
 
-function translateKey(propKey : string) : string {
-  if (propKey === 'transform') {
-    // We currently special case the key for `transform`. iOS uses the
-    // transformMatrix name and Android uses the decomposedMatrix name.
-    // TODO: We could unify these names and just use the name `transform`
-    // all the time. Just need to update the native side.
-    if (Platform.OS === 'android') {
-      return 'decomposedMatrix';
-    } else {
-      return 'transformMatrix';
-    }
-  }
-  return propKey;
-}
-
 function defaultDiffer(prevProp: mixed, nextProp: mixed) : boolean {
   if (typeof nextProp !== 'object' || nextProp === null) {
     // Scalars have already been checked for equality
@@ -71,9 +55,17 @@ function diffNestedProperty(
   }
 
   // TODO: Walk both props in parallel instead of flattening.
+  // TODO: Move precomputeStyle to .process for each attribute.
 
-  var previousFlattenedStyle = flattenStyle(prevProp);
-  var nextFlattenedStyle = flattenStyle(nextProp);
+  var previousFlattenedStyle = precomputeStyle(
+    flattenStyle(prevProp),
+    validAttributes
+  );
+
+  var nextFlattenedStyle = precomputeStyle(
+    flattenStyle(nextProp),
+    validAttributes
+  );
 
   if (!previousFlattenedStyle || !nextFlattenedStyle) {
     if (nextFlattenedStyle) {
@@ -152,14 +144,7 @@ function diffProperties(
     if (!attributeConfig) {
       continue; // not a valid native prop
     }
-
-    var altKey = translateKey(propKey);
-    if (!attributeConfig[altKey]) {
-      // If there is no config for the alternative, bail out. Helps ART.
-      altKey = propKey;
-    }
-
-    if (updatePayload && updatePayload[altKey] !== undefined) {
+    if (updatePayload && updatePayload[propKey] !== undefined) {
       // If we're in a nested attribute set, we may have set this property
       // already. If so, bail out. The previous update is what counts.
       continue;
@@ -187,7 +172,7 @@ function diffProperties(
       // case: !Object is the default case
       if (defaultDiffer(prevProp, nextProp)) {
         // a normal leaf has changed
-        (updatePayload || (updatePayload = {}))[altKey] = nextProp;
+        (updatePayload || (updatePayload = {}))[propKey] = nextProp;
       }
     } else if (typeof attributeConfig.diff === 'function' ||
                typeof attributeConfig.process === 'function') {
@@ -201,7 +186,7 @@ function diffProperties(
         var nextValue = typeof attributeConfig.process === 'function' ?
                         attributeConfig.process(nextProp) :
                         nextProp;
-        (updatePayload || (updatePayload = {}))[altKey] = nextValue;
+        (updatePayload || (updatePayload = {}))[propKey] = nextValue;
       }
     } else {
       // default: fallthrough case when nested properties are defined
@@ -225,7 +210,6 @@ function diffProperties(
     if (!attributeConfig) {
       continue; // not a valid native prop
     }
-
     prevProp = prevProps[propKey];
     if (prevProp === undefined) {
       continue; // was already empty anyway
@@ -234,10 +218,9 @@ function diffProperties(
     if (typeof attributeConfig !== 'object' ||
         typeof attributeConfig.diff === 'function' ||
         typeof attributeConfig.process === 'function') {
-
       // case: CustomAttributeConfiguration | !Object
       // Flag the leaf property for removal by sending a sentinel.
-      (updatePayload || (updatePayload = {}))[translateKey(propKey)] = null;
+      (updatePayload || (updatePayload = {}))[propKey] = null;
     } else {
       // default:
       // This is a nested attribute configuration where all the properties

Libraries/ReactNative/__tests__/ReactNativeAttributePayload-test.js

@@ -4,13 +4,11 @@
 'use strict';
 
 jest.dontMock('ReactNativeAttributePayload');
-jest.dontMock('StyleSheetRegistry');
 jest.dontMock('deepDiffer');
-jest.dontMock('flattenStyle');
 jest.dontMock('styleDiffer');
-
+jest.dontMock('precomputeStyle');
+jest.dontMock('flattenStyle');
 var ReactNativeAttributePayload = require('ReactNativeAttributePayload');
-var StyleSheetRegistry = require('StyleSheetRegistry');
 
 var diff = ReactNativeAttributePayload.diff;
 
@@ -124,94 +122,6 @@ describe('ReactNativeAttributePayload', function() {
     )).toEqual(null);
   });
 
-  it('should flatten nested styles and predefined styles', () => {
-    var validStyleAttribute = { someStyle: { foo: true, bar: true } };
-
-    expect(diff(
-      {},
-      { someStyle: [{ foo: 1 }, { bar: 2 }]},
-      validStyleAttribute
-    )).toEqual({ foo: 1, bar: 2 });
-
-    expect(diff(
-      { someStyle: [{ foo: 1 }, { bar: 2 }]},
-      {},
-      validStyleAttribute
-    )).toEqual({ foo: null, bar: null });
-
-    var barStyle = StyleSheetRegistry.registerStyle({
-      bar: 3,
-    });
-
-    expect(diff(
-      {},
-      { someStyle: [[{ foo: 1 }, { foo: 2 }], barStyle]},
-      validStyleAttribute
-    )).toEqual({ foo: 2, bar: 3 });
-  });
-
-  it('should reset a value to a previous if it is removed', () => {
-    var validStyleAttribute = { someStyle: { foo: true, bar: true } };
-
-    expect(diff(
-      { someStyle: [{ foo: 1 }, { foo: 3 }]},
-      { someStyle: [{ foo: 1 }, { bar: 2 }]},
-      validStyleAttribute
-    )).toEqual({ foo: 1, bar: 2 });
-  });
-
-  it('should not clear removed props if they are still in another slot', () => {
-    var validStyleAttribute = { someStyle: { foo: true, bar: true } };
-
-    expect(diff(
-      { someStyle: [{}, { foo: 3, bar: 2 }]},
-      { someStyle: [{ foo: 3 }, { bar: 2 }]},
-      validStyleAttribute
-    )).toEqual(null);
-
-    expect(diff(
-      { someStyle: [{}, { foo: 3, bar: 2 }]},
-      { someStyle: [{ foo: 1, bar: 1 }, { bar: 2 }]},
-      validStyleAttribute
-    )).toEqual({ foo: 1 });
-  });
-
-  it('should clear a prop if a later style is explicit null/undefined', () => {
-    var validStyleAttribute = { someStyle: { foo: true, bar: true } };
-    expect(diff(
-      { someStyle: [{}, { foo: 3, bar: 2 }]},
-      { someStyle: [{ foo: 1 }, { bar: 2, foo: null }]},
-      validStyleAttribute
-    )).toEqual({ foo: null });
-
-    expect(diff(
-      { someStyle: [{ foo: 3 }, { foo: null, bar: 2 }]},
-      { someStyle: [{ foo: null }, { bar: 2 }]},
-      validStyleAttribute
-    )).toEqual(null);
-
-    expect(diff(
-      { someStyle: [{ foo: 1 }, { foo: null }]},
-      { someStyle: [{ foo: 2 }, { foo: null }]},
-      validStyleAttribute
-    )).toEqual(null);
-
-    // Test the same case with object equality because an early bailout doesn't
-    // work in this case.
-    var fooObj = { foo: 3 };
-    expect(diff(
-      { someStyle: [{ foo: 1 }, fooObj]},
-      { someStyle: [{ foo: 2 }, fooObj]},
-      validStyleAttribute
-    )).toEqual(null);
-
-    expect(diff(
-      { someStyle: [{ foo: 1 }, { foo: 3 }]},
-      { someStyle: [{ foo: 2 }, { foo: undefined }]},
-      validStyleAttribute
-    )).toEqual({ foo: null });
-  });
-
   // Function properties are just markers to native that events should be sent.
   it('should convert functions to booleans', () => {
     // Note that if the property changes from one function to another, we don't

Libraries/StyleSheet/TransformPropTypes.js

@@ -13,22 +13,6 @@
 
 var ReactPropTypes = require('ReactPropTypes');
 
-var ArrayOfNumberPropType = ReactPropTypes.arrayOf(ReactPropTypes.number);
-
-var TransformMatrixPropType = function(
-  props : Object,
-  propName : string,
-  componentName : string
-) : ?Error {
-  if (props.transform && props.transformMatrix) {
-    return new Error(
-      'transformMatrix and transform styles cannot be used on the same ' +
-      'component'
-    );
-  }
-  return ArrayOfNumberPropType(props, propName, componentName);
-};
-
 var TransformPropTypes = {
   transform: ReactPropTypes.arrayOf(
     ReactPropTypes.oneOfType([
@@ -46,7 +30,7 @@ var TransformPropTypes = {
       ReactPropTypes.shape({skewY: ReactPropTypes.string})
     ])
   ),
-  transformMatrix: TransformMatrixPropType,
+  transformMatrix: ReactPropTypes.arrayOf(ReactPropTypes.number),
 };
 
 module.exports = TransformPropTypes;

Libraries/StyleSheet/precomputeStyle.js

@@ -6,17 +6,74 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  *
- * @providesModule processTransform
+ * @providesModule precomputeStyle
  * @flow
  */
 'use strict';
 
 var MatrixMath = require('MatrixMath');
+var ReactNativeStyleAttributes = require('ReactNativeStyleAttributes');
 var Platform = require('Platform');
 
+var deepFreezeAndThrowOnMutationInDev = require('deepFreezeAndThrowOnMutationInDev');
 var invariant = require('invariant');
 var stringifySafe = require('stringifySafe');
 
+/**
+ * This method provides a hook where flattened styles may be precomputed or
+ * otherwise prepared to become better input data for native code.
+ */
+function precomputeStyle(style: ?Object, validAttributes: Object): ?Object {
+  if (!style) {
+    return style;
+  }
+
+  var hasPreprocessKeys = false;
+  for (var i = 0, keys = Object.keys(style); i < keys.length; i++) {
+    var key = keys[i];
+    if (_processor(key, validAttributes)) {
+      hasPreprocessKeys = true;
+      break;
+    }
+  }
+
+  if (!hasPreprocessKeys && !style.transform) {
+    return style;
+  }
+
+  var newStyle = {...style};
+  for (var i = 0, keys = Object.keys(style); i < keys.length; i++) {
+    var key = keys[i];
+    var process = _processor(key, validAttributes);
+    if (process) {
+      newStyle[key] = process(newStyle[key]);
+    }
+  }
+
+  if (style.transform) {
+    invariant(
+      !style.transformMatrix,
+      'transformMatrix and transform styles cannot be used on the same component'
+    );
+
+    newStyle = _precomputeTransforms(newStyle);
+  }
+
+  deepFreezeAndThrowOnMutationInDev(newStyle);
+  return newStyle;
+}
+
+function _processor(key: string, validAttributes: Object) {
+  var process = validAttributes[key] && validAttributes[key].process;
+  if (!process) {
+    process =
+      ReactNativeStyleAttributes[key] &&
+      ReactNativeStyleAttributes[key].process;
+  }
+
+  return process;
+}
+
 /**
  * Generate a transform matrix based on the provided transforms, and use that
  * within the style object instead.
@@ -25,7 +82,8 @@ var stringifySafe = require('stringifySafe');
  * be applied in an arbitrary order, and yet have a universal, singular
  * interface to native code.
  */
-function processTransform(transform: Object): Object {
+function _precomputeTransforms(style: Object): Object {
+  var {transform} = style;
   var result = MatrixMath.createIdentityMatrix();
 
   transform.forEach(transformation => {
@@ -86,9 +144,16 @@ function processTransform(transform: Object): Object {
   // get applied in the specific order of (1) translate (2) scale (3) rotate.
   // Once we can directly apply a matrix, we can remove this decomposition.
   if (Platform.OS === 'android') {
-    return MatrixMath.decomposeMatrix(result);
+    return {
+      ...style,
+      transformMatrix: result,
+      decomposedMatrix: MatrixMath.decomposeMatrix(result),
+    };
   }
-  return result;
+  return {
+    ...style,
+    transformMatrix: result,
+  };
 }
 
 /**
@@ -175,4 +240,4 @@ function _validateTransform(key, value, transformation) {
   }
 }
 
-module.exports = processTransform;
+module.exports = precomputeStyle;