From 51133235e0087976d00666faf1001ff6dd25f226 Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 14 Aug 2019 16:07:10 -0700 Subject: [PATCH] Add failure tests for duplicate props with the same name Summary: There are a couple of cases where props can conflict which would cause undefined behavior. We'll throw to protect against that. Now that we support type spread this is more possible without someone realizing. Reviewed By: JoshuaGross Differential Revision: D16813884 fbshipit-source-id: 1a8fce365ab315198abdff0de6006cfe34e84fb9 --- .../components/__test_fixtures__/failures.js | 101 ++++++++++++++++++ .../component-parser-test.js.snap | 6 ++ .../src/parsers/flow/components/props.js | 15 +++ 3 files changed, 122 insertions(+) diff --git a/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/failures.js b/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/failures.js index 469e51c9042..7ce34a5e5de 100644 --- a/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/failures.js +++ b/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/failures.js @@ -305,6 +305,104 @@ export default (codegenNativeComponent( ): NativeComponent); `; +const PROPS_CONFLICT_NAMES = ` +/** + * 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'; + +import type {ViewProps} from 'ViewPropTypes'; +import type {NativeComponent} from 'codegenNativeComponent'; + +const codegenNativeComponent = require('codegenNativeComponent'); + +export type ModuleProps = $ReadOnly<{| + ...ViewProps, + isEnabled: string, + + isEnabled: boolean, +|}>; + +export default (codegenNativeComponent( + 'Module', +): NativeComponent); +`; + +const PROPS_CONFLICT_WITH_SPREAD_PROPS = ` +/** + * 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'; + +import type {ViewProps} from 'ViewPropTypes'; +import type {NativeComponent} from 'codegenNativeComponent'; + +const codegenNativeComponent = require('codegenNativeComponent'); + +type PropsInFile = $ReadOnly<{| + isEnabled: boolean, +|}>; + +export type ModuleProps = $ReadOnly<{| + ...ViewProps, + + ...PropsInFile, + isEnabled: boolean, +|}>; + +export default (codegenNativeComponent( + 'Module', +): NativeComponent); +`; + +const PROPS_SPREAD_CONFLICTS_WITH_PROPS = ` +/** + * 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'; + +import type {ViewProps} from 'ViewPropTypes'; +import type {NativeComponent} from 'codegenNativeComponent'; + +const codegenNativeComponent = require('codegenNativeComponent'); + +type PropsInFile = $ReadOnly<{| + isEnabled: boolean, +|}>; + +export type ModuleProps = $ReadOnly<{| + ...ViewProps, + + isEnabled: boolean, + ...PropsInFile, +|}>; + +export default (codegenNativeComponent( + 'Module', +): NativeComponent); +`; + module.exports = { COMMANDS_DEFINED_INLINE, COMMANDS_DEFINED_MULTIPLE_TIMES, @@ -314,4 +412,7 @@ module.exports = { COMMANDS_DEFINED_WITH_NULLABLE_REF, NULLABLE_WITH_DEFAULT, NON_OPTIONAL_KEY_WITH_DEFAULT_VALUE, + PROPS_CONFLICT_NAMES, + PROPS_CONFLICT_WITH_SPREAD_PROPS, + PROPS_SPREAD_CONFLICTS_WITH_PROPS, }; diff --git a/packages/react-native-codegen/src/parsers/flow/components/__tests__/__snapshots__/component-parser-test.js.snap b/packages/react-native-codegen/src/parsers/flow/components/__tests__/__snapshots__/component-parser-test.js.snap index f22054b23e6..f55fe241dde 100644 --- a/packages/react-native-codegen/src/parsers/flow/components/__tests__/__snapshots__/component-parser-test.js.snap +++ b/packages/react-native-codegen/src/parsers/flow/components/__tests__/__snapshots__/component-parser-test.js.snap @@ -16,6 +16,12 @@ exports[`RN Codegen Flow Parser Fails with error message NON_OPTIONAL_KEY_WITH_D 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 PROPS_CONFLICT_NAMES 1`] = `"A prop was already defined with the name isEnabled"`; + +exports[`RN Codegen Flow Parser Fails with error message PROPS_CONFLICT_WITH_SPREAD_PROPS 1`] = `"A prop was already defined with the name isEnabled"`; + +exports[`RN Codegen Flow Parser Fails with error message PROPS_SPREAD_CONFLICTS_WITH_PROPS 1`] = `"A prop was already defined with the name isEnabled"`; + exports[`RN Codegen Flow Parser can generate fixture ALL_PROP_TYPES_NO_EVENTS 1`] = ` Object { "modules": Object { diff --git a/packages/react-native-codegen/src/parsers/flow/components/props.js b/packages/react-native-codegen/src/parsers/flow/components/props.js index 7f682975035..da299e04626 100644 --- a/packages/react-native-codegen/src/parsers/flow/components/props.js +++ b/packages/react-native-codegen/src/parsers/flow/components/props.js @@ -302,6 +302,17 @@ function buildPropSchema(property, types: TypeMap): ?PropTypeShape { // $FlowFixMe there's no flowtype for ASTs type PropAST = Object; +function verifyPropNotAlreadyDefined( + props: $ReadOnlyArray, + needleProp: PropAST, +) { + const propName = needleProp.key.name; + const foundProp = props.some(prop => prop.key.name === propName); + if (foundProp) { + throw new Error(`A prop was already defined with the name ${propName}`); + } +} + function flattenProperties( typeDefinition: $ReadOnlyArray, types: TypeMap, @@ -319,8 +330,12 @@ function flattenProperties( }) .reduce((acc, item) => { if (Array.isArray(item)) { + item.forEach(prop => { + verifyPropNotAlreadyDefined(acc, prop); + }); return acc.concat(item); } else { + verifyPropNotAlreadyDefined(acc, item); acc.push(item); return acc; }