Refactor Attribute Processing (Step 3)

Summary: Decouple processStyle from the main reconciliation. It is now a process
extension to the style attribute `transform`. This effectively decouples a
large portion of special cases and helper dependencies from the reconciler.

The transform attribute becomes translated into the transformMatrix attribute on
the native side so this becomes a little weird in that I have to special case
it. I don't think it is worth while having a general solution for this so I
intend to rename the native attribute to `transform` and just have it accept the
resolved transform. Then I can remove the special cases.

The next step is generalizing the flattenStyle function and optimizing it.

@​public

Reviewed By: @vjeux

Differential Revision: D2460465

fb-gh-sync-id: 243e7fd77d282b401bc2c028aec8d57f24522a8e

Libraries/ReactNative/ReactNativeAttributePayload.js

@@ -11,10 +11,11 @@
  */
 '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;
@@ -29,6 +30,21 @@ 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
@@ -55,17 +71,9 @@ function diffNestedProperty(
   }
 
   // TODO: Walk both props in parallel instead of flattening.
-  // TODO: Move precomputeStyle to .process for each attribute.
 
-  var previousFlattenedStyle = precomputeStyle(
-    flattenStyle(prevProp),
-    validAttributes
-  );
-
-  var nextFlattenedStyle = precomputeStyle(
-    flattenStyle(nextProp),
-    validAttributes
-  );
+  var previousFlattenedStyle = flattenStyle(prevProp);
+  var nextFlattenedStyle = flattenStyle(nextProp);
 
   if (!previousFlattenedStyle || !nextFlattenedStyle) {
     if (nextFlattenedStyle) {
@@ -144,7 +152,14 @@ function diffProperties(
     if (!attributeConfig) {
       continue; // not a valid native prop
     }
-    if (updatePayload && updatePayload[propKey] !== undefined) {
+
+    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 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;
@@ -172,7 +187,7 @@ function diffProperties(
       // case: !Object is the default case
       if (defaultDiffer(prevProp, nextProp)) {
         // a normal leaf has changed
-        (updatePayload || (updatePayload = {}))[propKey] = nextProp;
+        (updatePayload || (updatePayload = {}))[altKey] = nextProp;
       }
     } else if (typeof attributeConfig.diff === 'function' ||
                typeof attributeConfig.process === 'function') {
@@ -186,7 +201,7 @@ function diffProperties(
         var nextValue = typeof attributeConfig.process === 'function' ?
                         attributeConfig.process(nextProp) :
                         nextProp;
-        (updatePayload || (updatePayload = {}))[propKey] = nextValue;
+        (updatePayload || (updatePayload = {}))[altKey] = nextValue;
       }
     } else {
       // default: fallthrough case when nested properties are defined
@@ -210,6 +225,7 @@ function diffProperties(
     if (!attributeConfig) {
       continue; // not a valid native prop
     }
+
     prevProp = prevProps[propKey];
     if (prevProp === undefined) {
       continue; // was already empty anyway
@@ -218,9 +234,10 @@ 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 = {}))[propKey] = null;
+      (updatePayload || (updatePayload = {}))[translateKey(propKey)] = null;
     } else {
       // default:
       // This is a nested attribute configuration where all the properties

Libraries/ReactNative/__tests__/ReactNativeAttributePayload-test.js

@@ -4,11 +4,13 @@
 'use strict';
 
 jest.dontMock('ReactNativeAttributePayload');
+jest.dontMock('StyleSheetRegistry');
 jest.dontMock('deepDiffer');
-jest.dontMock('styleDiffer');
-jest.dontMock('precomputeStyle');
 jest.dontMock('flattenStyle');
+jest.dontMock('styleDiffer');
+
 var ReactNativeAttributePayload = require('ReactNativeAttributePayload');
+var StyleSheetRegistry = require('StyleSheetRegistry');
 
 var diff = ReactNativeAttributePayload.diff;
 
@@ -122,6 +124,94 @@ 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