From 678d5f7cd1f866f771a5d94dc231ccc6ec21b671 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Wed, 31 Jul 2019 14:04:11 -0700 Subject: [PATCH] Back out "[RNCodegen] codegenNativeCommands takes list of supported commands" Summary: Original commit changeset: 189754a567a3 Reviewed By: rickhanlonii Differential Revision: D16586885 fbshipit-source-id: 34a8f8395ca73e190ccf0242f02626094f6d87b6 --- Libraries/Utilities/codegenNativeCommands.js | 6 +- .../__test_fixtures__/fixtures.js | 4 +- .../package.json | 2 +- .../components/__test_fixtures__/failures.js | 97 +------------------ .../components/__test_fixtures__/fixtures.js | 12 +-- .../component-parser-test.js.snap | 4 - .../src/parsers/flow/components/index.js | 64 ++---------- .../src/parsers/flow/components/options.js | 32 ------ 8 files changed, 19 insertions(+), 202 deletions(-) diff --git a/Libraries/Utilities/codegenNativeCommands.js b/Libraries/Utilities/codegenNativeCommands.js index b0daa447d7f..a6dc594f642 100644 --- a/Libraries/Utilities/codegenNativeCommands.js +++ b/Libraries/Utilities/codegenNativeCommands.js @@ -10,11 +10,7 @@ 'use strict'; -type Options = $ReadOnly<{| - supportedCommands: $ReadOnlyArray, -|}>; - -function codegenNativeCommands(options: Options<$Keys>): T { +function codegenNativeCommands(): T { return (({}: any): T); } diff --git a/packages/babel-plugin-inline-view-configs/__test_fixtures__/fixtures.js b/packages/babel-plugin-inline-view-configs/__test_fixtures__/fixtures.js index 9f0925af87f..c40d4ed27f7 100644 --- a/packages/babel-plugin-inline-view-configs/__test_fixtures__/fixtures.js +++ b/packages/babel-plugin-inline-view-configs/__test_fixtures__/fixtures.js @@ -46,9 +46,7 @@ type ModuleProps = $ReadOnly<{| onBubblingEventDefinedInlineNull: BubblingEventHandler, |}>; -export const Commands = codegenNativeCommands({ - supportedCommands: ['hotspotUpdate', 'scrollTo'], -}); +export const Commands = codegenNativeCommands(); export default codegenNativeComponent('Module', { interfaceOnly: true, diff --git a/packages/babel-plugin-inline-view-configs/package.json b/packages/babel-plugin-inline-view-configs/package.json index ad1dc6e0d0c..d61d30de9b3 100644 --- a/packages/babel-plugin-inline-view-configs/package.json +++ b/packages/babel-plugin-inline-view-configs/package.json @@ -1,5 +1,5 @@ { - "version": "0.0.4", + "version": "0.0.3", "name": "babel-plugin-inline-view-configs", "description": "Babel plugin to inline view configs for React Native", "repository": { 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 de68dd37b1b..d9a2fafd1b8 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 @@ -41,9 +41,7 @@ export type ModuleProps = $ReadOnly<{| export const Commands = codegenNativeCommands<{ +hotspotUpdate: (ref: React.Ref<'RCTView'>, x: Int32, y: Int32) => void; -}>({ - supportedCommands: ['hotspotUpdate'] -}); +}>(); export default codegenNativeComponent('Module'); `; @@ -81,12 +79,8 @@ export type ModuleProps = $ReadOnly<{| // No props or events |}>; -export const Commands = codegenNativeCommands({ - supportedCommands: ['hotspotUpdate'] -}); -export const Commands2 = codegenNativeCommands({ - supportedCommands: ['hotspotUpdate'] -}); +export const Commands = codegenNativeCommands(); +export const Commands2 = codegenNativeCommands(); export default codegenNativeComponent('Module'); `; @@ -124,93 +118,10 @@ export type ModuleProps = $ReadOnly<{| // No props or events |}>; -export const Commands = codegenNativeCommands({ - supportedCommands: ['hotspotUpdate'] -}); - -export default codegenNativeComponent('Module'); -`; - -const COMMANDS_DEFINED_WITH_MISMATCHED_METHOD_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'; - -const codegenNativeComponent = require('codegenNativeComponent'); -const codegenNativeCommands = require('codegenNativeCommands'); - -import type { - Int32, - BubblingEventHandler, - DirectEventHandler, -} from 'CodegenTypes'; - -import type {ViewProps} from 'ViewPropTypes'; - -interface NativeCommands { - +hotspotUpdate: (viewRef: React.Ref<'RCTView'>, x: Int32, y: Int32) => void; - +scrollTo: (viewRef: React.Ref<'RCTView'>, y: Int32, animated: boolean) => void; -} - -export type ModuleProps = $ReadOnly<{| - ...ViewProps, - // No props or events -|}>; - -export const Commands = codegenNativeCommands({ - supportedCommands: ['scrollTo'] -}); - -export default codegenNativeComponent('Module'); -`; - -const COMMANDS_DEFINED_WITHOUT_METHOD_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'; - -const codegenNativeComponent = require('codegenNativeComponent'); -const codegenNativeCommands = require('codegenNativeCommands'); - -import type { - Int32, - BubblingEventHandler, - DirectEventHandler, -} from 'CodegenTypes'; - -import type {ViewProps} from 'ViewPropTypes'; - -interface NativeCommands { - +hotspotUpdate: (viewRef: React.Ref<'RCTView'>, x: Int32, y: Int32) => void; - +scrollTo: (viewRef: React.Ref<'RCTView'>, y: Int32, animated: boolean) => void; -} - -export type ModuleProps = $ReadOnly<{| - ...ViewProps, - // No props or events -|}>; - export const Commands = codegenNativeCommands(); export default codegenNativeComponent('Module'); `; - const NULLABLE_WITH_DEFAULT = ` /** * Copyright (c) Facebook, Inc. and its affiliates. @@ -275,8 +186,6 @@ export default codegenNativeComponent('Module'); module.exports = { COMMANDS_DEFINED_INLINE, COMMANDS_DEFINED_MULTIPLE_TIMES, - COMMANDS_DEFINED_WITH_MISMATCHED_METHOD_NAMES, - COMMANDS_DEFINED_WITHOUT_METHOD_NAMES, COMMANDS_DEFINED_WITHOUT_REF, NULLABLE_WITH_DEFAULT, NON_OPTIONAL_KEY_WITH_DEFAULT_VALUE, diff --git a/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/fixtures.js index babaa9dc8f8..a70b7846919 100644 --- a/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/fixtures.js +++ b/packages/react-native-codegen/src/parsers/flow/components/__test_fixtures__/fixtures.js @@ -559,9 +559,7 @@ export type ModuleProps = $ReadOnly<{| // No props or events |}>; -export const Commands = codegenNativeCommands({ - supportedCommands: ['hotspotUpdate', 'scrollTo'] -}); +export const Commands = codegenNativeCommands(); export default codegenNativeComponent('Module'); `; @@ -605,9 +603,7 @@ export type ModuleProps = $ReadOnly<{| // No props or events |}>; -export const Commands = codegenNativeCommands({ - supportedCommands: ['scrollTo'] -}); +export const Commands = codegenNativeCommands(); export default codegenNativeComponent('Module'); `; @@ -660,9 +656,7 @@ export type ModuleProps = $ReadOnly<{| onDirectEventDefinedInlineWithPaperName: DirectEventHandler, |}>; -export const Commands = codegenNativeCommands({ - supportedCommands: ['scrollTo'] -}); +export const Commands = codegenNativeCommands(); export default codegenNativeComponent('Module'); `; 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 8c57f4725c8..dc4046fc89f 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 @@ -4,10 +4,6 @@ exports[`RN Codegen Flow Parser Fails with error message COMMANDS_DEFINED_INLINE exports[`RN Codegen Flow Parser Fails with error message COMMANDS_DEFINED_MULTIPLE_TIMES 1`] = `"codegenNativeCommands may only be called once in a file"`; -exports[`RN Codegen Flow Parser Fails with error message COMMANDS_DEFINED_WITH_MISMATCHED_METHOD_NAMES 1`] = `"codegenNativeCommands expected the same supportedCommands specified in the NativeCommands interface: hotspotUpdate, scrollTo"`; - -exports[`RN Codegen Flow Parser Fails with error message COMMANDS_DEFINED_WITHOUT_METHOD_NAMES 1`] = `"codegenNativeCommands must be passed options including the supported commands"`; - exports[`RN Codegen Flow Parser Fails with error message COMMANDS_DEFINED_WITHOUT_REF 1`] = `"The first argument of method hotspotUpdate must be of type React.Ref<>"`; 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"`; diff --git a/packages/react-native-codegen/src/parsers/flow/components/index.js b/packages/react-native-codegen/src/parsers/flow/components/index.js index a9a79b6c928..c683850ede1 100644 --- a/packages/react-native-codegen/src/parsers/flow/components/index.js +++ b/packages/react-native-codegen/src/parsers/flow/components/index.js @@ -14,7 +14,7 @@ import type {ComponentSchemaBuilderConfig} from './schema.js'; const {getCommands} = require('./commands'); const {getEvents} = require('./events'); const {getProps} = require('./props'); -const {getCommandOptions, getOptions} = require('./options'); +const {getOptions} = require('./options'); const {getExtendsProps} = require('./extends'); function findComponentConfig(ast) { @@ -58,12 +58,11 @@ function findComponentConfig(ast) { const commandsTypeNames = namedExports .map(statement => { - let callExpression; let calleeName; try { - callExpression = statement.declaration.declarations[0].init; - calleeName = callExpression.callee.name; + calleeName = statement.declaration.declarations[0].init.callee.name; } catch (e) { + // Not a function call return; } @@ -71,14 +70,8 @@ function findComponentConfig(ast) { return; } - // const statement.declaration.declarations[0].init - if (callExpression.arguments.length !== 1) { - throw new Error( - 'codegenNativeCommands must be passed options including the supported commands', - ); - } - - const typeArgumentParam = callExpression.typeArguments.params[0]; + const typeArgumentParam = + statement.declaration.declarations[0].init.typeArguments.params[0]; if (typeArgumentParam.type !== 'GenericTypeAnnotation') { throw new Error( @@ -86,10 +79,7 @@ function findComponentConfig(ast) { ); } - return { - commandTypeName: typeArgumentParam.id.name, - commandOptionsExpression: callExpression.arguments[0], - }; + return typeArgumentParam.id.name; }) .filter(Boolean); @@ -99,8 +89,7 @@ function findComponentConfig(ast) { return { ...foundConfig, - commandTypeName: commandsTypeNames[0]?.commandTypeName, - commandOptionsExpression: commandsTypeNames[0]?.commandOptionsExpression, + commandTypeName: commandsTypeNames[0], }; } @@ -115,7 +104,7 @@ function getPropProperties(propsTypeName, types) { } } -function getCommandProperties(commandTypeName, types, commandOptions) { +function getCommandProperties(commandTypeName, types) { if (commandTypeName == null) { return []; } @@ -130,39 +119,13 @@ function getCommandProperties(commandTypeName, types, commandOptions) { ); } - let properties; try { - properties = typeAlias.body.properties; + return typeAlias.body.properties; } catch (e) { throw new Error( `Failed to find type definition for "${commandTypeName}", please check that you have a valid codegen flow file`, ); } - - const flowPropertyNames = properties - .map(property => property?.key?.name) - .filter(Boolean); - - if (commandOptions == null || commandOptions.supportedCommands == null) { - throw new Error( - 'codegenNativeCommands must be given an options object with supportedCommands array', - ); - } - - if ( - commandOptions.supportedCommands.length !== flowPropertyNames.length || - !commandOptions.supportedCommands.every(supportedCommand => - flowPropertyNames.includes(supportedCommand), - ) - ) { - throw new Error( - `codegenNativeCommands expected the same supportedCommands specified in the ${commandTypeName} interface: ${flowPropertyNames.join( - ', ', - )}`, - ); - } - - return properties; } // $FlowFixMe there's no flowtype for AST @@ -171,18 +134,11 @@ function processComponent(ast, types): ComponentSchemaBuilderConfig { componentName, propsTypeName, commandTypeName, - commandOptionsExpression, optionsExpression, } = findComponentConfig(ast); const propProperties = getPropProperties(propsTypeName, types); - const commandOptions = getCommandOptions(commandOptionsExpression); - - const commandProperties = getCommandProperties( - commandTypeName, - types, - commandOptions, - ); + const commandProperties = getCommandProperties(commandTypeName, types); const extendsProps = getExtendsProps(propProperties); const options = getOptions(optionsExpression); diff --git a/packages/react-native-codegen/src/parsers/flow/components/options.js b/packages/react-native-codegen/src/parsers/flow/components/options.js index 9419fe35c6d..23ada1a6be1 100644 --- a/packages/react-native-codegen/src/parsers/flow/components/options.js +++ b/packages/react-native-codegen/src/parsers/flow/components/options.js @@ -15,37 +15,6 @@ import type {OptionsShape} from '../../../CodegenSchema.js'; // $FlowFixMe there's no flowtype for ASTs type OptionsAST = Object; -export type CommandOptions = $ReadOnly<{| - supportedCommands: $ReadOnlyArray, -|}>; - -function getCommandOptions( - commandOptionsExpression: OptionsAST, -): ?CommandOptions { - if (commandOptionsExpression == null) { - return null; - } - - let foundOptions; - try { - foundOptions = commandOptionsExpression.properties.reduce( - (options, prop) => { - options[prop.key.name] = (prop?.value?.elements || []).map( - element => element?.value, - ); - return options; - }, - {}, - ); - } catch (e) { - throw new Error( - 'Failed to parse command options, please check that they are defined correctly', - ); - } - - return foundOptions; -} - function getOptions(optionsExpression: OptionsAST): ?OptionsShape { if (!optionsExpression) { return null; @@ -75,6 +44,5 @@ function getOptions(optionsExpression: OptionsAST): ?OptionsShape { } module.exports = { - getCommandOptions, getOptions, };