From 011561bd8cd35675c3ef208187aa9c6fcaeff95f Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Fri, 16 Dec 2022 04:26:43 -0800 Subject: [PATCH] Make C++ struct generator type-safe (#35656) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/35656 Changelog: [Internal] ## Change: https://github.com/facebook/react-native/pull/35265 added a struct generator - but it is not type safe :-( E.g. you can write a TM Spec type as: ``` export type CustomType = { key: string; enabled: boolean; time?: number; } ``` And a C++ type as: ``` using CustomType = NativeSampleModuleBaseCustomType>; template <> struct Bridging : NativeSampleModuleBaseCustomTypeBridging> {}; ``` and it will still compile :-( - which should not. The reason is that the generated structs don't validate the members type :-( ``` template struct NativeSampleModuleBaseCustomType { P0 key; P1 enabled; P2 time; bool operator==(const NativeSampleModuleBaseCustomType &other) const { return key == other.key && enabled == other.enabled && time == other.time; } }; template struct NativeSampleModuleBaseCustomTypeBridging { static NativeSampleModuleBaseCustomType fromJs( jsi::Runtime &rt, const jsi::Object &value, const std::shared_ptr &jsInvoker) { NativeSampleModuleBaseCustomType result{ bridging::fromJs(rt, value.getProperty(rt, "key"), jsInvoker), bridging::fromJs(rt, value.getProperty(rt, "enabled"), jsInvoker), bridging::fromJs(rt, value.getProperty(rt, "time"), jsInvoker)}; return result; } static jsi::Object toJs( jsi::Runtime &rt, const NativeSampleModuleBaseCustomType &value) { auto result = facebook::jsi::Object(rt); result.setProperty(rt, "key", bridging::toJs(rt, value.key)); result.setProperty(rt, "enabled", bridging::toJs(rt, value.enabled)); if (value.time) { result.setProperty(rt, "time", bridging::toJs(rt, value.time.value())); } keyToJs(rt, value.key); return result; } }; ``` This fixes that, by simply emitting conversion functions for each member such as ``` #ifdef DEBUG static bool keyToJs(jsi::Runtime &rt, P0 value) { return bridging::toJs(rt, value); } static double enabledToJs(jsi::Runtime &rt, P1 value) { return bridging::toJs(rt, value); } static jsi::String timeToJs(jsi::Runtime &rt, P2 value) { return bridging::toJs(rt, value); } #endif ``` Reviewed By: cipolleschi Differential Revision: D42082423 fbshipit-source-id: 5133f14e2aa8351e9bbbf614117a3d5894b17fa6 --- .../src/generators/modules/GenerateModuleH.js | 54 ++- .../GenerateModuleH-test.js.snap | 313 +++++++++++++----- 2 files changed, 257 insertions(+), 110 deletions(-) diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js index 0ed01458cb6e02..7c3e6e44b26205 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js @@ -210,6 +210,20 @@ function createStructs( const templateParameter = value.properties .map((v, i) => 'P' + i) .join(', '); + const paramemterConversion = value.properties + .map((v, i) => { + const translatedParam = translatePrimitiveJSTypeToCpp( + v.typeAnnotation, + false, + typeName => + `Unsupported type for param "${v.name}". Found: ${typeName}`, + resolveAlias, + ); + return ` static ${translatedParam} ${v.name}ToJs(jsi::Runtime &rt, P${i} value) { + return bridging::toJs(rt, value); + }`; + }) + .join('\n'); return `#pragma mark - ${structName} template <${templateParameterWithTypename}> @@ -238,25 +252,29 @@ ${value.properties return result; } +#ifdef DEBUG +${paramemterConversion} +#endif + static jsi::Object toJs( - jsi::Runtime &rt, - const ${structName}<${templateParameter}> &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); -${value.properties - .map((v, i) => { - if (v.optional) { - return ` if (value.${v.name}) { - result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}.value(), jsInvoker)); - }`; - } else { - return ` result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}, jsInvoker));`; - } - }) - .join('\n')} - return result; - } -}; + jsi::Runtime &rt, + const ${structName}<${templateParameter}> &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + ${value.properties + .map((v, i) => { + if (v.optional) { + return ` if (value.${v.name}) { + result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}.value(), jsInvoker)); + }`; + } else { + return ` result.setProperty(rt, "${v.name}", bridging::toJs(rt, value.${v.name}, jsInvoker));`; + } + }) + .join('\n')} + return result; + } + }; `; }) diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap index 6f52b6506a647c..82800da2235299 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap @@ -223,15 +223,21 @@ struct SampleTurboModuleCxxBaseObjectAliasBridging { return result; } - static jsi::Object toJs( - jsi::Runtime &rt, - const SampleTurboModuleCxxBaseObjectAlias &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - result.setProperty(rt, \\"x\\", bridging::toJs(rt, value.x, jsInvoker)); - return result; +#ifdef DEBUG + static double xToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); } -}; +#endif + + static jsi::Object toJs( + jsi::Runtime &rt, + const SampleTurboModuleCxxBaseObjectAlias &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + result.setProperty(rt, \\"x\\", bridging::toJs(rt, value.x, jsInvoker)); + return result; + } + }; class JSI_EXPORT NativeSampleTurboModuleCxxSpecJSI : public TurboModule { protected: @@ -418,25 +424,43 @@ struct AliasTurboModuleBaseOptionsBridging { return result; } +#ifdef DEBUG + static jsi::Object offsetToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); + } + static jsi::Object sizeToJs(jsi::Runtime &rt, P1 value) { + return bridging::toJs(rt, value); + } + static jsi::Object displaySizeToJs(jsi::Runtime &rt, P2 value) { + return bridging::toJs(rt, value); + } + static jsi::String resizeModeToJs(jsi::Runtime &rt, P3 value) { + return bridging::toJs(rt, value); + } + static bool allowExternalStorageToJs(jsi::Runtime &rt, P4 value) { + return bridging::toJs(rt, value); + } +#endif + static jsi::Object toJs( - jsi::Runtime &rt, - const AliasTurboModuleBaseOptions &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - result.setProperty(rt, \\"offset\\", bridging::toJs(rt, value.offset, jsInvoker)); + jsi::Runtime &rt, + const AliasTurboModuleBaseOptions &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + result.setProperty(rt, \\"offset\\", bridging::toJs(rt, value.offset, jsInvoker)); result.setProperty(rt, \\"size\\", bridging::toJs(rt, value.size, jsInvoker)); if (value.displaySize) { - result.setProperty(rt, \\"displaySize\\", bridging::toJs(rt, value.displaySize.value(), jsInvoker)); - } + result.setProperty(rt, \\"displaySize\\", bridging::toJs(rt, value.displaySize.value(), jsInvoker)); + } if (value.resizeMode) { - result.setProperty(rt, \\"resizeMode\\", bridging::toJs(rt, value.resizeMode.value(), jsInvoker)); - } + result.setProperty(rt, \\"resizeMode\\", bridging::toJs(rt, value.resizeMode.value(), jsInvoker)); + } if (value.allowExternalStorage) { - result.setProperty(rt, \\"allowExternalStorage\\", bridging::toJs(rt, value.allowExternalStorage.value(), jsInvoker)); - } - return result; - } -}; + result.setProperty(rt, \\"allowExternalStorage\\", bridging::toJs(rt, value.allowExternalStorage.value(), jsInvoker)); + } + return result; + } + }; class JSI_EXPORT AliasTurboModuleCxxSpecJSI : public TurboModule { protected: @@ -546,22 +570,43 @@ struct CameraRollManagerBasePhotoIdentifierImageBridging { return result; } +#ifdef DEBUG + static jsi::String uriToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); + } + static double playableDurationToJs(jsi::Runtime &rt, P1 value) { + return bridging::toJs(rt, value); + } + static double widthToJs(jsi::Runtime &rt, P2 value) { + return bridging::toJs(rt, value); + } + static double heightToJs(jsi::Runtime &rt, P3 value) { + return bridging::toJs(rt, value); + } + static bool isStoredToJs(jsi::Runtime &rt, P4 value) { + return bridging::toJs(rt, value); + } + static jsi::String filenameToJs(jsi::Runtime &rt, P5 value) { + return bridging::toJs(rt, value); + } +#endif + static jsi::Object toJs( - jsi::Runtime &rt, - const CameraRollManagerBasePhotoIdentifierImage &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - result.setProperty(rt, \\"uri\\", bridging::toJs(rt, value.uri, jsInvoker)); + jsi::Runtime &rt, + const CameraRollManagerBasePhotoIdentifierImage &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + result.setProperty(rt, \\"uri\\", bridging::toJs(rt, value.uri, jsInvoker)); result.setProperty(rt, \\"playableDuration\\", bridging::toJs(rt, value.playableDuration, jsInvoker)); result.setProperty(rt, \\"width\\", bridging::toJs(rt, value.width, jsInvoker)); result.setProperty(rt, \\"height\\", bridging::toJs(rt, value.height, jsInvoker)); if (value.isStored) { - result.setProperty(rt, \\"isStored\\", bridging::toJs(rt, value.isStored.value(), jsInvoker)); - } + result.setProperty(rt, \\"isStored\\", bridging::toJs(rt, value.isStored.value(), jsInvoker)); + } result.setProperty(rt, \\"filename\\", bridging::toJs(rt, value.filename, jsInvoker)); - return result; - } -}; + return result; + } + }; #pragma mark - CameraRollManagerBasePhotoIdentifier @@ -585,15 +630,21 @@ struct CameraRollManagerBasePhotoIdentifierBridging { return result; } - static jsi::Object toJs( - jsi::Runtime &rt, - const CameraRollManagerBasePhotoIdentifier &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - result.setProperty(rt, \\"node\\", bridging::toJs(rt, value.node, jsInvoker)); - return result; +#ifdef DEBUG + static jsi::Object nodeToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); } -}; +#endif + + static jsi::Object toJs( + jsi::Runtime &rt, + const CameraRollManagerBasePhotoIdentifier &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + result.setProperty(rt, \\"node\\", bridging::toJs(rt, value.node, jsInvoker)); + return result; + } + }; #pragma mark - CameraRollManagerBasePhotoIdentifiersPage @@ -619,16 +670,25 @@ struct CameraRollManagerBasePhotoIdentifiersPageBridging { return result; } +#ifdef DEBUG + static jsi::Array edgesToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); + } + static jsi::Object page_infoToJs(jsi::Runtime &rt, P1 value) { + return bridging::toJs(rt, value); + } +#endif + static jsi::Object toJs( - jsi::Runtime &rt, - const CameraRollManagerBasePhotoIdentifiersPage &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - result.setProperty(rt, \\"edges\\", bridging::toJs(rt, value.edges, jsInvoker)); + jsi::Runtime &rt, + const CameraRollManagerBasePhotoIdentifiersPage &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + result.setProperty(rt, \\"edges\\", bridging::toJs(rt, value.edges, jsInvoker)); result.setProperty(rt, \\"page_info\\", bridging::toJs(rt, value.page_info, jsInvoker)); - return result; - } -}; + return result; + } + }; #pragma mark - CameraRollManagerBaseGetPhotosParams @@ -664,33 +724,57 @@ struct CameraRollManagerBaseGetPhotosParamsBridging { return result; } +#ifdef DEBUG + static double firstToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); + } + static jsi::String afterToJs(jsi::Runtime &rt, P1 value) { + return bridging::toJs(rt, value); + } + static jsi::String groupNameToJs(jsi::Runtime &rt, P2 value) { + return bridging::toJs(rt, value); + } + static jsi::String groupTypesToJs(jsi::Runtime &rt, P3 value) { + return bridging::toJs(rt, value); + } + static jsi::String assetTypeToJs(jsi::Runtime &rt, P4 value) { + return bridging::toJs(rt, value); + } + static double maxSizeToJs(jsi::Runtime &rt, P5 value) { + return bridging::toJs(rt, value); + } + static jsi::Array mimeTypesToJs(jsi::Runtime &rt, P6 value) { + return bridging::toJs(rt, value); + } +#endif + static jsi::Object toJs( - jsi::Runtime &rt, - const CameraRollManagerBaseGetPhotosParams &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - result.setProperty(rt, \\"first\\", bridging::toJs(rt, value.first, jsInvoker)); + jsi::Runtime &rt, + const CameraRollManagerBaseGetPhotosParams &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + result.setProperty(rt, \\"first\\", bridging::toJs(rt, value.first, jsInvoker)); if (value.after) { - result.setProperty(rt, \\"after\\", bridging::toJs(rt, value.after.value(), jsInvoker)); - } + result.setProperty(rt, \\"after\\", bridging::toJs(rt, value.after.value(), jsInvoker)); + } if (value.groupName) { - result.setProperty(rt, \\"groupName\\", bridging::toJs(rt, value.groupName.value(), jsInvoker)); - } + result.setProperty(rt, \\"groupName\\", bridging::toJs(rt, value.groupName.value(), jsInvoker)); + } if (value.groupTypes) { - result.setProperty(rt, \\"groupTypes\\", bridging::toJs(rt, value.groupTypes.value(), jsInvoker)); - } + result.setProperty(rt, \\"groupTypes\\", bridging::toJs(rt, value.groupTypes.value(), jsInvoker)); + } if (value.assetType) { - result.setProperty(rt, \\"assetType\\", bridging::toJs(rt, value.assetType.value(), jsInvoker)); - } + result.setProperty(rt, \\"assetType\\", bridging::toJs(rt, value.assetType.value(), jsInvoker)); + } if (value.maxSize) { - result.setProperty(rt, \\"maxSize\\", bridging::toJs(rt, value.maxSize.value(), jsInvoker)); - } + result.setProperty(rt, \\"maxSize\\", bridging::toJs(rt, value.maxSize.value(), jsInvoker)); + } if (value.mimeTypes) { - result.setProperty(rt, \\"mimeTypes\\", bridging::toJs(rt, value.mimeTypes.value(), jsInvoker)); - } - return result; - } -}; + result.setProperty(rt, \\"mimeTypes\\", bridging::toJs(rt, value.mimeTypes.value(), jsInvoker)); + } + return result; + } + }; class JSI_EXPORT NativeCameraRollManagerCxxSpecJSI : public TurboModule { protected: @@ -791,25 +875,43 @@ struct ExceptionsManagerBaseStackFrameBridging { return result; } +#ifdef DEBUG + static double columnToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); + } + static jsi::String fileToJs(jsi::Runtime &rt, P1 value) { + return bridging::toJs(rt, value); + } + static double lineNumberToJs(jsi::Runtime &rt, P2 value) { + return bridging::toJs(rt, value); + } + static jsi::String methodNameToJs(jsi::Runtime &rt, P3 value) { + return bridging::toJs(rt, value); + } + static bool collapseToJs(jsi::Runtime &rt, P4 value) { + return bridging::toJs(rt, value); + } +#endif + static jsi::Object toJs( - jsi::Runtime &rt, - const ExceptionsManagerBaseStackFrame &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - if (value.column) { - result.setProperty(rt, \\"column\\", bridging::toJs(rt, value.column.value(), jsInvoker)); - } + jsi::Runtime &rt, + const ExceptionsManagerBaseStackFrame &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + if (value.column) { + result.setProperty(rt, \\"column\\", bridging::toJs(rt, value.column.value(), jsInvoker)); + } result.setProperty(rt, \\"file\\", bridging::toJs(rt, value.file, jsInvoker)); if (value.lineNumber) { - result.setProperty(rt, \\"lineNumber\\", bridging::toJs(rt, value.lineNumber.value(), jsInvoker)); - } + result.setProperty(rt, \\"lineNumber\\", bridging::toJs(rt, value.lineNumber.value(), jsInvoker)); + } result.setProperty(rt, \\"methodName\\", bridging::toJs(rt, value.methodName, jsInvoker)); if (value.collapse) { - result.setProperty(rt, \\"collapse\\", bridging::toJs(rt, value.collapse.value(), jsInvoker)); - } - return result; - } -}; + result.setProperty(rt, \\"collapse\\", bridging::toJs(rt, value.collapse.value(), jsInvoker)); + } + return result; + } + }; #pragma mark - ExceptionsManagerBaseExceptionData @@ -847,12 +949,39 @@ struct ExceptionsManagerBaseExceptionDataBridging { return result; } +#ifdef DEBUG + static jsi::String messageToJs(jsi::Runtime &rt, P0 value) { + return bridging::toJs(rt, value); + } + static jsi::String originalMessageToJs(jsi::Runtime &rt, P1 value) { + return bridging::toJs(rt, value); + } + static jsi::String nameToJs(jsi::Runtime &rt, P2 value) { + return bridging::toJs(rt, value); + } + static jsi::String componentStackToJs(jsi::Runtime &rt, P3 value) { + return bridging::toJs(rt, value); + } + static jsi::Array stackToJs(jsi::Runtime &rt, P4 value) { + return bridging::toJs(rt, value); + } + static double idToJs(jsi::Runtime &rt, P5 value) { + return bridging::toJs(rt, value); + } + static bool isFatalToJs(jsi::Runtime &rt, P6 value) { + return bridging::toJs(rt, value); + } + static jsi::Object extraDataToJs(jsi::Runtime &rt, P7 value) { + return bridging::toJs(rt, value); + } +#endif + static jsi::Object toJs( - jsi::Runtime &rt, - const ExceptionsManagerBaseExceptionData &value, - const std::shared_ptr &jsInvoker) { - auto result = facebook::jsi::Object(rt); - result.setProperty(rt, \\"message\\", bridging::toJs(rt, value.message, jsInvoker)); + jsi::Runtime &rt, + const ExceptionsManagerBaseExceptionData &value, + const std::shared_ptr &jsInvoker) { + auto result = facebook::jsi::Object(rt); + result.setProperty(rt, \\"message\\", bridging::toJs(rt, value.message, jsInvoker)); result.setProperty(rt, \\"originalMessage\\", bridging::toJs(rt, value.originalMessage, jsInvoker)); result.setProperty(rt, \\"name\\", bridging::toJs(rt, value.name, jsInvoker)); result.setProperty(rt, \\"componentStack\\", bridging::toJs(rt, value.componentStack, jsInvoker)); @@ -860,11 +989,11 @@ struct ExceptionsManagerBaseExceptionDataBridging { result.setProperty(rt, \\"id\\", bridging::toJs(rt, value.id, jsInvoker)); result.setProperty(rt, \\"isFatal\\", bridging::toJs(rt, value.isFatal, jsInvoker)); if (value.extraData) { - result.setProperty(rt, \\"extraData\\", bridging::toJs(rt, value.extraData.value(), jsInvoker)); - } - return result; - } -}; + result.setProperty(rt, \\"extraData\\", bridging::toJs(rt, value.extraData.value(), jsInvoker)); + } + return result; + } + }; class JSI_EXPORT NativeExceptionsManagerCxxSpecJSI : public TurboModule { protected: