From dc52f8c2e67dd130a86eb71a33fae1d3cb916472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Osadnik?= Date: Tue, 9 Jul 2019 14:40:51 -0700 Subject: [PATCH] Force property to be optional if value has WithDefault Summary: It's pointless to handle non optional key if value has withDefaul so I'm adding a rule into parser preventing from such cases Reviewed By: lunaleaps Differential Revision: D16166709 fbshipit-source-id: 38cef522b217917a3a4886d857720932f2ebb475 --- .../Slider/SliderNativeComponent.js | 2 +- .../flow/__test_fixtures__/failures.js | 32 +++++++ .../flow/__test_fixtures__/fixtures.js | 9 -- .../__snapshots__/parser-test.js.snap | 93 +------------------ .../src/parsers/flow/props.js | 9 ++ 5 files changed, 44 insertions(+), 101 deletions(-) diff --git a/Libraries/Components/Slider/SliderNativeComponent.js b/Libraries/Components/Slider/SliderNativeComponent.js index f6475bbd882..95e68062362 100644 --- a/Libraries/Components/Slider/SliderNativeComponent.js +++ b/Libraries/Components/Slider/SliderNativeComponent.js @@ -45,7 +45,7 @@ type NativeProps = $ReadOnly<{| thumbImage?: ?ImageSource, thumbTintColor?: ?ColorValue, trackImage?: ?ImageSource, - value: WithDefault, + value?: WithDefault, // Events onChange?: ?BubblingEventHandler, diff --git a/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/failures.js b/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/failures.js index cbadbc2e9cd..bd4ec135fef 100644 --- a/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/failures.js +++ b/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/failures.js @@ -396,7 +396,39 @@ export type ModuleProps = $ReadOnly<{| export default codegenNativeComponent('Module'); `; +const NON_OPTIONAL_KEY_WITH_DEFAULT_VALUE = ` +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @flow + */ + +'use strict'; + +const codegenNativeComponent = require('codegenNativeComponent'); + +import type { + WithDefault, + Float, +} from 'CodegenTypes'; + +import type {ViewProps} from 'ViewPropTypes'; + + +export type ModuleProps = $ReadOnly<{| + ...ViewProps, + required_key_with_default: WithDefault, +|}>; + +export default codegenNativeComponent('Module'); +`; + module.exports = { + NON_OPTIONAL_KEY_WITH_DEFAULT_VALUE, NATIVE_MODULES_WITH_PROMISE_WITHOUT_TYPE, NATIVE_MODULES_WITH_ARRAY_WITH_NO_TYPE_FOR_CONTENT_AS_PARAM, NATIVE_MODULES_WITH_ARRAY_WITH_NO_TYPE_FOR_CONTENT, diff --git a/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/fixtures.js index d0365fdd257..9af6d72e2f1 100644 --- a/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/fixtures.js +++ b/packages/react-native-codegen/src/parsers/flow/__test_fixtures__/fixtures.js @@ -449,46 +449,38 @@ type ModuleProps = $ReadOnly<{| // Boolean props boolean_required: boolean, boolean_optional_key?: WithDefault, - boolean_optional_value: WithDefault, boolean_optional_both?: WithDefault, // String props string_required: string, string_optional_key?: WithDefault, - string_optional_value: WithDefault, string_optional_both?: WithDefault, // String props, null default string_null_optional_key?: WithDefault, - string_null_optional_value: WithDefault, string_null_optional_both?: WithDefault, // Stringish props stringish_required: Stringish, stringish_optional_key?: WithDefault, - stringish_optional_value: WithDefault, stringish_optional_both?: WithDefault, // Stringish props, null default stringish_null_optional_key?: WithDefault, - stringish_null_optional_value: WithDefault, stringish_null_optional_both?: WithDefault, // Float props float_required: Float, float_optional_key?: WithDefault, - float_optional_value: WithDefault, float_optional_both?: WithDefault, // Int32 props int32_required: Int32, int32_optional_key?: WithDefault, - int32_optional_value: WithDefault, int32_optional_both?: WithDefault, // String enum props enum_optional_key?: WithDefault<('small' | 'large'), 'small'>, - enum_optional_value: WithDefault<('small' | 'large'), 'small'>, enum_optional_both?: WithDefault<('small' | 'large'), 'small'>, // ImageSource props @@ -573,7 +565,6 @@ type ModuleProps = $ReadOnly<{| // String enum props array_enum_optional_key?: WithDefault<$ReadOnlyArray<('small' | 'large')>, 'small'>, - array_enum_optional_value: WithDefault<$ReadOnlyArray<('small' | 'large')>, 'small'>, array_enum_optional_both?: WithDefault<$ReadOnlyArray<('small' | 'large')>, 'small'>, // ImageSource props diff --git a/packages/react-native-codegen/src/parsers/flow/__tests__/__snapshots__/parser-test.js.snap b/packages/react-native-codegen/src/parsers/flow/__tests__/__snapshots__/parser-test.js.snap index 3f9b1abb98c..229b7709e09 100644 --- a/packages/react-native-codegen/src/parsers/flow/__tests__/__snapshots__/parser-test.js.snap +++ b/packages/react-native-codegen/src/parsers/flow/__tests__/__snapshots__/parser-test.js.snap @@ -24,6 +24,8 @@ exports[`RN Codegen Flow Parser Fails with error message NATIVE_MODULES_WITH_NOT exports[`RN Codegen Flow Parser Fails with error message NATIVE_MODULES_WITH_PROMISE_WITHOUT_TYPE 1`] = `"Unsupported return promise type for getBool: expected to find annotation for type of promise content"`; +exports[`RN Codegen Flow Parser Fails with error message NON_OPTIONAL_KEY_WITH_DEFAULT_VALUE 1`] = `"key required_key_with_default must be optional if used with WithDefault<> annotation"`; + exports[`RN Codegen Flow Parser Fails with error message NULLABLE_WITH_DEFAULT 1`] = `"WithDefault<> is optional and does not need to be marked as optional. Please remove the ? annotation in front of it."`; exports[`RN Codegen Flow Parser Fails with error message TWO_NATIVE_MODULES_EXPORTED_WITH_DEFAULT 1`] = `"File should contain only one default export."`; @@ -59,14 +61,6 @@ Object { "type": "BooleanTypeAnnotation", }, }, - Object { - "name": "boolean_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": true, - "type": "BooleanTypeAnnotation", - }, - }, Object { "name": "boolean_optional_both", "optional": true, @@ -91,14 +85,6 @@ Object { "type": "StringTypeAnnotation", }, }, - Object { - "name": "string_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": "", - "type": "StringTypeAnnotation", - }, - }, Object { "name": "string_optional_both", "optional": true, @@ -115,14 +101,6 @@ Object { "type": "StringTypeAnnotation", }, }, - Object { - "name": "string_null_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": null, - "type": "StringTypeAnnotation", - }, - }, Object { "name": "string_null_optional_both", "optional": true, @@ -147,14 +125,6 @@ Object { "type": "StringTypeAnnotation", }, }, - Object { - "name": "stringish_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": "", - "type": "StringTypeAnnotation", - }, - }, Object { "name": "stringish_optional_both", "optional": true, @@ -171,14 +141,6 @@ Object { "type": "StringTypeAnnotation", }, }, - Object { - "name": "stringish_null_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": null, - "type": "StringTypeAnnotation", - }, - }, Object { "name": "stringish_null_optional_both", "optional": true, @@ -203,14 +165,6 @@ Object { "type": "FloatTypeAnnotation", }, }, - Object { - "name": "float_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": 1.1, - "type": "FloatTypeAnnotation", - }, - }, Object { "name": "float_optional_both", "optional": true, @@ -235,14 +189,6 @@ Object { "type": "Int32TypeAnnotation", }, }, - Object { - "name": "int32_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": 1, - "type": "Int32TypeAnnotation", - }, - }, Object { "name": "int32_optional_both", "optional": true, @@ -267,22 +213,6 @@ Object { "type": "StringEnumTypeAnnotation", }, }, - Object { - "name": "enum_optional_value", - "optional": true, - "typeAnnotation": Object { - "default": "small", - "options": Array [ - Object { - "name": "small", - }, - Object { - "name": "large", - }, - ], - "type": "StringEnumTypeAnnotation", - }, - }, Object { "name": "enum_optional_both", "optional": true, @@ -633,25 +563,6 @@ Object { "type": "ArrayTypeAnnotation", }, }, - Object { - "name": "array_enum_optional_value", - "optional": true, - "typeAnnotation": Object { - "elementType": Object { - "default": "small", - "options": Array [ - Object { - "name": "small", - }, - Object { - "name": "large", - }, - ], - "type": "StringEnumTypeAnnotation", - }, - "type": "ArrayTypeAnnotation", - }, - }, Object { "name": "array_enum_optional_both", "optional": true, diff --git a/packages/react-native-codegen/src/parsers/flow/props.js b/packages/react-native-codegen/src/parsers/flow/props.js index 9d7907ddf8d..368f854aa1d 100644 --- a/packages/react-native-codegen/src/parsers/flow/props.js +++ b/packages/react-native-codegen/src/parsers/flow/props.js @@ -187,6 +187,15 @@ function buildPropSchema(property): ?PropTypeShape { (value.type === 'GenericTypeAnnotation' && typeAnnotation.id.name === 'WithDefault'); + if ( + !property.optional && + value.type === 'GenericTypeAnnotation' && + typeAnnotation.id.name === 'WithDefault' + ) { + throw new Error( + `key ${name} must be optional if used with WithDefault<> annotation`, + ); + } if ( value.type === 'NullableTypeAnnotation' && (typeAnnotation.type === 'GenericTypeAnnotation' &&