From 3cc5601c7ca3295675a0d3f277f964d758d10808 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Tue, 12 May 2020 18:57:34 -0700 Subject: [PATCH 1/3] Allow storing non-WinRT types in ReactPropertyBag --- vnext/Microsoft.ReactNative.Cxx/BoxedValue.h | 31 +++++++++++++++++++ .../Microsoft.ReactNative.Cxx.vcxitems | 1 + .../Microsoft.ReactNative.Cxx/NativeModules.h | 18 +---------- .../ReactPropertyBag.h | 24 +++++++++++--- .../ReactPropertyBagTests.cpp | 22 +++++++++++++ 5 files changed, 74 insertions(+), 22 deletions(-) create mode 100644 vnext/Microsoft.ReactNative.Cxx/BoxedValue.h diff --git a/vnext/Microsoft.ReactNative.Cxx/BoxedValue.h b/vnext/Microsoft.ReactNative.Cxx/BoxedValue.h new file mode 100644 index 00000000000..86add9e7cec --- /dev/null +++ b/vnext/Microsoft.ReactNative.Cxx/BoxedValue.h @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#pragma once +#ifndef MICROSOFT_REACTNATIVE_BOXEDVALUE +#define MICROSOFT_REACTNATIVE_BOXEDVALUE + +#include + +namespace winrt::Microsoft::ReactNative { + +template +struct BoxedValue : implements, IBoxedValue> { + template + BoxedValue(TArgs &&... args) noexcept : m_value(std::forward(args)...) {} + + int64_t GetPtr() noexcept { + return reinterpret_cast(&m_value); + } + + static T &GetValueUnsafe(IBoxedValue const &boxedValue) noexcept { + return *reinterpret_cast(boxedValue.GetPtr()); + } + + private: + T m_value{}; +}; + +} // namespace winrt::Microsoft::ReactNative + +#endif // MICROSOFT_REACTNATIVE_BOXEDVALUE diff --git a/vnext/Microsoft.ReactNative.Cxx/Microsoft.ReactNative.Cxx.vcxitems b/vnext/Microsoft.ReactNative.Cxx/Microsoft.ReactNative.Cxx.vcxitems index 9a476a959fb..38328ca6aa6 100644 --- a/vnext/Microsoft.ReactNative.Cxx/Microsoft.ReactNative.Cxx.vcxitems +++ b/vnext/Microsoft.ReactNative.Cxx/Microsoft.ReactNative.Cxx.vcxitems @@ -17,6 +17,7 @@ + diff --git a/vnext/Microsoft.ReactNative.Cxx/NativeModules.h b/vnext/Microsoft.ReactNative.Cxx/NativeModules.h index 19ab45c0080..23532730abc 100644 --- a/vnext/Microsoft.ReactNative.Cxx/NativeModules.h +++ b/vnext/Microsoft.ReactNative.Cxx/NativeModules.h @@ -4,6 +4,7 @@ #pragma once #include "winrt/Microsoft.ReactNative.h" +#include "BoxedValue.h" #include "JSValueReader.h" #include "JSValueWriter.h" #include "ModuleRegistration.h" @@ -1105,23 +1106,6 @@ struct TurboModuleSpec { } }; -template -struct BoxedValue : implements, IBoxedValue> { - template - BoxedValue(TArgs &&... args) noexcept : m_value(std::forward(args)...) {} - - int64_t GetPtr() noexcept { - return reinterpret_cast(&m_value); - } - - static T &GetValueUnsafe(IBoxedValue const &boxedValue) noexcept { - return *reinterpret_cast(boxedValue.GetPtr()); - } - - private: - T m_value{}; -}; - template inline ReactModuleProvider MakeModuleProvider() noexcept { return [](IReactModuleBuilder const &moduleBuilder) noexcept { diff --git a/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h b/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h index 1dc6d09bcdf..4148e33d9ee 100644 --- a/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h +++ b/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h @@ -37,12 +37,14 @@ // directly because their null value may indicated absent property value. // For other types we return std::optional. It has std::nullopt value in case if // no property value is found or if it has a wrong type. -// To avoid compilation errors the non-IInspectable types must be WinRT types which are described here: +// To pass values through the ABI boundary the non-IInspectable types must be WinRT types +// which are described here: // https://docs.microsoft.com/en-us/uwp/api/windows.foundation.propertytype?view=winrt-18362 // #include #include +#include "BoxedValue.h" namespace winrt::Microsoft::ReactNative { @@ -113,10 +115,10 @@ struct ReactPropertyId { IReactPropertyName m_handle; }; -// ReactPropertyBag is a wrapper for IReactPropertyBag to stores strongly-typed properties in a thread-safe way. +// ReactPropertyBag is a wrapper for IReactPropertyBag to store strongly-typed properties in a thread-safe way. // Types inherited from IInspectable are stored directly. // Values of other types are boxed with help of winrt::box_value. -// Only WinRT types can be stored. +// Non-WinRT types are wrapped with help of BoxedValue template. struct ReactPropertyBag { // Property result type is either T or std::optional. // T is returned for types inherited from IInspectable. @@ -235,7 +237,11 @@ struct ReactPropertyBag { private: template static Windows::Foundation::IInspectable ToObject(T const &value) noexcept { - return winrt::box_value(value); + if constexpr (impl::has_category_v) { + return box_value(value); + } else { + return make>(value); + } } template @@ -247,7 +253,7 @@ struct ReactPropertyBag { static auto FromObject(Windows::Foundation::IInspectable const &obj) noexcept { if constexpr (std::is_base_of_v) { return obj.try_as(); - } else { + } else if constexpr (impl::has_category_v) { if (obj) { #ifdef WINRT_IMPL_IUNKNOWN_DEFINED if constexpr (std::is_same_v) { @@ -267,6 +273,14 @@ struct ReactPropertyBag { } } + return std::optional{}; + } else { + if (obj) { + if (auto temp = obj.try_as()) { + return std::optional{BoxedValue::GetValueUnsafe(temp)}; + } + } + return std::optional{}; } } diff --git a/vnext/Microsoft.ReactNative.IntegrationTests/ReactPropertyBagTests.cpp b/vnext/Microsoft.ReactNative.IntegrationTests/ReactPropertyBagTests.cpp index 27168257d81..f96cee63e26 100644 --- a/vnext/Microsoft.ReactNative.IntegrationTests/ReactPropertyBagTests.cpp +++ b/vnext/Microsoft.ReactNative.IntegrationTests/ReactPropertyBagTests.cpp @@ -6,6 +6,7 @@ using namespace winrt; using namespace Microsoft::ReactNative; +using namespace Windows::Foundation; namespace ReactNativeIntegrationTests { @@ -522,6 +523,27 @@ TEST_CLASS (ReactPropertyBagTests) { pb.Remove(fooName); TestCheck(!pb.Get(fooName)); } + + TEST_METHOD(PropertyBag_Property_Functor) { + ReactPropertyId> fooName{L"Foo"}; + ReactPropertyBag pb{ReactPropertyBagHelper::CreatePropertyBag()}; + + TestCheck(!pb.Get(fooName)); + Mso::Functor createValue1 = []() noexcept { + return winrt::box_value(5); + }; + TestCheckEqual(createValue1, *pb.GetOrCreate(fooName, [&createValue1]() { return createValue1; })); + TestCheckEqual(createValue1, *pb.Get(fooName)); + TestCheckEqual(5, winrt::unbox_value((*pb.Get(fooName))())); + + Mso::Functor createValue2 = []() { return winrt::box_value(10); }; + pb.Set(fooName, createValue2); + TestCheckEqual(createValue2, *pb.Get(fooName)); + TestCheckEqual(10, winrt::unbox_value((*pb.Get(fooName))())); + TestCheck(createValue1 != *pb.Get(fooName)); + pb.Remove(fooName); + TestCheck(!pb.Get(fooName)); + } }; } // namespace ReactNativeIntegrationTests From 659ee1f52cd12e69e908c9a8a37eed8dc844166d Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Tue, 12 May 2020 18:58:00 -0700 Subject: [PATCH 2/3] Change files --- ...tive-windows-2020-05-12-18-57-59-UnsafePropertyId.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 change/react-native-windows-2020-05-12-18-57-59-UnsafePropertyId.json diff --git a/change/react-native-windows-2020-05-12-18-57-59-UnsafePropertyId.json b/change/react-native-windows-2020-05-12-18-57-59-UnsafePropertyId.json new file mode 100644 index 00000000000..2841e2d184d --- /dev/null +++ b/change/react-native-windows-2020-05-12-18-57-59-UnsafePropertyId.json @@ -0,0 +1,8 @@ +{ + "type": "prerelease", + "comment": "Allow storing non-WinRT types in ReactPropertyBag", + "packageName": "react-native-windows", + "email": "vmorozov@microsoft.com", + "dependentChangeType": "patch", + "date": "2020-05-13T01:57:59.370Z" +} From 9972ab8f8ecbdac2bb5551a227fc877f6bbdd255 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Tue, 12 May 2020 20:43:25 -0700 Subject: [PATCH 3/3] Addressed PR feedback --- vnext/Microsoft.ReactNative.Cxx/BoxedValue.h | 2 +- vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vnext/Microsoft.ReactNative.Cxx/BoxedValue.h b/vnext/Microsoft.ReactNative.Cxx/BoxedValue.h index 86add9e7cec..841391a718b 100644 --- a/vnext/Microsoft.ReactNative.Cxx/BoxedValue.h +++ b/vnext/Microsoft.ReactNative.Cxx/BoxedValue.h @@ -14,7 +14,7 @@ struct BoxedValue : implements, IBoxedValue> { template BoxedValue(TArgs &&... args) noexcept : m_value(std::forward(args)...) {} - int64_t GetPtr() noexcept { + int64_t GetPtr() const noexcept { return reinterpret_cast(&m_value); } diff --git a/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h b/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h index 4148e33d9ee..fcf75ff7a22 100644 --- a/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h +++ b/vnext/Microsoft.ReactNative.Cxx/ReactPropertyBag.h @@ -118,7 +118,7 @@ struct ReactPropertyId { // ReactPropertyBag is a wrapper for IReactPropertyBag to store strongly-typed properties in a thread-safe way. // Types inherited from IInspectable are stored directly. // Values of other types are boxed with help of winrt::box_value. -// Non-WinRT types are wrapped with help of BoxedValue template. +// Non-WinRT types are wrapped with the help of BoxedValue template. struct ReactPropertyBag { // Property result type is either T or std::optional. // T is returned for types inherited from IInspectable.