From 8183afeb815cddb92ec006f6c7c5fa08ece2867a Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Thu, 14 Dec 2023 06:54:49 -0800 Subject: [PATCH] Use enum classes in C++ Turbo Modules (#41923) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/41923 Changelog: [Internal][BREAKING] Use C++ enum classes in C++ Turbo Modules Problem: Using **C styles** `enums` can easily cause compiliation errors if symbol names collide. This code does not compile: ``` enum CustomEnumInt { A = 23, B = 42 }; static int A = 22; ``` This **C++ code**, using `enum classes` compiles: ``` enum class CustomEnumInt : int32_t { A = 23, B = 42 }; static int A = 22; ``` Reviewed By: rshest Differential Revision: D52098598 fbshipit-source-id: c919bd2e41970c83a032fec91b0537cd6fae8397 --- .../GenerateModuleH-test.js.snap | 64 +++++++++---------- .../src/generators/modules/GenerateModuleH.js | 8 +-- .../GenerateModuleH-test.js.snap | 36 +++++------ .../NativeCxxModuleExample.h | 5 +- 4 files changed, 55 insertions(+), 58 deletions(-) diff --git a/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleH-test.js.snap b/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleH-test.js.snap index 58da463847e..5473654dc10 100644 --- a/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleH-test.js.snap +++ b/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleH-test.js.snap @@ -194,7 +194,7 @@ private: #pragma mark - NativeEnumTurboModuleStatusRegularEnum -enum NativeEnumTurboModuleStatusRegularEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusRegularEnum { Active, Paused, Off }; template <> struct Bridging { @@ -226,7 +226,7 @@ struct Bridging { #pragma mark - NativeEnumTurboModuleStatusStrEnum -enum NativeEnumTurboModuleStatusStrEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusStrEnum { Active, Paused, Off }; template <> struct Bridging { @@ -258,7 +258,7 @@ struct Bridging { #pragma mark - NativeEnumTurboModuleStatusNumEnum -enum NativeEnumTurboModuleStatusNumEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusNumEnum { Active, Paused, Off }; template <> struct Bridging { @@ -290,7 +290,7 @@ struct Bridging { #pragma mark - NativeEnumTurboModuleStatusFractionEnum -enum NativeEnumTurboModuleStatusFractionEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusFractionEnum { Active, Paused, Off }; template <> struct Bridging { @@ -401,11 +401,11 @@ struct [[deprecated(\\"Use NativeEnumTurboModuleStateTypeWithEnumsBridging inste return bridging::toJs(rt, value); } - static int numToJs(jsi::Runtime &rt, P3 value) { + static jsi::Value numToJs(jsi::Runtime &rt, P3 value) { return bridging::toJs(rt, value); } - static double fractionToJs(jsi::Runtime &rt, P4 value) { + static jsi::Value fractionToJs(jsi::Runtime &rt, P4 value) { return bridging::toJs(rt, value); } #endif @@ -510,11 +510,11 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging { return bridging::toJs(rt, value); } - static int numToJs(jsi::Runtime &rt, decltype(types.num) value) { + static jsi::Value numToJs(jsi::Runtime &rt, decltype(types.num) value) { return bridging::toJs(rt, value); } - static double fractionToJs(jsi::Runtime &rt, decltype(types.fraction) value) { + static jsi::Value fractionToJs(jsi::Runtime &rt, decltype(types.fraction) value) { return bridging::toJs(rt, value); } #endif @@ -540,9 +540,9 @@ protected: public: virtual jsi::String getStatusRegular(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::String getStatusStr(jsi::Runtime &rt, jsi::Object statusProp) = 0; - virtual int getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) = 0; - virtual double getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) = 0; - virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, int c, double d) = 0; + virtual jsi::Value getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) = 0; + virtual jsi::Value getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) = 0; + virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c, jsi::Value d) = 0; virtual jsi::Object getStateTypeWithEnums(jsi::Runtime &rt, jsi::Object paramOfTypeWithEnums) = 0; }; @@ -583,23 +583,23 @@ private: return bridging::callFromJs( rt, &T::getStatusStr, jsInvoker_, instance_, std::move(statusProp)); } - int getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) override { + jsi::Value getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) override { static_assert( bridging::getParameterCount(&T::getStatusNum) == 2, \\"Expected getStatusNum(...) to have 2 parameters\\"); - return bridging::callFromJs( + return bridging::callFromJs( rt, &T::getStatusNum, jsInvoker_, instance_, std::move(statusProp)); } - double getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) override { + jsi::Value getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) override { static_assert( bridging::getParameterCount(&T::getStatusFraction) == 2, \\"Expected getStatusFraction(...) to have 2 parameters\\"); - return bridging::callFromJs( + return bridging::callFromJs( rt, &T::getStatusFraction, jsInvoker_, instance_, std::move(statusProp)); } - jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, int c, double d) override { + jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c, jsi::Value d) override { static_assert( bridging::getParameterCount(&T::getStateType) == 5, \\"Expected getStateType(...) to have 5 parameters\\"); @@ -2542,7 +2542,7 @@ private: #pragma mark - NativeEnumTurboModuleStatusRegularEnum -enum NativeEnumTurboModuleStatusRegularEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusRegularEnum { Active, Paused, Off }; template <> struct Bridging { @@ -2574,7 +2574,7 @@ struct Bridging { #pragma mark - NativeEnumTurboModuleStatusStrEnum -enum NativeEnumTurboModuleStatusStrEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusStrEnum { Active, Paused, Off }; template <> struct Bridging { @@ -2606,7 +2606,7 @@ struct Bridging { #pragma mark - NativeEnumTurboModuleStatusNumEnum -enum NativeEnumTurboModuleStatusNumEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusNumEnum { Active, Paused, Off }; template <> struct Bridging { @@ -2638,7 +2638,7 @@ struct Bridging { #pragma mark - NativeEnumTurboModuleStatusFractionEnum -enum NativeEnumTurboModuleStatusFractionEnum { Active, Paused, Off }; +enum class NativeEnumTurboModuleStatusFractionEnum { Active, Paused, Off }; template <> struct Bridging { @@ -2749,11 +2749,11 @@ struct [[deprecated(\\"Use NativeEnumTurboModuleStateTypeWithEnumsBridging inste return bridging::toJs(rt, value); } - static int numToJs(jsi::Runtime &rt, P3 value) { + static jsi::Value numToJs(jsi::Runtime &rt, P3 value) { return bridging::toJs(rt, value); } - static double fractionToJs(jsi::Runtime &rt, P4 value) { + static jsi::Value fractionToJs(jsi::Runtime &rt, P4 value) { return bridging::toJs(rt, value); } #endif @@ -2858,11 +2858,11 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging { return bridging::toJs(rt, value); } - static int numToJs(jsi::Runtime &rt, decltype(types.num) value) { + static jsi::Value numToJs(jsi::Runtime &rt, decltype(types.num) value) { return bridging::toJs(rt, value); } - static double fractionToJs(jsi::Runtime &rt, decltype(types.fraction) value) { + static jsi::Value fractionToJs(jsi::Runtime &rt, decltype(types.fraction) value) { return bridging::toJs(rt, value); } #endif @@ -2888,9 +2888,9 @@ protected: public: virtual jsi::String getStatusRegular(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::String getStatusStr(jsi::Runtime &rt, jsi::Object statusProp) = 0; - virtual int getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) = 0; - virtual double getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) = 0; - virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, int c, double d) = 0; + virtual jsi::Value getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) = 0; + virtual jsi::Value getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) = 0; + virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c, jsi::Value d) = 0; virtual jsi::Object getStateTypeWithEnums(jsi::Runtime &rt, jsi::Object paramOfTypeWithEnums) = 0; }; @@ -2931,23 +2931,23 @@ private: return bridging::callFromJs( rt, &T::getStatusStr, jsInvoker_, instance_, std::move(statusProp)); } - int getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) override { + jsi::Value getStatusNum(jsi::Runtime &rt, jsi::Object statusProp) override { static_assert( bridging::getParameterCount(&T::getStatusNum) == 2, \\"Expected getStatusNum(...) to have 2 parameters\\"); - return bridging::callFromJs( + return bridging::callFromJs( rt, &T::getStatusNum, jsInvoker_, instance_, std::move(statusProp)); } - double getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) override { + jsi::Value getStatusFraction(jsi::Runtime &rt, jsi::Object statusProp) override { static_assert( bridging::getParameterCount(&T::getStatusFraction) == 2, \\"Expected getStatusFraction(...) to have 2 parameters\\"); - return bridging::callFromJs( + return bridging::callFromJs( rt, &T::getStatusFraction, jsInvoker_, instance_, std::move(statusProp)); } - jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, int c, double d) override { + jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c, jsi::Value d) override { static_assert( bridging::getParameterCount(&T::getStateType) == 5, \\"Expected getStateType(...) to have 5 parameters\\"); diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js index f95bfec694c..f351cf02fa6 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js @@ -180,11 +180,7 @@ function translatePrimitiveJSTypeToCpp( case 'EnumDeclaration': switch (realTypeAnnotation.memberType) { case 'NumberTypeAnnotation': - return getAreEnumMembersInteger( - enumMap[realTypeAnnotation.name].members, - ) - ? wrap('int') - : wrap('double'); + return wrap('jsi::Value'); case 'StringTypeAnnotation': return wrap('jsi::String'); default: @@ -466,7 +462,7 @@ const EnumTemplate = ({ return ` #pragma mark - ${enumName} -enum ${enumName} { ${values} }; +enum class ${enumName} { ${values} }; template <> struct Bridging<${enumName}> { 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 60be24c0acf..8fcc178ac21 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 @@ -206,7 +206,7 @@ namespace facebook::react { #pragma mark - SampleTurboModuleCxxEnumInt -enum SampleTurboModuleCxxEnumInt { IA, IB }; +enum class SampleTurboModuleCxxEnumInt { IA, IB }; template <> struct Bridging { @@ -234,7 +234,7 @@ struct Bridging { #pragma mark - SampleTurboModuleCxxEnumFloat -enum SampleTurboModuleCxxEnumFloat { FA, FB }; +enum class SampleTurboModuleCxxEnumFloat { FA, FB }; template <> struct Bridging { @@ -262,7 +262,7 @@ struct Bridging { #pragma mark - SampleTurboModuleCxxEnumNone -enum SampleTurboModuleCxxEnumNone { NA, NB }; +enum class SampleTurboModuleCxxEnumNone { NA, NB }; template <> struct Bridging { @@ -290,7 +290,7 @@ struct Bridging { #pragma mark - SampleTurboModuleCxxEnumStr -enum SampleTurboModuleCxxEnumStr { SA, SB }; +enum class SampleTurboModuleCxxEnumStr { SA, SB }; template <> struct Bridging { @@ -995,12 +995,12 @@ public: virtual jsi::Array getArray(jsi::Runtime &rt, jsi::Array arg) = 0; virtual bool getBool(jsi::Runtime &rt, bool arg) = 0; virtual jsi::Object getConstants(jsi::Runtime &rt) = 0; - virtual int getCustomEnum(jsi::Runtime &rt, int arg) = 0; + virtual jsi::Value getCustomEnum(jsi::Runtime &rt, jsi::Value arg) = 0; virtual jsi::Object getCustomHostObject(jsi::Runtime &rt) = 0; virtual jsi::String consumeCustomHostObject(jsi::Runtime &rt, jsi::Object customHostObject) = 0; virtual jsi::Object getBinaryTreeNode(jsi::Runtime &rt, jsi::Object arg) = 0; virtual jsi::Object getGraphNode(jsi::Runtime &rt, jsi::Object arg) = 0; - virtual double getNumEnum(jsi::Runtime &rt, int arg) = 0; + virtual jsi::Value getNumEnum(jsi::Runtime &rt, jsi::Value arg) = 0; virtual jsi::String getStrEnum(jsi::Runtime &rt, jsi::String arg) = 0; virtual jsi::Object getMap(jsi::Runtime &rt, jsi::Object arg) = 0; virtual double getNumber(jsi::Runtime &rt, double arg) = 0; @@ -1066,12 +1066,12 @@ private: return bridging::callFromJs( rt, &T::getConstants, jsInvoker_, instance_); } - int getCustomEnum(jsi::Runtime &rt, int arg) override { + jsi::Value getCustomEnum(jsi::Runtime &rt, jsi::Value arg) override { static_assert( bridging::getParameterCount(&T::getCustomEnum) == 2, \\"Expected getCustomEnum(...) to have 2 parameters\\"); - return bridging::callFromJs( + return bridging::callFromJs( rt, &T::getCustomEnum, jsInvoker_, instance_, std::move(arg)); } jsi::Object getCustomHostObject(jsi::Runtime &rt) override { @@ -1106,12 +1106,12 @@ private: return bridging::callFromJs( rt, &T::getGraphNode, jsInvoker_, instance_, std::move(arg)); } - double getNumEnum(jsi::Runtime &rt, int arg) override { + jsi::Value getNumEnum(jsi::Runtime &rt, jsi::Value arg) override { static_assert( bridging::getParameterCount(&T::getNumEnum) == 2, \\"Expected getNumEnum(...) to have 2 parameters\\"); - return bridging::callFromJs( + return bridging::callFromJs( rt, &T::getNumEnum, jsInvoker_, instance_, std::move(arg)); } jsi::String getStrEnum(jsi::Runtime &rt, jsi::String arg) override { @@ -2598,7 +2598,7 @@ namespace facebook::react { #pragma mark - SampleTurboModuleNumEnum -enum SampleTurboModuleNumEnum { ONE, TWO }; +enum class SampleTurboModuleNumEnum { ONE, TWO }; template <> struct Bridging { @@ -2626,7 +2626,7 @@ struct Bridging { #pragma mark - SampleTurboModuleFloatEnum -enum SampleTurboModuleFloatEnum { POINT_ZERO, POINT_ONE, POINT_TWO }; +enum class SampleTurboModuleFloatEnum { POINT_ZERO, POINT_ONE, POINT_TWO }; template <> struct Bridging { @@ -2658,7 +2658,7 @@ struct Bridging { #pragma mark - SampleTurboModuleStringEnum -enum SampleTurboModuleStringEnum { HELLO, GoodBye }; +enum class SampleTurboModuleStringEnum { HELLO, GoodBye }; template <> struct Bridging { @@ -2697,11 +2697,11 @@ public: virtual jsi::Object getObject(jsi::Runtime &rt, jsi::Object arg) = 0; virtual double getRootTag(jsi::Runtime &rt, double arg) = 0; virtual jsi::Object getValue(jsi::Runtime &rt, double x, jsi::String y, jsi::Object z) = 0; - virtual int getEnumReturn(jsi::Runtime &rt) = 0; + virtual jsi::Value getEnumReturn(jsi::Runtime &rt) = 0; virtual void getValueWithCallback(jsi::Runtime &rt, jsi::Function callback) = 0; virtual jsi::Value getValueWithPromise(jsi::Runtime &rt, bool error) = 0; virtual jsi::Value getValueWithOptionalArg(jsi::Runtime &rt, std::optional parameter) = 0; - virtual jsi::String getEnums(jsi::Runtime &rt, int enumInt, double enumFloat, jsi::String enumString) = 0; + virtual jsi::String getEnums(jsi::Runtime &rt, jsi::Value enumInt, jsi::Value enumFloat, jsi::String enumString) = 0; }; @@ -2797,12 +2797,12 @@ private: return bridging::callFromJs( rt, &T::getValue, jsInvoker_, instance_, std::move(x), std::move(y), std::move(z)); } - int getEnumReturn(jsi::Runtime &rt) override { + jsi::Value getEnumReturn(jsi::Runtime &rt) override { static_assert( bridging::getParameterCount(&T::getEnumReturn) == 1, \\"Expected getEnumReturn(...) to have 1 parameters\\"); - return bridging::callFromJs( + return bridging::callFromJs( rt, &T::getEnumReturn, jsInvoker_, instance_); } void getValueWithCallback(jsi::Runtime &rt, jsi::Function callback) override { @@ -2829,7 +2829,7 @@ private: return bridging::callFromJs( rt, &T::getValueWithOptionalArg, jsInvoker_, instance_, std::move(parameter)); } - jsi::String getEnums(jsi::Runtime &rt, int enumInt, double enumFloat, jsi::String enumString) override { + jsi::String getEnums(jsi::Runtime &rt, jsi::Value enumInt, jsi::Value enumFloat, jsi::String enumString) override { static_assert( bridging::getParameterCount(&T::getEnums) == 4, \\"Expected getEnums(...) to have 4 parameters\\"); diff --git a/packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.h b/packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.h index aaec3839fcb..9017ab9f7b6 100644 --- a/packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.h +++ b/packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.h @@ -47,11 +47,12 @@ struct Bridging : NativeCxxModuleExampleCxxValueStructBridging {}; #pragma mark - enums -enum CustomEnumInt { A = 23, B = 42 }; +enum class CustomEnumInt : int32_t { A = 23, B = 42 }; template <> struct Bridging { - static CustomEnumInt fromJs(jsi::Runtime& rt, int32_t value) { + static CustomEnumInt fromJs(jsi::Runtime& rt, jsi::Value rawValue) { + auto value = static_cast(rawValue.asNumber()); if (value == 23) { return CustomEnumInt::A; } else if (value == 42) {