From b2d25c8731000190ee99bb03982ab6fa4eec9a16 Mon Sep 17 00:00:00 2001 From: Chi Tsai Date: Thu, 28 Aug 2025 19:21:27 -0700 Subject: [PATCH] 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 --- .../ReactCommon/jsi/jsi/decorator.h | 23 +++++++++ .../ReactCommon/jsi/jsi/jsi-inl.h | 14 +++++ .../react-native/ReactCommon/jsi/jsi/jsi.cpp | 27 ++++++++++ .../react-native/ReactCommon/jsi/jsi/jsi.h | 28 ++++++++++ .../ReactCommon/jsi/jsi/test/testlib.cpp | 51 +++++++++++++++++++ 5 files changed, 143 insertions(+) diff --git a/packages/react-native/ReactCommon/jsi/jsi/decorator.h b/packages/react-native/ReactCommon/jsi/jsi/decorator.h index d736aeaaf61..5aefd7d3c38 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/decorator.h +++ b/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 { 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 { 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 { 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_}; diff --git a/packages/react-native/ReactCommon/jsi/jsi/jsi-inl.h b/packages/react-native/ReactCommon/jsi/jsi/jsi-inl.h index ed4b6fcd8df..2f70a59483c 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/jsi-inl.h +++ b/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 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(value))); } +template +void Object::setProperty(Runtime& runtime, const Value& name, T&& value) const { + setPropertyValue( + runtime, name, detail::toValue(runtime, std::forward(value))); +} + inline void Object::deleteProperty(Runtime& runtime, const char* name) const { deleteProperty(runtime, String::createFromAscii(runtime, name)); } diff --git a/packages/react-native/ReactCommon/jsi/jsi/jsi.cpp b/packages/react-native/ReactCommon/jsi/jsi/jsi.cpp index 5e2b24cf67b..571b41360a1 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/jsi.cpp +++ b/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(); diff --git a/packages/react-native/ReactCommon/jsi/jsi/jsi.h b/packages/react-native/ReactCommon/jsi/jsi/jsi.h index e994769c453..4c21f5db290 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/jsi.h +++ b/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 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 + 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; }; diff --git a/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp b/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp index 195ac28ec4f..b37b4bfe471 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp +++ b/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 { + 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) {