Skip to content

Commit

Permalink
Don't support float enums
Browse files Browse the repository at this point in the history 
Summary:
Changelog: [General][BREAKING] Don't support 'float' enums in Turbo Modules

- The current implementation of 'float enums' in C++ does not work as invalid results are returned.
- At potential fix could still cause rounding errors when crossing language bounaries, e.g. `4.6` can become `4.5599999942..`
- C++ enum classes don't support float: https://eel.is/c++draft/dcl.enum#2.sentence-4

> The type-specifier-seq of an enum-base shall name an integral type; any cv-qualification is ignored.

Hence removing the feature of `float enums` for now

Reviewed By: NickGerleman

Differential Revision: D52120405

fbshipit-source-id: 3685ad0629e16ff9db424ba67e07d09df6027553
  • Loading branch information
@christophpurrer @kosmydel
christophpurrer authored and kosmydel committed May 6, 2024
1 parent 9038c70 commit 93cc871
Show file tree
Hide file tree
Showing 16 changed files with 75 additions and 323 deletions.
9 changes: 0 additions & 9 deletions packages/react-native-codegen/e2e/__test_fixtures__/modules/NativeEnumTurboModule.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,21 @@ export enum StatusNumEnum {
Off = 0,
}

export enum StatusFractionEnum {
Active = 0.2,
Paused = 0.1,
Off = 0.0,
}

export type StateTypeWithEnums = {|
state: string,
regular: StatusRegularEnum,
str: StatusStrEnum,
num: StatusNumEnum,
fraction: StatusFractionEnum,
|};

export interface Spec extends TurboModule {
+getStatusRegular: (statusProp: StateType) => StatusRegularEnum;
+getStatusStr: (statusProp: StateType) => StatusStrEnum;
+getStatusNum: (statusProp: StateType) => StatusNumEnum;
+getStatusFraction: (statusProp: StateType) => StatusFractionEnum;
+getStateType: (
a: StatusRegularEnum,
b: StatusStrEnum,
c: StatusNumEnum,
d: StatusFractionEnum,
) => StateType;
+getStateTypeWithEnums: (
paramOfTypeWithEnums: StateTypeWithEnums,
Expand Down
24 changes: 4 additions & 20 deletions ...s/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleCpp-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,12 @@ static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusNum(js
count <= 0 ? throw jsi::JSError(rt, \\"Expected argument in position 0 to be passed\\") : args[0].asObject(rt)
);
}
static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusFraction(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
return static_cast<NativeEnumTurboModuleCxxSpecJSI *>(&turboModule)->getStatusFraction(
rt,
count <= 0 ? throw jsi::JSError(rt, \\"Expected argument in position 0 to be passed\\") : args[0].asObject(rt)
);
}
static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateType(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
return static_cast<NativeEnumTurboModuleCxxSpecJSI *>(&turboModule)->getStateType(
rt,
count <= 0 ? throw jsi::JSError(rt, \\"Expected argument in position 0 to be passed\\") : args[0].asString(rt),
count <= 1 ? throw jsi::JSError(rt, \\"Expected argument in position 1 to be passed\\") : args[1].asString(rt),
count <= 2 ? throw jsi::JSError(rt, \\"Expected argument in position 2 to be passed\\") : args[2].asNumber(),
count <= 3 ? throw jsi::JSError(rt, \\"Expected argument in position 3 to be passed\\") : args[3].asNumber()
count <= 2 ? throw jsi::JSError(rt, \\"Expected argument in position 2 to be passed\\") : args[2].asNumber()
);
}
static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateTypeWithEnums(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
Expand All @@ -123,8 +116,7 @@ NativeEnumTurboModuleCxxSpecJSI::NativeEnumTurboModuleCxxSpecJSI(std::shared_ptr
methodMap_[\\"getStatusRegular\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusRegular};
methodMap_[\\"getStatusStr\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusStr};
methodMap_[\\"getStatusNum\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusNum};
methodMap_[\\"getStatusFraction\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusFraction};
methodMap_[\\"getStateType\\"] = MethodMetadata {4, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateType};
methodMap_[\\"getStateType\\"] = MethodMetadata {3, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateType};
methodMap_[\\"getStateTypeWithEnums\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateTypeWithEnums};
}
static jsi::Value __hostFunction_NativeNullableTurboModuleCxxSpecJSI_getBool(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
Expand Down Expand Up @@ -917,19 +909,12 @@ static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusNum(js
count <= 0 ? throw jsi::JSError(rt, \\"Expected argument in position 0 to be passed\\") : args[0].asObject(rt)
);
}
static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusFraction(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
return static_cast<NativeEnumTurboModuleCxxSpecJSI *>(&turboModule)->getStatusFraction(
rt,
count <= 0 ? throw jsi::JSError(rt, \\"Expected argument in position 0 to be passed\\") : args[0].asObject(rt)
);
}
static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateType(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
return static_cast<NativeEnumTurboModuleCxxSpecJSI *>(&turboModule)->getStateType(
rt,
count <= 0 ? throw jsi::JSError(rt, \\"Expected argument in position 0 to be passed\\") : args[0].asString(rt),
count <= 1 ? throw jsi::JSError(rt, \\"Expected argument in position 1 to be passed\\") : args[1].asString(rt),
count <= 2 ? throw jsi::JSError(rt, \\"Expected argument in position 2 to be passed\\") : args[2].asNumber(),
count <= 3 ? throw jsi::JSError(rt, \\"Expected argument in position 3 to be passed\\") : args[3].asNumber()
count <= 2 ? throw jsi::JSError(rt, \\"Expected argument in position 2 to be passed\\") : args[2].asNumber()
);
}
static jsi::Value __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateTypeWithEnums(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
Expand All @@ -944,8 +929,7 @@ NativeEnumTurboModuleCxxSpecJSI::NativeEnumTurboModuleCxxSpecJSI(std::shared_ptr
methodMap_[\\"getStatusRegular\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusRegular};
methodMap_[\\"getStatusStr\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusStr};
methodMap_[\\"getStatusNum\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusNum};
methodMap_[\\"getStatusFraction\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStatusFraction};
methodMap_[\\"getStateType\\"] = MethodMetadata {4, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateType};
methodMap_[\\"getStateType\\"] = MethodMetadata {3, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateType};
methodMap_[\\"getStateTypeWithEnums\\"] = MethodMetadata {1, __hostFunction_NativeEnumTurboModuleCxxSpecJSI_getStateTypeWithEnums};
}
static jsi::Value __hostFunction_NativeNullableTurboModuleCxxSpecJSI_getBool(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
Expand Down
128 changes: 16 additions & 112 deletions ...ges/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleH-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,38 +287,6 @@ struct Bridging<NativeEnumTurboModuleStatusNumEnum> {
}
}
};

#pragma mark - NativeEnumTurboModuleStatusFractionEnum

enum class NativeEnumTurboModuleStatusFractionEnum { Active, Paused, Off };

template <>
struct Bridging<NativeEnumTurboModuleStatusFractionEnum> {
static NativeEnumTurboModuleStatusFractionEnum fromJs(jsi::Runtime &rt, const jsi::Value &rawValue) {
double value = (double)rawValue.asNumber();
if (value == 0.2f) {
return NativeEnumTurboModuleStatusFractionEnum::Active;
} else if (value == 0.1f) {
return NativeEnumTurboModuleStatusFractionEnum::Paused;
} else if (value == 0f) {
return NativeEnumTurboModuleStatusFractionEnum::Off;
} else {
throw jsi::JSError(rt, \\"No appropriate enum member found for value\\");
}
}

static jsi::Value toJs(jsi::Runtime &rt, NativeEnumTurboModuleStatusFractionEnum value) {
if (value == NativeEnumTurboModuleStatusFractionEnum::Active) {
return bridging::toJs(rt, 0.2f);
} else if (value == NativeEnumTurboModuleStatusFractionEnum::Paused) {
return bridging::toJs(rt, 0.1f);
} else if (value == NativeEnumTurboModuleStatusFractionEnum::Off) {
return bridging::toJs(rt, 0f);
} else {
throw jsi::JSError(rt, \\"No appropriate enum member found for enum value\\");
}
}
};

#pragma mark - NativeEnumTurboModuleStateType

Expand Down Expand Up @@ -363,15 +331,14 @@ struct NativeEnumTurboModuleStateTypeBridging {

#pragma mark - NativeEnumTurboModuleStateTypeWithEnums

template <typename P0, typename P1, typename P2, typename P3, typename P4>
template <typename P0, typename P1, typename P2, typename P3>
struct NativeEnumTurboModuleStateTypeWithEnums {
P0 state;
P1 regular;
P2 str;
P3 num;
P4 fraction;
bool operator==(const NativeEnumTurboModuleStateTypeWithEnums &other) const {
return state == other.state && regular == other.regular && str == other.str && num == other.num && fraction == other.fraction;
return state == other.state && regular == other.regular && str == other.str && num == other.num;
}
};

Expand All @@ -387,8 +354,7 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging {
bridging::fromJs<decltype(types.state)>(rt, value.getProperty(rt, \\"state\\"), jsInvoker),
bridging::fromJs<decltype(types.regular)>(rt, value.getProperty(rt, \\"regular\\"), jsInvoker),
bridging::fromJs<decltype(types.str)>(rt, value.getProperty(rt, \\"str\\"), jsInvoker),
bridging::fromJs<decltype(types.num)>(rt, value.getProperty(rt, \\"num\\"), jsInvoker),
bridging::fromJs<decltype(types.fraction)>(rt, value.getProperty(rt, \\"fraction\\"), jsInvoker)};
bridging::fromJs<decltype(types.num)>(rt, value.getProperty(rt, \\"num\\"), jsInvoker)};
return result;
}

Expand All @@ -408,10 +374,6 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging {
static jsi::Value numToJs(jsi::Runtime &rt, decltype(types.num) value) {
return bridging::toJs(rt, value);
}

static jsi::Value fractionToJs(jsi::Runtime &rt, decltype(types.fraction) value) {
return bridging::toJs(rt, value);
}
#endif

static jsi::Object toJs(
Expand All @@ -423,7 +385,6 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging {
result.setProperty(rt, \\"regular\\", bridging::toJs(rt, value.regular, jsInvoker));
result.setProperty(rt, \\"str\\", bridging::toJs(rt, value.str, jsInvoker));
result.setProperty(rt, \\"num\\", bridging::toJs(rt, value.num, jsInvoker));
result.setProperty(rt, \\"fraction\\", bridging::toJs(rt, value.fraction, jsInvoker));
return result;
}
};
Expand All @@ -436,8 +397,7 @@ public:
virtual jsi::String getStatusRegular(jsi::Runtime &rt, jsi::Object statusProp) = 0;
virtual jsi::String getStatusStr(jsi::Runtime &rt, jsi::Object statusProp) = 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 getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c) = 0;
virtual jsi::Object getStateTypeWithEnums(jsi::Runtime &rt, jsi::Object paramOfTypeWithEnums) = 0;

};
Expand Down Expand Up @@ -486,21 +446,13 @@ private:
return bridging::callFromJs<jsi::Value>(
rt, &T::getStatusNum, jsInvoker_, instance_, std::move(statusProp));
}
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<jsi::Value>(
rt, &T::getStatusFraction, jsInvoker_, instance_, std::move(statusProp));
}
jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c, jsi::Value d) override {
jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c) override {
static_assert(
bridging::getParameterCount(&T::getStateType) == 5,
\\"Expected getStateType(...) to have 5 parameters\\");
bridging::getParameterCount(&T::getStateType) == 4,
\\"Expected getStateType(...) to have 4 parameters\\");

return bridging::callFromJs<jsi::Object>(
rt, &T::getStateType, jsInvoker_, instance_, std::move(a), std::move(b), std::move(c), std::move(d));
rt, &T::getStateType, jsInvoker_, instance_, std::move(a), std::move(b), std::move(c));
}
jsi::Object getStateTypeWithEnums(jsi::Runtime &rt, jsi::Object paramOfTypeWithEnums) override {
static_assert(
Expand Down Expand Up @@ -2289,38 +2241,6 @@ struct Bridging<NativeEnumTurboModuleStatusNumEnum> {
}
}
};

#pragma mark - NativeEnumTurboModuleStatusFractionEnum

enum class NativeEnumTurboModuleStatusFractionEnum { Active, Paused, Off };

template <>
struct Bridging<NativeEnumTurboModuleStatusFractionEnum> {
static NativeEnumTurboModuleStatusFractionEnum fromJs(jsi::Runtime &rt, const jsi::Value &rawValue) {
double value = (double)rawValue.asNumber();
if (value == 0.2f) {
return NativeEnumTurboModuleStatusFractionEnum::Active;
} else if (value == 0.1f) {
return NativeEnumTurboModuleStatusFractionEnum::Paused;
} else if (value == 0f) {
return NativeEnumTurboModuleStatusFractionEnum::Off;
} else {
throw jsi::JSError(rt, \\"No appropriate enum member found for value\\");
}
}

static jsi::Value toJs(jsi::Runtime &rt, NativeEnumTurboModuleStatusFractionEnum value) {
if (value == NativeEnumTurboModuleStatusFractionEnum::Active) {
return bridging::toJs(rt, 0.2f);
} else if (value == NativeEnumTurboModuleStatusFractionEnum::Paused) {
return bridging::toJs(rt, 0.1f);
} else if (value == NativeEnumTurboModuleStatusFractionEnum::Off) {
return bridging::toJs(rt, 0f);
} else {
throw jsi::JSError(rt, \\"No appropriate enum member found for enum value\\");
}
}
};

#pragma mark - NativeEnumTurboModuleStateType

Expand Down Expand Up @@ -2365,15 +2285,14 @@ struct NativeEnumTurboModuleStateTypeBridging {

#pragma mark - NativeEnumTurboModuleStateTypeWithEnums

template <typename P0, typename P1, typename P2, typename P3, typename P4>
template <typename P0, typename P1, typename P2, typename P3>
struct NativeEnumTurboModuleStateTypeWithEnums {
P0 state;
P1 regular;
P2 str;
P3 num;
P4 fraction;
bool operator==(const NativeEnumTurboModuleStateTypeWithEnums &other) const {
return state == other.state && regular == other.regular && str == other.str && num == other.num && fraction == other.fraction;
return state == other.state && regular == other.regular && str == other.str && num == other.num;
}
};

Expand All @@ -2389,8 +2308,7 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging {
bridging::fromJs<decltype(types.state)>(rt, value.getProperty(rt, \\"state\\"), jsInvoker),
bridging::fromJs<decltype(types.regular)>(rt, value.getProperty(rt, \\"regular\\"), jsInvoker),
bridging::fromJs<decltype(types.str)>(rt, value.getProperty(rt, \\"str\\"), jsInvoker),
bridging::fromJs<decltype(types.num)>(rt, value.getProperty(rt, \\"num\\"), jsInvoker),
bridging::fromJs<decltype(types.fraction)>(rt, value.getProperty(rt, \\"fraction\\"), jsInvoker)};
bridging::fromJs<decltype(types.num)>(rt, value.getProperty(rt, \\"num\\"), jsInvoker)};
return result;
}

Expand All @@ -2410,10 +2328,6 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging {
static jsi::Value numToJs(jsi::Runtime &rt, decltype(types.num) value) {
return bridging::toJs(rt, value);
}

static jsi::Value fractionToJs(jsi::Runtime &rt, decltype(types.fraction) value) {
return bridging::toJs(rt, value);
}
#endif

static jsi::Object toJs(
Expand All @@ -2425,7 +2339,6 @@ struct NativeEnumTurboModuleStateTypeWithEnumsBridging {
result.setProperty(rt, \\"regular\\", bridging::toJs(rt, value.regular, jsInvoker));
result.setProperty(rt, \\"str\\", bridging::toJs(rt, value.str, jsInvoker));
result.setProperty(rt, \\"num\\", bridging::toJs(rt, value.num, jsInvoker));
result.setProperty(rt, \\"fraction\\", bridging::toJs(rt, value.fraction, jsInvoker));
return result;
}
};
Expand All @@ -2438,8 +2351,7 @@ public:
virtual jsi::String getStatusRegular(jsi::Runtime &rt, jsi::Object statusProp) = 0;
virtual jsi::String getStatusStr(jsi::Runtime &rt, jsi::Object statusProp) = 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 getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c) = 0;
virtual jsi::Object getStateTypeWithEnums(jsi::Runtime &rt, jsi::Object paramOfTypeWithEnums) = 0;

};
Expand Down Expand Up @@ -2488,21 +2400,13 @@ private:
return bridging::callFromJs<jsi::Value>(
rt, &T::getStatusNum, jsInvoker_, instance_, std::move(statusProp));
}
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<jsi::Value>(
rt, &T::getStatusFraction, jsInvoker_, instance_, std::move(statusProp));
}
jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c, jsi::Value d) override {
jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, jsi::String b, jsi::Value c) override {
static_assert(
bridging::getParameterCount(&T::getStateType) == 5,
\\"Expected getStateType(...) to have 5 parameters\\");
bridging::getParameterCount(&T::getStateType) == 4,
\\"Expected getStateType(...) to have 4 parameters\\");

return bridging::callFromJs<jsi::Object>(
rt, &T::getStateType, jsInvoker_, instance_, std::move(a), std::move(b), std::move(c), std::move(d));
rt, &T::getStateType, jsInvoker_, instance_, std::move(a), std::move(b), std::move(c));
}
jsi::Object getStateTypeWithEnums(jsi::Runtime &rt, jsi::Object paramOfTypeWithEnums) override {
static_assert(
Expand Down
Loading

0 comments on commit 93cc871

Please sign in to comment.