Add requiresImmediateCxxNativeModuleSetup property to RCTCxxModule and immediately initialize CxxNativeModule if true.

[tomduncalf]

This allows for CxxModules with methods which require a bridge instance (for example, emitting an event) to be called from native code, without requiring them to be initialised by some other means (e.g. being called from Javascript, or calling getMethods) first.

Rationale

I have been experimenting with using pure C++ modules with React Native, as our application is made up of a C++ core with a React Native UI, and makes use of a lot of communication both ways across the bridge. Creating Objective C wrappers for every call has proven to be quite verbose, so native C++ modules would be quite a productivity and code complexity win.

Unfortunately there isn't a great deal of documentation around this topic, so I've been figuring it out as I go along.

I was able to create a CxxModule accessible from Javascript, but as described in #18509, I've been having issues getting a reference to the CxxModule from native code in order to emit events from C++ to Javascript.

The crux of the issue seems to be that the module instance returned from e.g. moduleForName is the CxxModule rather than CxxNativeModule instance, and so has no _instance set, and therefore cannot communicate back to React Native. In addition to this, the module is not initialised until you either call into it from Javascript or call e.g. getMethods, and accessing modules this way requires Objective C code to access the bridge and get the module instance so is not pure C++.

I've spent quite a lot of time looking at the React Native source code, attempting to determine why things work the way they do, and have considered several approaches to solving this problem (e.g. providing a way to get the CxxNativeModule from the module registry via the bridge), but after a lot of experimentation, I decided upon the approach in this PR. It is the smallest change I could think of to achieve the goals, and is low risk in the sense that it opt-in and does not modify the default behaviour of React Native.

Example code

I've created a simple example of how this can be used at https://github.com/tomduncalf/react-native-cxx-module/tree/working - the relevant part is in https://github.com/tomduncalf/react-native-cxx-module/tree/working/ios/rnfresh/ReactNative, and the changes from the "not working" version can be seen at tomduncalf/react-native-cxx-module@40235ce

By specifying that a module requires immediate CxxNativeModule setup, we know that the constructor of the CxxModule will be called at app startup time, then we can store a reference to it somewhere shared (in this case, the ModuleRegistry, which could equally be a singleton), and we can then access a fully working instance of our module anywhere from our native codebase.

Note that this code is currently iOS only - any advice on the right approach for Android would be welcome, but I am currently not targetting Android so have no device to test on.

If this is not the right approach I'd be very happy to discuss further - I thought this was a good way to get the ball rolling, as there are no docs about this and it would be a massive help for our project!

Test Plan

Tested in existing project and in the simple example above. There appear to be no automated tests around this area, but the change is low risk in that it does not change defaults.

Related PRs

There is no related documentation, but I'd be happy to create some if we are happy with this approach!

Release Notes

[INTERNAL] [ENHANCEMENT] [CxxModule] - Allow a RCTCxxModule to specify that it requiresImmediateCxxNativeModuleSetup, which will cause the module to be immediately initialised rather than lazily.

[tomduncalf]

Sorry, will look into the test failure Monday

[tomduncalf]

Actually looks the failure is due to some Haste issue, don’t think it’s caused by my changes - can I rerun the tests?

[facebook-github-bot]

@tomduncalf I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[tomduncalf]

Hi @mhorowitz, you seem like the person who has worked most on this code – is there any chance you could take a look at this? Thanks!

[facebook-github-bot]

@tomduncalf I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[oNaiPs]

@tomduncalf you might be able to get away without the changes if you reimplement RCTCxxModule outside of the react native sources

[tomduncalf]

True. For now we are working around it by calling in from JavaScript at startup time, which works okay but feels messy!

…

On 21 Jun 2018, at 02:00, José Luis Pereira ***@***.***> wrote: @tomduncalf you might be able to get away without the changes if you reimplement RCTCxxModule outside of the react native sources — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[matthargett]

why not rename lazyInit() and make it public instead?

also, can you test in an Android simulator at least? I can advocate for this change, and understand the value, adding no ability to manually test in RNTester or on Expo snack makes it difficult.

thanks!

[tomduncalf]

Yeah that would probably be a valid approach too. Unfortunately it’s not been possible to get any RN contributors attention on this so as I mentioned above, our solution for now is to call into the module from JS at app start up time to make it initialise - would rather avoid forking/hacks if possible. I believe they are making large changes to some of the native interop stuff in React Native next year, so it might be that a different solution emerges then!

…

On 10 Nov 2018, at 01:17, Matt Hargett ***@***.***> wrote: why not rename lazyInit() and make it public instead? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[cpojer]

Thank you for this PR. I think this somewhat conflicts with our plans for TurboModules, which should support your use case in some form as well once it is ready to use. See react-native-community/discussions-and-proposals#40 (It seems like you are already giving input there, but just making sure there is a link for other people who may see this PR).

[tomduncalf]

Thanks Christoph - yeah, I’m already investigating JSI and turbo modules and I think it will suit our use case perfectly so I’m happy for this to be closed.

…

On 29 Jan 2019, at 17:19, Christoph Nakazawa ***@***.***> wrote: Thank you for this PR. I think this somewhat conflicts with our plans for TurboModules, which should support your use case in some form as well once it is ready to use. See react-native-community/discussions-and-proposals#40 (It seems like you are already giving input there, but just making sure there is a link for other people who may see this PR). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.