Define precise return types for toJs conversions, instead of generic jsi::Value (facebook#51224)

Summary:
Pull Request resolved: facebook#51224

Changelog: [Internal]

We have a bunch of places where we rely on implicit conversion operators of `jsi::Value` and return some primitive type.

This doesn't work well with Bridging, because currently it doesn't take into account these implicit operator conversions: primitives won't be treated as primitivies, but rather as generic `jsi::Value`, which could be many things.

We should be explicit about return type in `toJs`, because it affects the type checking logic.

Reviewed By: javache

Differential Revision: D74478571

fbshipit-source-id: 0633159c5af3a02aafe14e2b137c133d4554a5f8

Author: hoxyq

packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

@@ -125,7 +125,7 @@ namespace facebook::react {
 
 template <>
 struct Bridging<JsErrorHandler::ProcessedError::StackFrame> {
-  static jsi::Value toJs(
+  static jsi::Object toJs(
       jsi::Runtime& runtime,
       const JsErrorHandler::ProcessedError::StackFrame& frame) {
     auto stackFrame = jsi::Object(runtime);
@@ -143,7 +143,7 @@ struct Bridging<JsErrorHandler::ProcessedError::StackFrame> {
 
 template <>
 struct Bridging<JsErrorHandler::ProcessedError> {
-  static jsi::Value toJs(
+  static jsi::Object toJs(
       jsi::Runtime& runtime,
       const JsErrorHandler::ProcessedError& error) {
     auto data = jsi::Object(runtime);
@@ -341,7 +341,7 @@ void JsErrorHandler::handleErrorWithCppPipeline(
       .extraData = std::move(extraData),
   };
 
-  auto data = bridging::toJs(runtime, processedError).asObject(runtime);
+  auto data = bridging::toJs(runtime, processedError);
 
   auto isComponentError =
       isTruthy(runtime, errorObj.getProperty(runtime, "isComponentError"));

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

@@ -17,7 +17,7 @@ struct Bridging<bool> {
     return value.asBool();
   }
 
-  static jsi::Value toJs(jsi::Runtime&, bool value) {
+  static bool toJs(jsi::Runtime& /*unused*/, bool value) {
     return value;
   }
 };

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

@@ -13,45 +13,45 @@ namespace facebook::react {
 
 template <>
 struct Bridging<double> {
-  static double fromJs(jsi::Runtime&, const jsi::Value& value) {
+  static double fromJs(jsi::Runtime& /*unused*/, const jsi::Value& value) {
     return value.asNumber();
   }
 
-  static jsi::Value toJs(jsi::Runtime&, double value) {
+  static double toJs(jsi::Runtime& /*unused*/, double value) {
     return value;
   }
 };
 
 template <>
 struct Bridging<float> {
-  static float fromJs(jsi::Runtime&, const jsi::Value& value) {
+  static float fromJs(jsi::Runtime& /*unused*/, const jsi::Value& value) {
     return (float)value.asNumber();
   }
 
-  static jsi::Value toJs(jsi::Runtime&, float value) {
-    return (double)value;
+  static float toJs(jsi::Runtime& /*unused*/, float value) {
+    return value;
   }
 };
 
 template <>
 struct Bridging<int32_t> {
-  static int32_t fromJs(jsi::Runtime&, const jsi::Value& value) {
+  static int32_t fromJs(jsi::Runtime& /*unused*/, const jsi::Value& value) {
     return (int32_t)value.asNumber();
   }
 
-  static jsi::Value toJs(jsi::Runtime&, int32_t value) {
+  static int32_t toJs(jsi::Runtime& /*unused*/, int32_t value) {
     return value;
   }
 };
 
 template <>
 struct Bridging<uint32_t> {
-  static uint32_t fromJs(jsi::Runtime&, const jsi::Value& value) {
+  static uint32_t fromJs(jsi::Runtime& /*unused*/, const jsi::Value& value) {
     return (uint32_t)value.asNumber();
   }
 
-  static jsi::Value toJs(jsi::Runtime&, uint32_t value) {
-    return (double)value;
+  static jsi::Value toJs(jsi::Runtime& /*unused*/, uint32_t value) {
+    return double(value);
   }
 };
 

packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp

@@ -46,8 +46,8 @@ TEST_F(BridgingTest, boolTest) {
   EXPECT_FALSE(bridging::fromJs<bool>(rt, jsi::Value(false), invoker));
   EXPECT_JSI_THROW(bridging::fromJs<bool>(rt, jsi::Value(1), invoker));
 
-  EXPECT_TRUE(bridging::toJs(rt, true).asBool());
-  EXPECT_FALSE(bridging::toJs(rt, false).asBool());
+  EXPECT_TRUE(bridging::toJs(rt, true));
+  EXPECT_FALSE(bridging::toJs(rt, false));
 }
 
 TEST_F(BridgingTest, numberTest) {
@@ -56,10 +56,9 @@ TEST_F(BridgingTest, numberTest) {
   EXPECT_DOUBLE_EQ(1.2, bridging::fromJs<double>(rt, jsi::Value(1.2), invoker));
   EXPECT_JSI_THROW(bridging::fromJs<double>(rt, jsi::Value(true), invoker));
 
-  EXPECT_EQ(1, static_cast<int>(bridging::toJs(rt, 1).asNumber()));
-  EXPECT_FLOAT_EQ(
-      1.2f, static_cast<float>(bridging::toJs(rt, 1.2f).asNumber()));
-  EXPECT_DOUBLE_EQ(1.2, bridging::toJs(rt, 1.2).asNumber());
+  EXPECT_EQ(1, static_cast<int>(bridging::toJs(rt, 1)));
+  EXPECT_FLOAT_EQ(1.2f, static_cast<float>(bridging::toJs(rt, 1.2f)));
+  EXPECT_DOUBLE_EQ(1.2, bridging::toJs(rt, 1.2));
 
   EXPECT_EQ(
       42,

packages/react-native/ReactCommon/react/nativemodule/webperformance/NativePerformance.h

@@ -42,10 +42,8 @@ struct Bridging<PerformanceEntryType> {
     return static_cast<PerformanceEntryType>(value.asNumber());
   }
 
-  static jsi::Value toJs(
-      jsi::Runtime& /*rt*/,
-      const PerformanceEntryType& value) {
-    return {static_cast<int>(value)};
+  static int toJs(jsi::Runtime& /*rt*/, const PerformanceEntryType& value) {
+    return static_cast<int>(value);
   }
 };
 

packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.h

@@ -62,7 +62,7 @@ struct Bridging<CustomEnumInt> {
     }
   }
 
-  static jsi::Value toJs(jsi::Runtime& rt, CustomEnumInt value) {
+  static int32_t toJs(jsi::Runtime& rt, CustomEnumInt value) {
     return bridging::toJs(rt, static_cast<int32_t>(value));
   }
 };