Skip to content

Commit

Permalink
Add failure tests for duplicate props with the same name
Browse files Browse the repository at this point in the history
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
  • Loading branch information
elicwhite authored and facebook-github-bot committed Aug 14, 2019
1 parent ac6c747 commit 5113323
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,104 @@ export default (codegenNativeComponent<ModuleProps>(
): NativeComponent<ModuleProps>);
`;

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<ModuleProps>(
'Module',
): NativeComponent<ModuleProps>);
`;

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<ModuleProps>(
'Module',
): NativeComponent<ModuleProps>);
`;

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<ModuleProps>(
'Module',
): NativeComponent<ModuleProps>);
`;

module.exports = {
COMMANDS_DEFINED_INLINE,
COMMANDS_DEFINED_MULTIPLE_TIMES,
Expand All @@ -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,
};
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions packages/react-native-codegen/src/parsers/flow/components/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,17 @@ function buildPropSchema(property, types: TypeMap): ?PropTypeShape {
// $FlowFixMe there's no flowtype for ASTs
type PropAST = Object;

function verifyPropNotAlreadyDefined(
props: $ReadOnlyArray<PropAST>,
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<PropAST>,
types: TypeMap,
Expand All @@ -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;
}
Expand Down

0 comments on commit 5113323

Please sign in to comment.