From 0ed31eb3d66b7f8a2d752d93b5ec8578544e3cff Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Thu, 2 Feb 2017 06:23:30 -0800 Subject: [PATCH] BREAKING - Improve JS transform validation, add tests 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 https://github.com/facebook/react-native/pull/12115 Differential Revision: D4488933 Pulled By: mkonicek fbshipit-source-id: a714e6175b2892284a44c870506165099efec1ed --- .../processTransform-test.js.snap | 27 ++++++ .../__tests__/processTransform-test.js | 86 +++++++++++++++++++ Libraries/StyleSheet/processTransform.js | 27 +++++- 3 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 Libraries/StyleSheet/__tests__/__snapshots__/processTransform-test.js.snap create mode 100644 Libraries/StyleSheet/__tests__/processTransform-test.js diff --git a/Libraries/StyleSheet/__tests__/__snapshots__/processTransform-test.js.snap b/Libraries/StyleSheet/__tests__/__snapshots__/processTransform-test.js.snap new file mode 100644 index 000000000..548a7a814 --- /dev/null +++ b/Libraries/StyleSheet/__tests__/__snapshots__/processTransform-test.js.snap @@ -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 by ."`; + +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\":[]}"`; diff --git a/Libraries/StyleSheet/__tests__/processTransform-test.js b/Libraries/StyleSheet/__tests__/processTransform-test.js new file mode 100644 index 000000000..ac5cde1cc --- /dev/null +++ b/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(); + }); + }); +}); diff --git a/Libraries/StyleSheet/processTransform.js b/Libraries/StyleSheet/processTransform.js index a00dc1d01..695bb07f0 100644 --- a/Libraries/StyleSheet/processTransform.js +++ b/Libraries/StyleSheet/processTransform.js @@ -25,7 +25,7 @@ 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 processTransform(transform: Array): Array | Array { if (__DEV__) { _validateTransforms(transform); } @@ -115,9 +115,15 @@ function _convertToRadians(value: string): number { return value.indexOf('rad') > -1 ? floatValue : floatValue * Math.PI / 180; } -function _validateTransforms(transform: Object): void { +function _validateTransforms(transform: Array): void { 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]; _validateTransform(key, value, transformation); }); @@ -154,6 +160,12 @@ function _validateTransform(key, value, transformation) { ); break; 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; case 'rotateX': case 'rotateY': @@ -188,13 +200,20 @@ function _validateTransform(key, value, transformation) { stringifySafe(transformation), ); break; - default: + case 'translateX': + case 'translateY': + case 'scale': + case 'scaleX': + case 'scaleY': invariant( typeof value === 'number', 'Transform with key of "%s" must be a number: %s', key, stringifySafe(transformation), ); + break; + default: + invariant(false, 'Invalid transform %s: %s', key, stringifySafe(transformation)); } }