Skip to content

Commit

Permalink
BREAKING - Improve JS transform validation, add tests
Browse files Browse the repository at this point in the history
Summary:
This improves JS validations of the transform object and makes it a bit stricter (hence the breaking change). When moving transform objects parsing to native (#10658) the validations got out of sync a bit, this makes sure JS validations are the same or stricter than the native ones to make sure we get consistent errors across platforms.

See #12110 for an example of an error that now gets caught by JS validations.

Also added snapshot tests for the errors to make sure `processTransform` throws when passing invalid values. It only tests the validation since the object parsing is now done natively for iOS and Android.

**Test plan**
Test that there are no errors in UIExplorer
Run new unit tests
Closes #12115

Differential Revision: D4488933

Pulled By: mkonicek

fbshipit-source-id: a714e6175b2892284a44c870506165099efec1ed
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Feb 2, 2017
1 parent a2c84d1 commit 0ed31eb
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 4 deletions.
@@ -0,0 +1,27 @@
exports[`processTransform validation should throw on invalid transform property 1`] = `"Invalid transform translateW: {\"translateW\":10}"`;
exports[`processTransform validation should throw on object with multiple properties 1`] = `"You must specify exactly one property per transform object. Passed properties: {\"scale\":0.5,\"translateY\":10}"`;
exports[`processTransform validation should throw when not passing an array to an array prop 1`] = `"Transform with key of matrix must have an array as the value: {\"matrix\":\"not-a-matrix\"}"`;
exports[`processTransform validation should throw when not passing an array to an array prop 2`] = `"Transform with key of translate must have an array as the value: {\"translate\":10}"`;
exports[`processTransform validation should throw when passing a matrix of the wrong size 1`] = `"Matrix transform must have a length of 9 (2d) or 16 (3d). Provided matrix has a length of 4: {\"matrix\":[1,1,1,1]}"`;
exports[`processTransform validation should throw when passing a perspective of 0 1`] = `"Transform with key of \"perspective\" cannot be zero: {\"perspective\":0}"`;
exports[`processTransform validation should throw when passing a translate of the wrong size 1`] = `"Transform with key translate must be an array of length 2 or 3, found 1: {\"translate\":[1]}"`;
exports[`processTransform validation should throw when passing a translate of the wrong size 2`] = `"Transform with key translate must be an array of length 2 or 3, found 4: {\"translate\":[1,1,1,1]}"`;
exports[`processTransform validation should throw when passing an Animated.Value 1`] = `"You passed an Animated.Value to a normal component. You need to wrap that component in an Animated. For example, replace <View /> by <Animated.View />."`;
exports[`processTransform validation should throw when passing an invalid angle prop 1`] = `"Transform with key of \"rotate\" must be a string: {\"rotate\":10}"`;
exports[`processTransform validation should throw when passing an invalid angle prop 2`] = `"Rotate transform must be expressed in degrees (deg) or radians (rad): {\"skewX\":\"10drg\"}"`;
exports[`processTransform validation should throw when passing an invalid value to a number prop 1`] = `"Transform with key of \"translateY\" must be a number: {\"translateY\":\"20deg\"}"`;
exports[`processTransform validation should throw when passing an invalid value to a number prop 2`] = `"Transform with key of \"scale\" must be a number: {\"scale\":{\"x\":10,\"y\":10}}"`;
exports[`processTransform validation should throw when passing an invalid value to a number prop 3`] = `"Transform with key of \"perspective\" must be a number: {\"perspective\":[]}"`;
86 changes: 86 additions & 0 deletions Libraries/StyleSheet/__tests__/processTransform-test.js
@@ -0,0 +1,86 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* 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.
*/
'use strict';

jest.disableAutomock();

const processTransform = require('processTransform');

describe('processTransform', () => {
describe('validation', () => {
it('should accept an empty array', () => {
processTransform([]);
});

it('should accept a simple valid transform', () => {
processTransform([
{scale: 0.5},
{translateX: 10},
{translateY: 20},
{rotate: '10deg'},
]);
});

it('should throw on object with multiple properties', () => {
expect(() => processTransform([{scale: 0.5, translateY: 10}])).toThrowErrorMatchingSnapshot();
});

it('should throw on invalid transform property', () => {
expect(() => processTransform([{translateW: 10}])).toThrowErrorMatchingSnapshot();
});

it('should throw when not passing an array to an array prop', () => {
expect(() => processTransform([{matrix: 'not-a-matrix'}])).toThrowErrorMatchingSnapshot();
expect(() => processTransform([{translate: 10}])).toThrowErrorMatchingSnapshot();
});

it('should accept a valid matrix', () => {
processTransform([{matrix: [1, 1, 1, 1, 1, 1, 1, 1, 1]}]);
processTransform([{matrix: [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]}]);
});

it('should throw when passing a matrix of the wrong size', () => {
expect(() => processTransform([{matrix: [1, 1, 1, 1]}])).toThrowErrorMatchingSnapshot();
});

it('should accept a valid translate', () => {
processTransform([{translate: [1, 1]}]);
processTransform([{translate: [1, 1, 1]}]);
});

it('should throw when passing a translate of the wrong size', () => {
expect(() => processTransform([{translate: [1]}])).toThrowErrorMatchingSnapshot();
expect(() => processTransform([{translate: [1, 1, 1, 1]}])).toThrowErrorMatchingSnapshot();
});

it('should throw when passing an invalid value to a number prop', () => {
expect(() => processTransform([{translateY: '20deg'}])).toThrowErrorMatchingSnapshot();
expect(() => processTransform([{scale: {x: 10, y: 10}}])).toThrowErrorMatchingSnapshot();
expect(() => processTransform([{perspective: []}])).toThrowErrorMatchingSnapshot();
});

it('should throw when passing a perspective of 0', () => {
expect(() => processTransform([{perspective: 0}])).toThrowErrorMatchingSnapshot();
});

it('should accept an angle in degrees or radians', () => {
processTransform([{skewY: '10deg'}]);
processTransform([{rotateX: '1.16rad'}]);
});

it('should throw when passing an invalid angle prop', () => {
expect(() => processTransform([{rotate: 10}])).toThrowErrorMatchingSnapshot();
expect(() => processTransform([{skewX: '10drg'}])).toThrowErrorMatchingSnapshot();
});

it('should throw when passing an Animated.Value', () => {
expect(() => processTransform([{rotate: {getValue: () => {}}}])).toThrowErrorMatchingSnapshot();
});
});
});
27 changes: 23 additions & 4 deletions Libraries/StyleSheet/processTransform.js
Expand Up @@ -25,7 +25,7 @@ var stringifySafe = require('stringifySafe');
* be applied in an arbitrary order, and yet have a universal, singular * be applied in an arbitrary order, and yet have a universal, singular
* interface to native code. * interface to native code.
*/ */
function processTransform(transform: Object): Object { function processTransform(transform: Array<Object>): Array<Object> | Array<number> {
if (__DEV__) { if (__DEV__) {
_validateTransforms(transform); _validateTransforms(transform);
} }
Expand Down Expand Up @@ -115,9 +115,15 @@ function _convertToRadians(value: string): number {
return value.indexOf('rad') > -1 ? floatValue : floatValue * Math.PI / 180; return value.indexOf('rad') > -1 ? floatValue : floatValue * Math.PI / 180;
} }


function _validateTransforms(transform: Object): void { function _validateTransforms(transform: Array<Object>): void {
transform.forEach(transformation => { transform.forEach(transformation => {
var key = Object.keys(transformation)[0]; var keys = Object.keys(transformation);
invariant(
keys.length === 1,
'You must specify exactly one property per transform object. Passed properties: %s',
stringifySafe(transformation),
);
var key = keys[0];
var value = transformation[key]; var value = transformation[key];
_validateTransform(key, value, transformation); _validateTransform(key, value, transformation);
}); });
Expand Down Expand Up @@ -154,6 +160,12 @@ function _validateTransform(key, value, transformation) {
); );
break; break;
case 'translate': case 'translate':
invariant(
value.length === 2 || value.length === 3,
'Transform with key translate must be an array of length 2 or 3, found %s: %s',
value.length,
stringifySafe(transformation),
);
break; break;
case 'rotateX': case 'rotateX':
case 'rotateY': case 'rotateY':
Expand Down Expand Up @@ -188,13 +200,20 @@ function _validateTransform(key, value, transformation) {
stringifySafe(transformation), stringifySafe(transformation),
); );
break; break;
default: case 'translateX':
case 'translateY':
case 'scale':
case 'scaleX':
case 'scaleY':
invariant( invariant(
typeof value === 'number', typeof value === 'number',
'Transform with key of "%s" must be a number: %s', 'Transform with key of "%s" must be a number: %s',
key, key,
stringifySafe(transformation), stringifySafe(transformation),
); );
break;
default:
invariant(false, 'Invalid transform %s: %s', key, stringifySafe(transformation));
} }
} }


Expand Down

0 comments on commit 0ed31eb

Please sign in to comment.