Skip to content

Commit

Permalink
Refactor Attribute Processing (Step 3)
Browse files Browse the repository at this point in the history
Reviewed By: @spicyj

Differential Revision: D2514193

fb-gh-sync-id: 8ba55014e8d052c69d8e1def327284ec974d6837
  • Loading branch information
sebmarkbage authored and facebook-github-bot-7 committed Oct 6, 2015
1 parent 2ea3b93 commit 6244fd0
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 91 deletions.
2 changes: 2 additions & 0 deletions Libraries/Components/View/ReactNativeStyleAttributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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 = {
Expand All @@ -27,6 +28,7 @@ var ReactNativeStyleAttributes = {
...keyMirror(ImageStylePropTypes),
};

ReactNativeStyleAttributes.transform = { process: processTransform };
ReactNativeStyleAttributes.transformMatrix = { diff: matricesDiffer };
ReactNativeStyleAttributes.shadowOffset = { diff: sizesDiffer };

Expand Down
3 changes: 0 additions & 3 deletions Libraries/ReactIOS/NativeMethodsMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ 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,
Expand Down
47 changes: 32 additions & 15 deletions Libraries/ReactNative/ReactNativeAttributePayload.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -144,7 +152,14 @@ function diffProperties(
if (!attributeConfig) {
continue; // not a valid native prop
}
if (updatePayload && updatePayload[propKey] !== undefined) {

var altKey = translateKey(propKey);
if (!validAttributes[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;
Expand Down Expand Up @@ -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') {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
18 changes: 17 additions & 1 deletion Libraries/StyleSheet/TransformPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@

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([
Expand All @@ -30,7 +46,7 @@ var TransformPropTypes = {
ReactPropTypes.shape({skewY: ReactPropTypes.string})
])
),
transformMatrix: ReactPropTypes.arrayOf(ReactPropTypes.number),
transformMatrix: TransformMatrixPropType,
};

module.exports = TransformPropTypes;
Original file line number Diff line number Diff line change
Expand Up @@ -6,74 +6,17 @@
* 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 precomputeStyle
* @providesModule processTransform
* @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.
Expand All @@ -82,8 +25,7 @@ function _processor(key: string, validAttributes: Object) {
* be applied in an arbitrary order, and yet have a universal, singular
* interface to native code.
*/
function _precomputeTransforms(style: Object): Object {
var {transform} = style;
function processTransform(transform: Object): Object {
var result = MatrixMath.createIdentityMatrix();

transform.forEach(transformation => {
Expand Down Expand Up @@ -144,16 +86,9 @@ function _precomputeTransforms(style: 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 {
...style,
transformMatrix: result,
decomposedMatrix: MatrixMath.decomposeMatrix(result),
};
return MatrixMath.decomposeMatrix(result);
}
return {
...style,
transformMatrix: result,
};
return result;
}

/**
Expand Down Expand Up @@ -240,4 +175,4 @@ function _validateTransform(key, value, transformation) {
}
}

module.exports = precomputeStyle;
module.exports = processTransform;

0 comments on commit 6244fd0

Please sign in to comment.