Add Value override for has/get/setProperty (#52910)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/52910

For `get/has/setProperty`, we should also be able to take in a generic
JS Value as the property key. This change adds the Value overload for
these APIs.

The default implementation will use `Reflect.get`, `Reflect.has`, and
`Reflect.set`.

Changelog: [Internal]

Reviewed By: lavenzg

Differential Revision: D79120823

fbshipit-source-id: 7e2e5ff1ca93397c549e7dd922797fe77aa97940

packages/react-native/ReactCommon/jsi/jsi/decorator.h

@@ -311,12 +311,18 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
   Value getProperty(const Object& o, const String& name) override {
     return plain_.getProperty(o, name);
   };
+  Value getProperty(const Object& o, const Value& name) override {
+    return plain_.getProperty(o, name);
+  }
   bool hasProperty(const Object& o, const PropNameID& name) override {
     return plain_.hasProperty(o, name);
   };
   bool hasProperty(const Object& o, const String& name) override {
     return plain_.hasProperty(o, name);
   };
+  bool hasProperty(const Object& o, const Value& name) override {
+    return plain_.hasProperty(o, name);
+  }
   void setPropertyValue(
       const Object& o,
       const PropNameID& name,
@@ -327,6 +333,10 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
       override {
     plain_.setPropertyValue(o, name, value);
   };
+  void setPropertyValue(const Object& o, const Value& name, const Value& value)
+      override {
+    plain_.setPropertyValue(o, name, value);
+  }
 
   void deleteProperty(const Object& object, const PropNameID& name) override {
     plain_.deleteProperty(object, name);
@@ -843,6 +853,10 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
     Around around{with_};
     return RD::getProperty(o, name);
   };
+  Value getProperty(const Object& o, const Value& name) override {
+    Around around{with_};
+    return RD::getProperty(o, name);
+  }
   bool hasProperty(const Object& o, const PropNameID& name) override {
     Around around{with_};
     return RD::hasProperty(o, name);
@@ -851,6 +865,10 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
     Around around{with_};
     return RD::hasProperty(o, name);
   };
+  bool hasProperty(const Object& o, const Value& name) override {
+    Around around{with_};
+    return RD::hasProperty(o, name);
+  }
   void setPropertyValue(
       const Object& o,
       const PropNameID& name,
@@ -863,6 +881,11 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
     Around around{with_};
     RD::setPropertyValue(o, name, value);
   };
+  void setPropertyValue(const Object& o, const Value& name, const Value& value)
+      override {
+    Around around{with_};
+    RD::setPropertyValue(o, name, value);
+  }
 
   void deleteProperty(const Object& object, const PropNameID& name) override {
     Around around{with_};

packages/react-native/ReactCommon/jsi/jsi/jsi-inl.h

@@ -115,6 +115,10 @@ inline Value Object::getProperty(Runtime& runtime, const PropNameID& name)
   return runtime.getProperty(*this, name);
 }
 
+inline Value Object::getProperty(Runtime& runtime, const Value& name) const {
+  return runtime.getProperty(*this, name);
+}
+
 inline bool Object::hasProperty(Runtime& runtime, const char* name) const {
   return hasProperty(runtime, String::createFromAscii(runtime, name));
 }
@@ -128,6 +132,10 @@ inline bool Object::hasProperty(Runtime& runtime, const PropNameID& name)
   return runtime.hasProperty(*this, name);
 }
 
+inline bool Object::hasProperty(Runtime& runtime, const Value& name) const {
+  return runtime.hasProperty(*this, name);
+}
+
 template <typename T>
 void Object::setProperty(Runtime& runtime, const char* name, T&& value) const {
   setProperty(
@@ -148,6 +156,12 @@ void Object::setProperty(Runtime& runtime, const PropNameID& name, T&& value)
       runtime, name, detail::toValue(runtime, std::forward<T>(value)));
 }
 
+template <typename T>
+void Object::setProperty(Runtime& runtime, const Value& name, T&& value) const {
+  setPropertyValue(
+      runtime, name, detail::toValue(runtime, std::forward<T>(value)));
+}
+
 inline void Object::deleteProperty(Runtime& runtime, const char* name) const {
   deleteProperty(runtime, String::createFromAscii(runtime, name));
 }

packages/react-native/ReactCommon/jsi/jsi/jsi.cpp

@@ -503,6 +503,33 @@ const void* Runtime::getRuntimeDataImpl(const UUID& uuid) {
   return nullptr;
 }
 
+Value Runtime::getProperty(const Object& object, const Value& name) {
+  auto getFn = global()
+                   .getPropertyAsObject(*this, "Reflect")
+                   .getPropertyAsFunction(*this, "get");
+  return getFn.call(*this, object, name);
+}
+
+bool Runtime::hasProperty(const Object& object, const Value& name) {
+  auto hasFn = global()
+                   .getPropertyAsObject(*this, "Reflect")
+                   .getPropertyAsFunction(*this, "has");
+  return hasFn.call(*this, object, name).getBool();
+}
+
+void Runtime::setPropertyValue(
+    const Object& object,
+    const Value& name,
+    const Value& value) {
+  auto setFn = global()
+                   .getPropertyAsObject(*this, "Reflect")
+                   .getPropertyAsFunction(*this, "set");
+  auto setResult = setFn.call(*this, object, name, value).getBool();
+  if (!setResult) {
+    throw JSError(*this, "Failed to set the property");
+  }
+}
+
 Pointer& Pointer::operator=(Pointer&& other) noexcept {
   if (ptr_) {
     ptr_->invalidate();

packages/react-native/ReactCommon/jsi/jsi/jsi.h

@@ -477,14 +477,18 @@ class JSI_EXPORT Runtime : public ICast {
 
   virtual Value getProperty(const Object&, const PropNameID& name) = 0;
   virtual Value getProperty(const Object&, const String& name) = 0;
+  virtual Value getProperty(const Object&, const Value& name);
   virtual bool hasProperty(const Object&, const PropNameID& name) = 0;
   virtual bool hasProperty(const Object&, const String& name) = 0;
+  virtual bool hasProperty(const Object&, const Value& name);
   virtual void setPropertyValue(
       const Object&,
       const PropNameID& name,
       const Value& value) = 0;
   virtual void
   setPropertyValue(const Object&, const String& name, const Value& value) = 0;
+  virtual void
+  setPropertyValue(const Object&, const Value& name, const Value& value);
 
   virtual void deleteProperty(const Object&, const PropNameID& name);
   virtual void deleteProperty(const Object&, const String& name);
@@ -958,6 +962,12 @@ class JSI_EXPORT Object : public Pointer {
   /// undefined value.
   Value getProperty(Runtime& runtime, const PropNameID& name) const;
 
+  /// \return the Property of the object with the given JS Value name. If the
+  /// name isn't a property on the object, returns the undefined value.This
+  /// attempts to convert the JS Value to convert to a property key. If the
+  /// conversion fails, this method may throw.
+  Value getProperty(Runtime& runtime, const Value& name) const;
+
   /// \return true if and only if the object has a property with the
   /// given ascii name.
   bool hasProperty(Runtime& runtime, const char* name) const;
@@ -970,6 +980,11 @@ class JSI_EXPORT Object : public Pointer {
   /// given PropNameID name.
   bool hasProperty(Runtime& runtime, const PropNameID& name) const;
 
+  /// \return true if and only if the object has a property with the given
+  /// JS Value name. This attempts to convert the JS Value to convert to a
+  /// property key. If the conversion fails, this method may throw.
+  bool hasProperty(Runtime& runtime, const Value& name) const;
+
   /// Sets the property value from a Value or anything which can be
   /// used to make one: nullptr_t, bool, double, int, const char*,
   /// String, or Object.
@@ -988,6 +1003,14 @@ class JSI_EXPORT Object : public Pointer {
   template <typename T>
   void setProperty(Runtime& runtime, const PropNameID& name, T&& value) const;
 
+  /// Sets the property value from a Value or anything which can be
+  /// used to make one: nullptr_t, bool, double, int, const char*,
+  /// String, or Object. This takes a JS Value as the property name, and
+  /// attempts to convert to a property key. If the conversion fails, this
+  /// method may throw.
+  template <typename T>
+  void setProperty(Runtime& runtime, const Value& name, T&& value) const;
+
   /// Delete the property with the given ascii name. Throws if the deletion
   /// failed.
   void deleteProperty(Runtime& runtime, const char* name) const;
@@ -1145,6 +1168,11 @@ class JSI_EXPORT Object : public Pointer {
     return runtime.setPropertyValue(*this, name, value);
   }
 
+  void setPropertyValue(Runtime& runtime, const Value& name, const Value& value)
+      const {
+    return runtime.setPropertyValue(*this, name, value);
+  }
+
   friend class Runtime;
   friend class Value;
 };

packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp

@@ -176,6 +176,57 @@ TEST_P(JSITest, ObjectTest) {
   Array names = obj.getPropertyNames(rt);
   EXPECT_EQ(names.size(rt), 1);
   EXPECT_EQ(names.getValueAtIndex(rt, 0).getString(rt).utf8(rt), "a");
+
+  // This Runtime Decorator is used to test the default implementation of
+  // Runtime::has/get/setProperty with Value overload
+  class RD : public RuntimeDecorator<Runtime, Runtime> {
+   public:
+    explicit RD(Runtime& rt) : RuntimeDecorator(rt) {}
+
+    Value getProperty(const Object& object, const Value& name) override {
+      return Runtime::getProperty(object, name);
+    }
+
+    bool hasProperty(const Object& object, const Value& name) override {
+      return Runtime::hasProperty(object, name);
+    }
+
+    void setPropertyValue(
+        const Object& object,
+        const Value& name,
+        const Value& value) override {
+      Runtime::setPropertyValue(object, name, value);
+    }
+  };
+
+  RD rd = RD(rt);
+
+  obj = eval("const obj = {}; obj;").getObject(rd);
+  auto propVal = Value(123);
+  obj.setProperty(rd, propVal, 456);
+  EXPECT_TRUE(obj.hasProperty(rd, propVal));
+  auto getRes = obj.getProperty(rd, propVal);
+  EXPECT_EQ(getRes.getNumber(), 456);
+
+  /// The property is non-writable so it should fail
+  obj = eval(
+            "Object.defineProperty(obj, '456', {"
+            "  value: 10,"
+            "  writable: false,});")
+            .getObject(rd);
+  auto unwritableProp = Value(456);
+  EXPECT_THROW(obj.setProperty(rd, unwritableProp, 1), JSError);
+
+  auto badObjKey = eval(
+      "var badObj = {"
+      "    toString: function() {"
+      "        throw new Error('something went wrong');"
+      "    }"
+      "};"
+      "badObj;");
+  EXPECT_THROW(obj.setProperty(rd, badObjKey, 123), JSError);
+  EXPECT_THROW(obj.hasProperty(rd, badObjKey), JSError);
+  EXPECT_THROW(obj.getProperty(rd, badObjKey), JSError);
 }
 
 TEST_P(JSITest, HostObjectTest) {