Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for C++ TurboModules Autolinking #2296

Merged
merged 7 commits into from
Feb 19, 2024

Conversation

cortinico
Copy link
Member

@cortinico cortinico commented Feb 12, 2024

Summary:

This adds support for C++ TurboModules Autolinking on Android.

I added those properties in the config command:

platforms.android.cxxModuleCMakeListsModuleName

Note: Only applicable when new architecture is turned on and for C++ TurboModules.

The name of the CMake target that is building the C++ Turbo Module. This is generally
different from the codegen target which is called react_codegen_<yourlibrary>.
You will need to create a custom CMakeLists.txt, which defines a target, depends on react_codegen_<yourlibrary> and builds your C++ Turbo Module.
Specify the name of the target here so that it can be properly linked to the rest of the app project.

platforms.android.cxxModuleCMakeListsPath

Note: Only applicable when new architecture is turned on and for C++ TurboModules.

A relative path to a custom CMakeLists.txt file that is used to build a C++ TurboModule. Relative to sourceDir.

platforms.android.cxxModuleHeaderName

Note: Only applicable when new architecture is turned on and for C++ TurboModules.

The name of the C++ TurboModule. Your C++ TurboModule generally should implement one of the JSI interface generated by Codegen. You need to specify the name of the class that is implementing the generated interface here, so that it can properly be linked in the rest of the app project.

We expect that your module exposes a filed called kModuleName which is a string that contains the name of the module. This field is properly populated by codegen, but if you don't use codegen, you will have to make sure that this field is present.

Say your module name is NativeCxxModuleExample, it will be included in the autolinking logic as follows:

// We will include a header called as your module
#include <NativeCxxModuleExample.h>

...

std::shared_ptr<TurboModule> rncli_cxxModuleProvider(
    const std::string& name,
    const std::shared_ptr<CallInvoker>& jsInvoker) {

  // Here we expect you have a field called kModuleName
  if (name == NativeCxxModuleExample::kModuleName) {
    // Here we initialize your module
    return std::make_shared<NativeCxxModuleExample>(jsInvoker);
  }
  return nullptr;
}

Test Plan:

I had to test it manually in a nightly project by tampering the node_modules folder and I managed to make it work. Not sure how I can test it more.

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

@github-actions github-actions bot added the docs Documentation change label Feb 12, 2024
@thymikee
Copy link
Member

So happy to see this! Let me know if we can help anyhow

@cortinico cortinico marked this pull request as ready for review February 16, 2024 13:17
@cortinico cortinico changed the title Add support for C++ TurboModules Autolinking feat: Add support for C++ TurboModules Autolinking Feb 16, 2024
@cortinico
Copy link
Member Author

docs/dependencies.md Outdated Show resolved Hide resolved
Co-authored-by: Szymon Rybczak <szymon.rybczak@gmail.com>
@WoLewicki
Copy link
Contributor

@cortinico did you maybe check what was needed for the autolinking to work in this PR: software-mansion/react-native-gesture-handler#2708? Would be nice to cover all the needed parts 🎉

@cortinico
Copy link
Member Author

@cortinico did you maybe check what was needed for the autolinking to work in this PR: software-mansion/react-native-gesture-handler#2708? Would be nice to cover all the needed parts 🎉

@WoLewicki I've briefly checked and it seems like Gesture Handler would fit the current setup for TM CXX Autolinking I'm proposing here (we'll have to adapt the setup a bit)

cortinico added a commit to cortinico/react-native that referenced this pull request Feb 19, 2024
… from RNCLI (facebook#43049)

Summary:

This connects the OnLoad.cpp file used by OSS apps with the `rncli_cxxModuleProvider`.
This method is created by the CLI and takes care of querying all the TM CXX Modules discovered and returning them.

This PR is currently waiting on react-native-community/cli#2296

Changelog:
[Internal] [Changed] - Hook the default-app-setup OnLoad.cpp file with the cxxModuleProvider from RNCLI

Reviewed By: cipolleschi

Differential Revision: D53812109
Copy link
Collaborator

@szymonrybczak szymonrybczak left a comment

Choose a reason for hiding this comment

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

:shipit:

@cortinico cortinico merged commit 77a3a43 into main Feb 19, 2024
10 checks passed
@cortinico cortinico deleted the nc/cxx-tm-autolinking branch February 19, 2024 14:12
cortinico added a commit to cortinico/react-native that referenced this pull request Feb 19, 2024
… from RNCLI (facebook#43049)

Summary:
Pull Request resolved: facebook#43049

This connects the OnLoad.cpp file used by OSS apps with the `rncli_cxxModuleProvider`.
This method is created by the CLI and takes care of querying all the TM CXX Modules discovered and returning them.

This PR is currently waiting on react-native-community/cli#2296

Changelog:
[Internal] [Changed] - Hook the default-app-setup OnLoad.cpp file with the cxxModuleProvider from RNCLI

Reviewed By: cipolleschi

Differential Revision: D53812109
cortinico added a commit to cortinico/react-native that referenced this pull request Feb 19, 2024
… from RNCLI (facebook#43049)

Summary:
Pull Request resolved: facebook#43049

This connects the OnLoad.cpp file used by OSS apps with the `rncli_cxxModuleProvider`.
This method is created by the CLI and takes care of querying all the TM CXX Modules discovered and returning them.

This PR is currently waiting on react-native-community/cli#2296

Changelog:
[Internal] [Changed] - Hook the default-app-setup OnLoad.cpp file with the cxxModuleProvider from RNCLI

Reviewed By: cipolleschi

Differential Revision: D53812109
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Feb 20, 2024
… from RNCLI (#43049)

Summary:
Pull Request resolved: #43049

This connects the OnLoad.cpp file used by OSS apps with the `rncli_cxxModuleProvider`.
This method is created by the CLI and takes care of querying all the TM CXX Modules discovered and returning them.

This PR is currently waiting on react-native-community/cli#2296

Changelog:
[Internal] [Changed] - Hook the default-app-setup OnLoad.cpp file with the cxxModuleProvider from RNCLI

Reviewed By: cipolleschi

Differential Revision: D53812109

fbshipit-source-id: 47bc0ea699516993070cfa0127de97853acf8890
huntie pushed a commit to facebook/react-native that referenced this pull request Feb 22, 2024
… from RNCLI (#43049)

Summary:
Pull Request resolved: #43049

This connects the OnLoad.cpp file used by OSS apps with the `rncli_cxxModuleProvider`.
This method is created by the CLI and takes care of querying all the TM CXX Modules discovered and returning them.

This PR is currently waiting on react-native-community/cli#2296

Changelog:
[Internal] [Changed] - Hook the default-app-setup OnLoad.cpp file with the cxxModuleProvider from RNCLI

Reviewed By: cipolleschi

Differential Revision: D53812109

fbshipit-source-id: 47bc0ea699516993070cfa0127de97853acf8890
@mrousavy
Copy link
Member

Is there any example C++ TurboModule that autolinks? I can't seem to get the CMakeLists configuration right for react-native-mmkv...

@cortinico
Copy link
Member Author

Is there any example C++ TurboModule that autolinks? I can't seem to get the CMakeLists configuration right for react-native-mmkv...

Sadly not. I had it working on my local setup, and I haven't had the time to work on samples or further docs.

@mrousavy
Copy link
Member

mrousavy commented Mar 25, 2024

Is my understanding correct that there are 3 CMakeLists.txt files per CxxTurboModule?

  1. One in cpp/CMakeLists.txt that builds the TurboModule (incl. all dependencies like MMKVCore, Skia, OpenGL or whatever)
  2. One in android/CMakeLists.txt that just depends on the one in ../cpp/ (this one sounds like it's not needed?)
  3. One generated by CodeGen (android/build/generated/...)

And then my react-native.config.js should look like this:

// https://github.com/react-native-community/cli/blob/main/docs/dependencies.md

module.exports = {
  dependency: {
    platforms: {
      /**
       * @type {import('@react-native-community/cli-types').IOSDependencyParams}
       */
      ios: {},
      /**
       * @type {import('@react-native-community/cli-types').AndroidDependencyParams}
       */
      android: {
        cxxModuleCMakeListsModuleName: 'reactnativemmkv',
        cxxModuleCMakeListsPath: './CMakeLists.txt',
        cxxModuleHeaderName: 'NativeMmkvModule',
      },
    },
  },
};

Not sure what I'm missing, but I can't seem to get that to build/link properly... Code is here: react-native-mmkv (feat/turbomodule)

The error I'm seeing right now is this:

CMake Error at /Users/mrousavy/Projects/react-native-mmkv/example/node_modules/react-native/ReactAndroid/cmake-utils/ReactNative-application.cmake:42 (add_library):
  add_library cannot create target "appmodules" because another target with
  the same name already exists.  The existing target is a shared library
  created in source directory
  "/Users/mrousavy/Projects/react-native-mmkv/example/node_modules/react-native/ReactAndroid/cmake-utils/default-app-setup".
  See documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
  CMakeLists.txt:31 (include)

..which seems to happen in ReactNative-application.cmake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation change feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants