Skip to content

Commit fa778ce

Browse files
acoates-msacoates-ms
committed
JSValue exports data fields - should export value reference accessors (microsoft#14707)
* JSValue shouldn't export data fields, which make it hard to use MS.RN.Cxx across dll boundaries * Change files * missing functions * fix
1 parent 30bb649 commit fa778ce

File tree

4 files changed

+54
-23
lines changed

4 files changed

+54
-23
lines changed

‎change/react-native-windows-9e27af67-9d3c-4e6c-bf06-48380424d48f.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "JSValue shouldn't export data fields, which make it hard to use MS.RN.Cxx across dll boundaries",
4+
"packageName": "react-native-windows",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎vnext/Microsoft.ReactNative.Cxx.UnitTests/JSValueTest.cpp‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,13 @@ TEST_CLASS (JSValueTest) {
188188
TEST_METHOD(TestObjectLiteral) {
189189
JSValue jsValue = JSValueObject{
190190
{"NullValue1", nullptr},
191-
{"NullValue2", JSValue::Null.Copy()},
191+
{"NullValue2", JSValue::NullRef().Copy()},
192192
{"ObjValue", JSValueObject{{"prop1", 2}}},
193-
{"ObjValueEmpty", JSValue::EmptyObject.Copy()},
193+
{"ObjValueEmpty", JSValue::EmptyObjectRef().Copy()},
194194
{"ArrayValue", JSValueArray{1, 2}},
195-
{"ArrayValueEmpty", JSValue::EmptyArray.Copy()},
195+
{"ArrayValueEmpty", JSValue::EmptyArrayRef().Copy()},
196196
{"StringValue1", "Hello"},
197-
{"StringValue2", JSValue::EmptyString.Copy()},
197+
{"StringValue2", JSValue::EmptyStringRef().Copy()},
198198
{"BoolValue", true},
199199
{"IntValue", 42},
200200
{"DoubleValue", 4.5}};
@@ -493,9 +493,9 @@ TEST_CLASS (JSValueTest) {
493493
auto AsObjectIsEmpty = [](JSValue const &value) { return value.AsObject().empty(); };
494494

495495
TestCheck(!AsObjectIsEmpty(JSValueObject{{"prop1", 42}}));
496-
TestCheck(AsObjectIsEmpty(JSValue::EmptyObject));
496+
TestCheck(AsObjectIsEmpty(JSValue::EmptyObjectRef()));
497497
TestCheck(AsObjectIsEmpty(JSValueArray{42, 78}));
498-
TestCheck(AsObjectIsEmpty(JSValue::EmptyArray));
498+
TestCheck(AsObjectIsEmpty(JSValue::EmptyArrayRef()));
499499
TestCheck(AsObjectIsEmpty(""));
500500
TestCheck(AsObjectIsEmpty("Hello"));
501501
TestCheck(AsObjectIsEmpty(true));
@@ -509,17 +509,17 @@ TEST_CLASS (JSValueTest) {
509509
TestCheck(AsObjectIsEmpty(std::numeric_limits<double>::quiet_NaN()));
510510
TestCheck(AsObjectIsEmpty(std::numeric_limits<double>::infinity()));
511511
TestCheck(AsObjectIsEmpty(-std::numeric_limits<double>::infinity()));
512-
TestCheck(AsObjectIsEmpty(JSValue::Null));
512+
TestCheck(AsObjectIsEmpty(JSValue::NullRef()));
513513
}
514514

515515
TEST_METHOD(TestAsArray) {
516516
// Any type except for Array is returned as EmptyArray.
517517
auto AsArrayIsEmpty = [](JSValue const &value) { return value.AsArray().empty(); };
518518

519519
TestCheck(AsArrayIsEmpty(JSValueObject{{"prop1", 42}}));
520-
TestCheck(AsArrayIsEmpty(JSValue::EmptyObject));
520+
TestCheck(AsArrayIsEmpty(JSValue::EmptyObjectRef()));
521521
TestCheck(!AsArrayIsEmpty(JSValueArray{42, 78}));
522-
TestCheck(AsArrayIsEmpty(JSValue::EmptyArray));
522+
TestCheck(AsArrayIsEmpty(JSValue::EmptyArrayRef()));
523523
TestCheck(AsArrayIsEmpty(""));
524524
TestCheck(AsArrayIsEmpty("Hello"));
525525
TestCheck(AsArrayIsEmpty(true));
@@ -533,7 +533,7 @@ TEST_CLASS (JSValueTest) {
533533
TestCheck(AsArrayIsEmpty(std::numeric_limits<double>::quiet_NaN()));
534534
TestCheck(AsArrayIsEmpty(std::numeric_limits<double>::infinity()));
535535
TestCheck(AsArrayIsEmpty(-std::numeric_limits<double>::infinity()));
536-
TestCheck(AsArrayIsEmpty(JSValue::Null));
536+
TestCheck(AsArrayIsEmpty(JSValue::NullRef()));
537537
}
538538

539539
// Check AsString, AsBoolean, AsInt64, and AsDouble conversions.
@@ -570,9 +570,9 @@ TEST_CLASS (JSValueTest) {
570570

571571
TEST_METHOD(TestAsConverters) {
572572
CheckAsConverter((JSValueObject{{"prop1", 42}}), "", true, 0, 0);
573-
CheckAsConverter(JSValue::EmptyObject, "", false, 0, 0);
573+
CheckAsConverter(JSValue::EmptyObjectRef(), "", false, 0, 0);
574574
CheckAsConverter((JSValueArray{42, 78}), "", true, 0, 0);
575-
CheckAsConverter(JSValue::EmptyArray, "", false, 0, 0);
575+
CheckAsConverter(JSValue::EmptyArrayRef(), "", false, 0, 0);
576576
CheckAsConverter("", "", false, 0, 0);
577577
CheckAsConverter(" ", " ", false, 0, 0);
578578
CheckAsConverter("42", "42", false, 42, 42);
@@ -624,7 +624,7 @@ TEST_CLASS (JSValueTest) {
624624
CheckAsConverter(NAN, "NaN", false, 0, NAN);
625625
CheckAsConverter(INFINITY, "Infinity", true, 0, INFINITY);
626626
CheckAsConverter(-INFINITY, "-Infinity", true, 0, -INFINITY);
627-
CheckAsConverter(JSValue::Null, "null", false, 0, 0);
627+
CheckAsConverter(JSValue::NullRef(), "null", false, 0, 0);
628628
}
629629

630630
TEST_METHOD(TestExplicitNumberConversion) {

‎vnext/Microsoft.ReactNative.Cxx/JSValue.cpp‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ JSValue const &JSValueObject::operator[](std::string_view propertyName) const no
278278
return it->second;
279279
}
280280

281-
return JSValue::Null;
281+
return JSValue::NullRef();
282282
}
283283

284284
bool JSValueObject::Equals(JSValueObject const &other) const noexcept {
@@ -434,6 +434,22 @@ void JSValueArray::WriteTo(IJSValueWriter const &writer) const noexcept {
434434
/*static*/ JSValue const JSValue::EmptyArray{JSValueArray{}};
435435
/*static*/ JSValue const JSValue::EmptyString{std::string{}};
436436

437+
const JSValue &JSValue::NullRef() noexcept {
438+
return JSValue::Null;
439+
}
440+
441+
const JSValue &JSValue::EmptyObjectRef() noexcept {
442+
return JSValue::EmptyObject;
443+
}
444+
445+
const JSValue &JSValue::EmptyArrayRef() noexcept {
446+
return JSValue::EmptyArray;
447+
}
448+
449+
const JSValue &JSValue::EmptyStringRef() noexcept {
450+
return JSValue::EmptyString;
451+
}
452+
437453
#pragma warning(push)
438454
#pragma warning(disable : 26495) // False positive for union member not initialized
439455
JSValue::JSValue(JSValue &&other) noexcept : m_type{other.m_type} {
@@ -739,7 +755,7 @@ JSValue const *JSValue::TryGetObjectProperty(std::string_view propertyName) cons
739755

740756
JSValue const &JSValue::GetObjectProperty(std::string_view propertyName) const noexcept {
741757
auto result = TryGetObjectProperty(propertyName);
742-
return result ? *result : Null;
758+
return result ? *result : NullRef();
743759
}
744760

745761
size_t JSValue::ItemCount() const noexcept {
@@ -752,7 +768,7 @@ JSValue const *JSValue::TryGetArrayItem(JSValueArray::size_type index) const noe
752768

753769
JSValue const &JSValue::GetArrayItem(JSValueArray::size_type index) const noexcept {
754770
auto result = TryGetArrayItem(index);
755-
return result ? *result : Null;
771+
return result ? *result : NullRef();
756772
}
757773

758774
bool JSValue::Equals(JSValue const &other) const noexcept {

‎vnext/Microsoft.ReactNative.Cxx/JSValue.h‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,17 +181,25 @@ bool operator!=(JSValueArray const &left, JSValueArray const &right) noexcept;
181181
//! For copy operations the explicit Copy() method must be used.
182182
//! Note that the move operations are not thread safe.
183183
struct JSValue {
184-
//! JSValue with JSValueType::Null.
184+
//! JSValue with JSValueType::Null. - Maybe removed in future version - replaced with NullRef
185185
static JSValue const Null;
186+
//! JSValue with JSValueType::Null.
187+
static const JSValue &NullRef() noexcept;
186188

187-
//! JSValue with empty object.
189+
//! JSValue with empty object. - Maybe removed in future version - replaced with EmptyObjectRef
188190
static JSValue const EmptyObject;
191+
//! JSValue with empty object.
192+
static const JSValue &EmptyObjectRef() noexcept;
189193

190-
//! JSValue with empty array.
194+
//! JSValue with empty array. - Maybe removed in future version - replaced with EmptyArrayRef
191195
static JSValue const EmptyArray;
196+
//! JSValue with empty array.
197+
static const JSValue &EmptyArrayRef() noexcept;
192198

193-
//! JSValue with empty string.
199+
//! JSValue with empty string. - Maybe removed in future version - replaced with EmptyStringRef
194200
static JSValue const EmptyString;
201+
//! JSValue with empty string.
202+
static const JSValue &EmptyStringRef() noexcept;
195203

196204
//! Create a Null JSValue.
197205
JSValue() noexcept;
@@ -654,11 +662,11 @@ inline double const *JSValue::TryGetDouble() const noexcept {
654662
}
655663

656664
inline JSValueObject const &JSValue::AsObject() const noexcept {
657-
return (m_type == JSValueType::Object) ? m_object : EmptyObject.m_object;
665+
return (m_type == JSValueType::Object) ? m_object : EmptyObjectRef().m_object;
658666
}
659667

660668
inline JSValueArray const &JSValue::AsArray() const noexcept {
661-
return (m_type == JSValueType::Array) ? m_array : EmptyArray.m_array;
669+
return (m_type == JSValueType::Array) ? m_array : EmptyArrayRef().m_array;
662670
}
663671

664672
inline int8_t JSValue::AsInt8() const noexcept {
@@ -860,7 +868,7 @@ inline const JSValueArray &JSValue::Array() const noexcept {
860868
}
861869

862870
inline const std::string &JSValue::String() const noexcept {
863-
return (m_type == JSValueType::String) ? m_string : EmptyString.m_string;
871+
return (m_type == JSValueType::String) ? m_string : EmptyStringRef().m_string;
864872
}
865873

866874
inline bool JSValue::Boolean() const noexcept {

0 commit comments

Comments
 (0)