Skip to content

Commit dd6d57e

Browse files
javachefacebook-github-bot
authored andcommitted
Fix cxx codegen handling of optional return types (facebook#36581)
Summary: Pull Request resolved: facebook#36581 Found that the current codegen did not properly handle a return type of `?bool` because the branch of `constexpr (is_optional_v<T>)` assumed that T was always a JSI value that needed conversion, and `supportsToJs<bool, bool>` is false. Changelog: [General][Fixed] Issue with TurboModule C++ codegen with optional return types Reviewed By: christophpurrer Differential Revision: D44302352 fbshipit-source-id: 0863de06da4e5e3c18f8a1ced7179d76d8e87b99
1 parent da027dd commit dd6d57e

File tree

5 files changed

+13
-7
lines changed

5 files changed

+13
-7
lines changed

packages/react-native/ReactCommon/react/bridging/Class.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ T callFromJs(
5050
rt, fromJs<Args>(rt, std::forward<JSArgs>(args), jsInvoker)...),
5151
jsInvoker);
5252

53-
} else if constexpr (is_optional_v<T>) {
53+
} else if constexpr (is_optional_jsi_v<T>) {
5454
static_assert(
5555
is_optional_v<R>
5656
? supportsToJs<typename R::value_type, typename T::value_type>
@@ -70,10 +70,8 @@ T callFromJs(
7070
}
7171

7272
return convert(rt, std::move(result));
73-
7473
} else {
7574
static_assert(std::is_convertible_v<R, T>, "Incompatible return type");
76-
7775
return (instance->*method)(
7876
rt, fromJs<Args>(rt, std::forward<JSArgs>(args), jsInvoker)...);
7977
}

packages/react-native/ReactCommon/react/bridging/Convert.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ struct is_optional<std::optional<T>> : std::true_type {};
3333
template <typename T>
3434
inline constexpr bool is_optional_v = is_optional<T>::value;
3535

36+
template <typename T, typename = void>
37+
inline constexpr bool is_optional_jsi_v = false;
38+
39+
template <typename T>
40+
inline constexpr bool
41+
is_optional_jsi_v<T, typename std::enable_if_t<is_optional_v<T>>> =
42+
is_jsi_v<typename T::value_type>;
43+
3644
template <typename T>
3745
struct Converter;
3846

packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,10 @@ AsyncPromise<std::string> NativeCxxModuleExample::getValueWithPromise(
117117
return promise;
118118
}
119119

120-
bool NativeCxxModuleExample::getWithWithOptionalArgs(
120+
std::optional<bool> NativeCxxModuleExample::getWithWithOptionalArgs(
121121
jsi::Runtime &rt,
122122
std::optional<bool> optionalArg) {
123-
return optionalArg.value_or(false);
123+
return optionalArg;
124124
}
125125

126126
void NativeCxxModuleExample::voidFunc(jsi::Runtime &rt) {

packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class NativeCxxModuleExample
120120

121121
AsyncPromise<std::string> getValueWithPromise(jsi::Runtime &rt, bool error);
122122

123-
bool getWithWithOptionalArgs(
123+
std::optional<bool> getWithWithOptionalArgs(
124124
jsi::Runtime &rt,
125125
std::optional<bool> optionalArg);
126126

packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export interface Spec extends TurboModule {
7070
+getValue: (x: number, y: string, z: ObjectStruct) => ValueStruct;
7171
+getValueWithCallback: (callback: (value: string) => void) => void;
7272
+getValueWithPromise: (error: boolean) => Promise<string>;
73-
+getWithWithOptionalArgs: (optionalArg?: boolean) => boolean;
73+
+getWithWithOptionalArgs: (optionalArg?: boolean) => ?boolean;
7474
+voidFunc: () => void;
7575
+emitCustomDeviceEvent: (eventName: string) => void;
7676
}

0 commit comments

Comments
 (0)