JSValue exports data fields - should export value reference accessors (#14707)

acoates-ms · web-flow · commit 105c93dbbed0 · 2025-05-28T23:29:38.000Z
* JSValue shouldn't export data fields, which make it hard to use MS.RN.Cxx across dll boundaries

* Change files

* missing functions

* fix
diff --git a/change/react-native-windows-9e27af67-9d3c-4e6c-bf06-48380424d48f.json b/change/react-native-windows-9e27af67-9d3c-4e6c-bf06-48380424d48f.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "JSValue shouldn't export data fields, which make it hard to use MS.RN.Cxx across dll boundaries",
+  "packageName": "react-native-windows",
+  "email": "30809111+acoates-ms@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}
diff --git a/vnext/Microsoft.ReactNative.Cxx.UnitTests/JSValueTest.cpp b/vnext/Microsoft.ReactNative.Cxx.UnitTests/JSValueTest.cpp
@@ -188,13 +188,13 @@ TEST_CLASS (JSValueTest) {
   TEST_METHOD(TestObjectLiteral) {
     JSValue jsValue = JSValueObject{
         {"NullValue1", nullptr},
-        {"NullValue2", JSValue::Null.Copy()},
+        {"NullValue2", JSValue::NullRef().Copy()},
         {"ObjValue", JSValueObject{{"prop1", 2}}},
-        {"ObjValueEmpty", JSValue::EmptyObject.Copy()},
+        {"ObjValueEmpty", JSValue::EmptyObjectRef().Copy()},
         {"ArrayValue", JSValueArray{1, 2}},
-        {"ArrayValueEmpty", JSValue::EmptyArray.Copy()},
+        {"ArrayValueEmpty", JSValue::EmptyArrayRef().Copy()},
         {"StringValue1", "Hello"},
-        {"StringValue2", JSValue::EmptyString.Copy()},
+        {"StringValue2", JSValue::EmptyStringRef().Copy()},
         {"BoolValue", true},
         {"IntValue", 42},
         {"DoubleValue", 4.5}};
@@ -493,9 +493,9 @@ TEST_CLASS (JSValueTest) {
     auto AsObjectIsEmpty = [](JSValue const &value) { return value.AsObject().empty(); };
 
     TestCheck(!AsObjectIsEmpty(JSValueObject{{"prop1", 42}}));
-    TestCheck(AsObjectIsEmpty(JSValue::EmptyObject));
+    TestCheck(AsObjectIsEmpty(JSValue::EmptyObjectRef()));
     TestCheck(AsObjectIsEmpty(JSValueArray{42, 78}));
-    TestCheck(AsObjectIsEmpty(JSValue::EmptyArray));
+    TestCheck(AsObjectIsEmpty(JSValue::EmptyArrayRef()));
     TestCheck(AsObjectIsEmpty(""));
     TestCheck(AsObjectIsEmpty("Hello"));
     TestCheck(AsObjectIsEmpty(true));
@@ -509,17 +509,17 @@ TEST_CLASS (JSValueTest) {
     TestCheck(AsObjectIsEmpty(std::numeric_limits<double>::quiet_NaN()));
     TestCheck(AsObjectIsEmpty(std::numeric_limits<double>::infinity()));
     TestCheck(AsObjectIsEmpty(-std::numeric_limits<double>::infinity()));
-    TestCheck(AsObjectIsEmpty(JSValue::Null));
+    TestCheck(AsObjectIsEmpty(JSValue::NullRef()));
   }
 
   TEST_METHOD(TestAsArray) {
     // Any type except for Array is returned as EmptyArray.
     auto AsArrayIsEmpty = [](JSValue const &value) { return value.AsArray().empty(); };
 
     TestCheck(AsArrayIsEmpty(JSValueObject{{"prop1", 42}}));
-    TestCheck(AsArrayIsEmpty(JSValue::EmptyObject));
+    TestCheck(AsArrayIsEmpty(JSValue::EmptyObjectRef()));
     TestCheck(!AsArrayIsEmpty(JSValueArray{42, 78}));
-    TestCheck(AsArrayIsEmpty(JSValue::EmptyArray));
+    TestCheck(AsArrayIsEmpty(JSValue::EmptyArrayRef()));
     TestCheck(AsArrayIsEmpty(""));
     TestCheck(AsArrayIsEmpty("Hello"));
     TestCheck(AsArrayIsEmpty(true));
@@ -533,7 +533,7 @@ TEST_CLASS (JSValueTest) {
     TestCheck(AsArrayIsEmpty(std::numeric_limits<double>::quiet_NaN()));
     TestCheck(AsArrayIsEmpty(std::numeric_limits<double>::infinity()));
     TestCheck(AsArrayIsEmpty(-std::numeric_limits<double>::infinity()));
-    TestCheck(AsArrayIsEmpty(JSValue::Null));
+    TestCheck(AsArrayIsEmpty(JSValue::NullRef()));
   }
 
   // Check AsString, AsBoolean, AsInt64, and AsDouble conversions.
@@ -570,9 +570,9 @@ TEST_CLASS (JSValueTest) {
 
   TEST_METHOD(TestAsConverters) {
     CheckAsConverter((JSValueObject{{"prop1", 42}}), "", true, 0, 0);
-    CheckAsConverter(JSValue::EmptyObject, "", false, 0, 0);
+    CheckAsConverter(JSValue::EmptyObjectRef(), "", false, 0, 0);
     CheckAsConverter((JSValueArray{42, 78}), "", true, 0, 0);
-    CheckAsConverter(JSValue::EmptyArray, "", false, 0, 0);
+    CheckAsConverter(JSValue::EmptyArrayRef(), "", false, 0, 0);
     CheckAsConverter("", "", false, 0, 0);
     CheckAsConverter("  ", "  ", false, 0, 0);
     CheckAsConverter("42", "42", false, 42, 42);
@@ -624,7 +624,7 @@ TEST_CLASS (JSValueTest) {
     CheckAsConverter(NAN, "NaN", false, 0, NAN);
     CheckAsConverter(INFINITY, "Infinity", true, 0, INFINITY);
     CheckAsConverter(-INFINITY, "-Infinity", true, 0, -INFINITY);
-    CheckAsConverter(JSValue::Null, "null", false, 0, 0);
+    CheckAsConverter(JSValue::NullRef(), "null", false, 0, 0);
   }
 
   TEST_METHOD(TestExplicitNumberConversion) {
diff --git a/vnext/Microsoft.ReactNative.Cxx/JSValue.cpp b/vnext/Microsoft.ReactNative.Cxx/JSValue.cpp
@@ -278,7 +278,7 @@ JSValue const &JSValueObject::operator[](std::string_view propertyName) const no
     return it->second;
   }
 
-  return JSValue::Null;
+  return JSValue::NullRef();
 }
 
 bool JSValueObject::Equals(JSValueObject const &other) const noexcept {
@@ -434,6 +434,22 @@ void JSValueArray::WriteTo(IJSValueWriter const &writer) const noexcept {
 /*static*/ JSValue const JSValue::EmptyArray{JSValueArray{}};
 /*static*/ JSValue const JSValue::EmptyString{std::string{}};
 
+const JSValue &JSValue::NullRef() noexcept {
+  return JSValue::Null;
+}
+
+const JSValue &JSValue::EmptyObjectRef() noexcept {
+  return JSValue::EmptyObject;
+}
+
+const JSValue &JSValue::EmptyArrayRef() noexcept {
+  return JSValue::EmptyArray;
+}
+
+const JSValue &JSValue::EmptyStringRef() noexcept {
+  return JSValue::EmptyString;
+}
+
 #pragma warning(push)
 #pragma warning(disable : 26495) // False positive for union member not initialized
 JSValue::JSValue(JSValue &&other) noexcept : m_type{other.m_type} {
@@ -739,7 +755,7 @@ JSValue const *JSValue::TryGetObjectProperty(std::string_view propertyName) cons
 
 JSValue const &JSValue::GetObjectProperty(std::string_view propertyName) const noexcept {
   auto result = TryGetObjectProperty(propertyName);
-  return result ? *result : Null;
+  return result ? *result : NullRef();
 }
 
 size_t JSValue::ItemCount() const noexcept {
@@ -752,7 +768,7 @@ JSValue const *JSValue::TryGetArrayItem(JSValueArray::size_type index) const noe
 
 JSValue const &JSValue::GetArrayItem(JSValueArray::size_type index) const noexcept {
   auto result = TryGetArrayItem(index);
-  return result ? *result : Null;
+  return result ? *result : NullRef();
 }
 
 bool JSValue::Equals(JSValue const &other) const noexcept {
diff --git a/vnext/Microsoft.ReactNative.Cxx/JSValue.h b/vnext/Microsoft.ReactNative.Cxx/JSValue.h
@@ -181,17 +181,25 @@ bool operator!=(JSValueArray const &left, JSValueArray const &right) noexcept;
 //! For copy operations the explicit Copy() method must be used.
 //! Note that the move operations are not thread safe.
 struct JSValue {
-  //! JSValue with JSValueType::Null.
+  //! JSValue with JSValueType::Null. - Maybe removed in future version - replaced with NullRef
   static JSValue const Null;
+  //! JSValue with JSValueType::Null.
+  static const JSValue &NullRef() noexcept;
 
-  //! JSValue with empty object.
+  //! JSValue with empty object. - Maybe removed in future version - replaced with EmptyObjectRef
   static JSValue const EmptyObject;
+  //! JSValue with empty object.
+  static const JSValue &EmptyObjectRef() noexcept;
 
-  //! JSValue with empty array.
+  //! JSValue with empty array. - Maybe removed in future version - replaced with EmptyArrayRef
   static JSValue const EmptyArray;
+  //! JSValue with empty array.
+  static const JSValue &EmptyArrayRef() noexcept;
 
-  //! JSValue with empty string.
+  //! JSValue with empty string. - Maybe removed in future version - replaced with EmptyStringRef
   static JSValue const EmptyString;
+  //! JSValue with empty string.
+  static const JSValue &EmptyStringRef() noexcept;
 
   //! Create a Null JSValue.
   JSValue() noexcept;
@@ -654,11 +662,11 @@ inline double const *JSValue::TryGetDouble() const noexcept {
 }
 
 inline JSValueObject const &JSValue::AsObject() const noexcept {
-  return (m_type == JSValueType::Object) ? m_object : EmptyObject.m_object;
+  return (m_type == JSValueType::Object) ? m_object : EmptyObjectRef().m_object;
 }
 
 inline JSValueArray const &JSValue::AsArray() const noexcept {
-  return (m_type == JSValueType::Array) ? m_array : EmptyArray.m_array;
+  return (m_type == JSValueType::Array) ? m_array : EmptyArrayRef().m_array;
 }
 
 inline int8_t JSValue::AsInt8() const noexcept {
@@ -860,7 +868,7 @@ inline const JSValueArray &JSValue::Array() const noexcept {
 }
 
 inline const std::string &JSValue::String() const noexcept {
-  return (m_type == JSValueType::String) ? m_string : EmptyString.m_string;
+  return (m_type == JSValueType::String) ? m_string : EmptyStringRef().m_string;
 }
 
 inline bool JSValue::Boolean() const noexcept {
