From 1b9b17ac96533496bd73c00accf0a5dbe950d3a2 Mon Sep 17 00:00:00 2001 From: Tom Underhill Date: Wed, 12 Feb 2020 09:56:14 -0800 Subject: [PATCH 1/3] Add `NativeColorType` opaque type to normalizeColor --- Libraries/ActionSheetIOS/ActionSheetIOS.js | 7 ++++- .../Components/CheckBox/CheckBox.android.js | 27 ++++++++++++++----- .../Picker/PickerAndroid.android.js | 8 +++++- Libraries/Components/Picker/PickerIOS.ios.js | 8 +++++- Libraries/Components/StatusBar/StatusBar.js | 9 +++++++ .../Touchable/TouchableNativeFeedback.js | 18 +++++++++---- Libraries/Share/Share.js | 5 ++++ Libraries/StyleSheet/processColor.js | 8 +++++- Libraries/StyleSheet/processColorArray.js | 5 +++- 9 files changed, 79 insertions(+), 16 deletions(-) diff --git a/Libraries/ActionSheetIOS/ActionSheetIOS.js b/Libraries/ActionSheetIOS/ActionSheetIOS.js index 1bd16980344472..b0dd906fd4e5f0 100644 --- a/Libraries/ActionSheetIOS/ActionSheetIOS.js +++ b/Libraries/ActionSheetIOS/ActionSheetIOS.js @@ -65,10 +65,15 @@ const ActionSheetIOS = { destructiveButtonIndices = [destructiveButtonIndex]; } + const processedTintColor = processColor(tintColor); + invariant( + processedTintColor == null || typeof processedTintColor === 'number', + 'Unexpected color given for ActionSheetIOS.showActionSheetWithOptions tintColor', + ); RCTActionSheetManager.showActionSheetWithOptions( { ...remainingOptions, - tintColor: processColor(tintColor), + tintColor: processedTintColor, destructiveButtonIndices, }, callback, diff --git a/Libraries/Components/CheckBox/CheckBox.android.js b/Libraries/Components/CheckBox/CheckBox.android.js index 179680a7e44a57..4f9fe3e758090c 100644 --- a/Libraries/Components/CheckBox/CheckBox.android.js +++ b/Libraries/Components/CheckBox/CheckBox.android.js @@ -12,6 +12,7 @@ const React = require('react'); const StyleSheet = require('../../StyleSheet/StyleSheet'); +const invariant = require('invariant'); const processColor = require('../../StyleSheet/processColor'); const nullthrows = require('nullthrows'); @@ -152,12 +153,26 @@ class CheckBox extends React.Component { }; _getTintColors(tintColors) { - return tintColors - ? { - true: processColor(tintColors.true), - false: processColor(tintColors.false), - } - : undefined; + if (tintColors) { + const processedTextColorTrue = processColor(tintColors.true); + invariant( + processedTextColorTrue == null || + typeof processedTextColorTrue === 'number', + 'Unexpected color given for tintColors.true', + ); + const processedTextColorFalse = processColor(tintColors.true); + invariant( + processedTextColorFalse == null || + typeof processedTextColorFalse === 'number', + 'Unexpected color given for tintColors.true', + ); + return { + true: processedTextColorTrue, + false: processedTextColorFalse, + }; + } else { + return undefined; + } } render() { diff --git a/Libraries/Components/Picker/PickerAndroid.android.js b/Libraries/Components/Picker/PickerAndroid.android.js index 6788aefd06fabc..5bd2b31b41f9dc 100644 --- a/Libraries/Components/Picker/PickerAndroid.android.js +++ b/Libraries/Components/Picker/PickerAndroid.android.js @@ -18,6 +18,7 @@ import AndroidDialogPickerNativeComponent, { } from './AndroidDialogPickerNativeComponent'; import * as React from 'react'; import StyleSheet from '../../StyleSheet/StyleSheet'; +import invariant from 'invariant'; import processColor from '../../StyleSheet/processColor'; import type {SyntheticEvent} from '../../Types/CoreEventTypes'; @@ -61,8 +62,13 @@ function PickerAndroid(props: Props): React.Node { selected = index; } const {color, label} = child.props; + const processedColor = processColor(color); + invariant( + processedColor == null || typeof processedColor === 'number', + 'Unexpected color given for PickerAndroid color prop', + ); return { - color: color == null ? null : processColor(color), + color: color == null ? null : processedColor, label, }; }); diff --git a/Libraries/Components/Picker/PickerIOS.ios.js b/Libraries/Components/Picker/PickerIOS.ios.js index 4222ebf7646e44..e99f2b9f679a2c 100644 --- a/Libraries/Components/Picker/PickerIOS.ios.js +++ b/Libraries/Components/Picker/PickerIOS.ios.js @@ -16,6 +16,7 @@ const React = require('react'); const StyleSheet = require('../../StyleSheet/StyleSheet'); const View = require('../View/View'); +const invariant = require('invariant'); const processColor = require('../../StyleSheet/processColor'); import RCTPickerNativeComponent, { @@ -86,10 +87,15 @@ class PickerIOS extends React.Component { if (child.props.value === props.selectedValue) { selectedIndex = index; } + const processedTextColor = processColor(child.props.color); + invariant( + processedTextColor == null || typeof processedTextColor === 'number', + 'Unexpected color given for PickerIOSItem color', + ); items.push({ value: child.props.value, label: child.props.label, - textColor: processColor(child.props.color), + textColor: processedTextColor, }); }); return {selectedIndex, items}; diff --git a/Libraries/Components/StatusBar/StatusBar.js b/Libraries/Components/StatusBar/StatusBar.js index b62c0dcb11a432..eb7e8661d07953 100644 --- a/Libraries/Components/StatusBar/StatusBar.js +++ b/Libraries/Components/StatusBar/StatusBar.js @@ -13,6 +13,7 @@ const Platform = require('../../Utilities/Platform'); const React = require('react'); +const invariant = require('invariant'); const processColor = require('../../StyleSheet/processColor'); import NativeStatusBarManagerAndroid from './NativeStatusBarManagerAndroid'; @@ -322,6 +323,10 @@ class StatusBar extends React.Component { ); return; } + invariant( + typeof processedColor === 'number', + 'Unexpected color given for StatusBar.setBackgroundColor', + ); NativeStatusBarManagerAndroid.setColor(processedColor, animated); } @@ -466,6 +471,10 @@ class StatusBar extends React.Component { } parsed to null or undefined`, ); } else { + invariant( + typeof processedColor === 'number', + 'Unexpected color given in StatusBar._updatePropsStack', + ); NativeStatusBarManagerAndroid.setColor( processedColor, mergedProps.backgroundColor.animated, diff --git a/Libraries/Components/Touchable/TouchableNativeFeedback.js b/Libraries/Components/Touchable/TouchableNativeFeedback.js index b6c87d09861774..a2903b3f849a4d 100644 --- a/Libraries/Components/Touchable/TouchableNativeFeedback.js +++ b/Libraries/Components/Touchable/TouchableNativeFeedback.js @@ -23,6 +23,7 @@ import Platform from '../../Utilities/Platform'; import View from '../../Components/View/View'; import processColor from '../../StyleSheet/processColor'; import * as React from 'react'; +import invariant from 'invariant'; type Props = $ReadOnly<{| ...React.ElementConfig, @@ -131,11 +132,18 @@ class TouchableNativeFeedback extends React.Component { borderless: boolean, color: ?number, type: 'RippleAndroid', - |}> = (color: string, borderless: boolean) => ({ - type: 'RippleAndroid', - color: processColor(color), - borderless, - }); + |}> = (color: string, borderless: boolean) => { + const processedColor = processColor(color); + invariant( + processedColor == null || typeof processedColor === 'number', + 'Unexpected color given for Ripple color', + ); + return { + type: 'RippleAndroid', + color: processedColor, + borderless, + }; + }; /** * Whether `useForeground` is supported. diff --git a/Libraries/Share/Share.js b/Libraries/Share/Share.js index 5b61f2919ddb34..0c2caa78553be5 100644 --- a/Libraries/Share/Share.js +++ b/Libraries/Share/Share.js @@ -109,6 +109,11 @@ class Share { return new Promise((resolve, reject) => { const tintColor = processColor(options.tintColor); + invariant( + tintColor == null || typeof tintColor === 'number', + 'Unexpected color given for options.tintColor', + ); + invariant( NativeActionSheetManager, 'NativeActionSheetManager is not registered on iOS, but it should be.', diff --git a/Libraries/StyleSheet/processColor.js b/Libraries/StyleSheet/processColor.js index 28293e0df1f0a2..1e75cc7bb56dd0 100644 --- a/Libraries/StyleSheet/processColor.js +++ b/Libraries/StyleSheet/processColor.js @@ -14,8 +14,14 @@ const Platform = require('../Utilities/Platform'); const normalizeColor = require('./normalizeColor'); +// TODO: This is an empty object for now, just to enforce that everything using this +// downstream is correct. This will be replaced with an import to other files +// with a platform specific implementation. See the PR for more information +// https://github.com/facebook/react-native/pull/27908 +export opaque type NativeColorType = {}; + /* eslint no-bitwise: 0 */ -function processColor(color?: ?(string | number)): ?number { +function processColor(color?: ?(string | number)): ?number | NativeColorType { if (color === undefined || color === null) { return color; } diff --git a/Libraries/StyleSheet/processColorArray.js b/Libraries/StyleSheet/processColorArray.js index 711dc32aee5413..e91e581ac6dde1 100644 --- a/Libraries/StyleSheet/processColorArray.js +++ b/Libraries/StyleSheet/processColorArray.js @@ -11,8 +11,11 @@ 'use strict'; const processColor = require('./processColor'); +import type {NativeColorType} from './processColor'; -function processColorArray(colors: ?Array): ?Array { +function processColorArray( + colors: ?Array, +): ?Array { return colors == null ? null : colors.map(processColor); } From 2514db4c3037a8a6cc63f8a9ea98a8435b27d03a Mon Sep 17 00:00:00 2001 From: Tom Underhill Date: Wed, 12 Feb 2020 10:01:59 -0800 Subject: [PATCH 2/3] Fixed invariant message string. --- Libraries/Components/CheckBox/CheckBox.android.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Libraries/Components/CheckBox/CheckBox.android.js b/Libraries/Components/CheckBox/CheckBox.android.js index 4f9fe3e758090c..aed9cd9eb389c4 100644 --- a/Libraries/Components/CheckBox/CheckBox.android.js +++ b/Libraries/Components/CheckBox/CheckBox.android.js @@ -164,7 +164,7 @@ class CheckBox extends React.Component { invariant( processedTextColorFalse == null || typeof processedTextColorFalse === 'number', - 'Unexpected color given for tintColors.true', + 'Unexpected color given for tintColors.false', ); return { true: processedTextColorTrue, From be42e831644943a54cc64928f80d056add89aa06 Mon Sep 17 00:00:00 2001 From: Tom Underhill Date: Wed, 12 Feb 2020 12:07:43 -0800 Subject: [PATCH 3/3] Added `ProcessedColorValue` type to wrap `?number | NativeColorType` as it does in the larger PR. --- Libraries/StyleSheet/processColor.js | 5 +++-- Libraries/StyleSheet/processColorArray.js | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Libraries/StyleSheet/processColor.js b/Libraries/StyleSheet/processColor.js index 1e75cc7bb56dd0..46b298e7be9e84 100644 --- a/Libraries/StyleSheet/processColor.js +++ b/Libraries/StyleSheet/processColor.js @@ -18,10 +18,11 @@ const normalizeColor = require('./normalizeColor'); // downstream is correct. This will be replaced with an import to other files // with a platform specific implementation. See the PR for more information // https://github.com/facebook/react-native/pull/27908 -export opaque type NativeColorType = {}; +opaque type NativeColorType = {}; +export type ProcessedColorValue = ?number | NativeColorType; /* eslint no-bitwise: 0 */ -function processColor(color?: ?(string | number)): ?number | NativeColorType { +function processColor(color?: ?(string | number)): ProcessedColorValue { if (color === undefined || color === null) { return color; } diff --git a/Libraries/StyleSheet/processColorArray.js b/Libraries/StyleSheet/processColorArray.js index e91e581ac6dde1..e84482c8cb681e 100644 --- a/Libraries/StyleSheet/processColorArray.js +++ b/Libraries/StyleSheet/processColorArray.js @@ -11,11 +11,11 @@ 'use strict'; const processColor = require('./processColor'); -import type {NativeColorType} from './processColor'; +import type {ProcessedColorValue} from './processColor'; function processColorArray( colors: ?Array, -): ?Array { +): ?Array { return colors == null ? null : colors.map(processColor); }