Skip to content

Conversation

@vmoroz
Copy link
Member

@vmoroz vmoroz commented May 21, 2020

This change is to address a customer need to be able to get a std::weak_ptr to a native module in case if the module must subscribe to some events. Since, a native module lifetime is bound to the React instance lifetime, we cannot use this pointer and we better use the std::weak_ptr<T>.
In this PR we enable the native modules to be inherited from the std::enable_shared_from_this<T> that allows use of the weak_from_this() function to get std::weak_ptr<T> to the module.

To support the new functionality we are doing the following changes:

  • Introduce a mechanism for a module to associate a custom factory function with a module type.
  • In case if custom factory is not provided then we offer two default native module factories:
    • MakeDefaultReactModuleWrapper - the default factory that wraps up native module into ReactNonAbiValue<TModule> to pass the module across the ABI boundary.
    • MakeDefaultSharedPtrReactModuleWrapper - the default factory that wraps up native module into ReactNonAbiValue<shared_ptr<TModule>> in case if module is inherited from std::enable_shared_from_this<T>.

The custom factory can be associated with a module type by providing GetReactModuleFactory function overload that has two arguments Module * pointer type and int. The int parameter is used to give the overload higher priority than the default GetReactModuleFactory function where we use int * when we pass 0 as a second argument.

To show the use of the std::weak_ptr we have added a new SampleSharedCppModule module to the sample project. It shows how the std::weak_ptr can be used with help of the std::enable_shared_from_this base and weak_from_this() method call.

Microsoft Reviewers: Open in CodeFlow

@vmoroz vmoroz requested review from a team, acoates-ms and aeulitz May 21, 2020 20:39
@vmoroz vmoroz requested a review from a team as a code owner May 21, 2020 20:39
Copy link

@NikoAri NikoAri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@vmoroz vmoroz added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label May 21, 2020
@ghost
Copy link

ghost commented May 21, 2020

Hello @vmoroz!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours, a condition that will be fulfilled in about 7 hours 48 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit c23d3de into microsoft:master May 22, 2020
ZihanChen-MSFT pushed a commit to ZihanChen-MSFT/react-native-windows that referenced this pull request May 22, 2020
nasadigital pushed a commit to nasadigital/react-native-windows that referenced this pull request May 26, 2020
@NickGerleman
Copy link
Contributor

Removing "Backport to 0.62" since we're about to rename it to "Backported to 0.62" as part of backporting label migration. This has been approved to go into 0.62 though, and such has the "Backport approved" label.

ghost pushed a commit that referenced this pull request Jun 6, 2020
* Implemented support for native module std::weak_ptr (#4980)

(cherry picked from commit c23d3de)

# Conflicts:
#	packages/microsoft-reactnative-sampleapps/windows/SampleLibraryCPP/SampleModuleCPP.h

* Fix ReactInstance error state to avoid crashes (#4986)

* Fix ReactInstance error state to avoid crashes

* Change files

* Clean m_redboxContent after RedBox closing

(cherry picked from commit 05779f2)

* ReactNotificationService to allow communication between native modules (#4953)

Only picking up changes required for ReactDispatcher.

(cherry picked from commit baba475)

* Add UIDispatcher property to ReactInstanceSettings and IReactContext (#5007)

* Add UIDispatcher property to ReactInstanceSettings and IReactContext

* Change files

* Fix ReactWindows-Desktop.sln compilation

* Address PR feedback for the GetCurrentUIThreadQueue

* Remove UIDispatcher setting from Playground project

(cherry picked from commit 7236d61)

# Conflicts:
#	packages/playground/windows/playground-win32/Playground-Win32.cpp
#	vnext/Desktop/React.Windows.Desktop.vcxproj.filters
#	vnext/Microsoft.ReactNative.Cxx.UnitTests/ReactContextTest.cpp
#	vnext/Microsoft.ReactNative.Cxx.UnitTests/ReactModuleBuilderMock.h
#	vnext/Microsoft.ReactNative.Cxx/ReactContext.h
#	vnext/Microsoft.ReactNative.Managed.UnitTests/ReactModuleBuilderMock.cs
#	vnext/Microsoft.ReactNative/IReactContext.cpp
#	vnext/Microsoft.ReactNative/IReactContext.h
#	vnext/Microsoft.ReactNative/IReactContext.idl
#	vnext/Microsoft.ReactNative/ReactInstanceSettings.h
#	vnext/Microsoft.ReactNative/ReactInstanceSettings.idl

* Fix PlaygroundWin32 compilation
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants