facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
116.27k stars 23.99k forks source link

Add BindingsInstaller for TurboModules on iOS #44486

Open Kudo opened 1 week ago

Kudo commented 1 week ago

Summary:

Add synchronous JS bindings installation for TurboModules. That would help some 3rd party JSI based modules to install JS bindings easier. Re-create from #43110 but for iOS

Changelog:

[IOS] [ADDED] - Add BindingsInstaller for TurboModules

Test Plan:

Added test in RN-Tester TurboModule test case

analysis-bot commented 1 week ago
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,495,560 -45,615
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,867,614 -44,415
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 26fc40d0f7a9c5a4880d97b8853350872f1890f0 Branch: main

RSNara commented 1 week ago

@Kudo, I think there might be another way to accomplish this.

What if we could initialize c++ modules with (application or react native)-provided java/objc dependencies? Would that be enough to fulfill your use-case? Also, to confirm, is this your use-case: We need to expose a jsi::HostFunction to javascript, that allows us to reach into java objects, and do java things, right?

There is a way to extend the C++ turbomodule infra, like how I described:

  1. Everyone registers their C++ turbomodules using CxxReactPackages (java <-> c++ hybrid objects).
  2. We extend auto-linking to pick up, coalesce, and register CxxReactPackages (like how it does ReactPackages).

This makes registering c++ only modules a bit harder. Right now, you just drop them into your code-base. But, c++ modules are already an advanced feature, that we only expect advanced users to use. So, maybe it's fine.

philIip commented 1 week ago

@Kudo, I think there might be another way to accomplish this.

What if we could initialize c++ modules with (application or react native)-provided java/objc dependencies? Would that be enough to fulfill your use-case? Also, to confirm, is this your use-case: We need to expose a jsi::HostFunction to javascript, that allows us to reach into java objects, and do java things, right?

There is a way to extend the C++ turbomodule infra, like how I described:

  1. Everyone registers their C++ turbomodules using CxxReactPackages (java <-> c++ hybrid objects).
  2. We extend auto-linking to pick up, coalesce, and register CxxReactPackages (like how it does ReactPackages).

This makes registering c++ only modules a bit harder. Right now, you just drop them into your code-base. But, c++ modules are already an advanced feature, that we only expect advanced users to use. So, maybe it's fine.

i think everyone is aligned with you on this being a nice north star for C++ TM, but this native hook can be seen as an intermediate step. and if implemented properly, this native hook should add minimal surface to our API surface area, and as u said it's an advanced use case - so is it worth all of the effort to build autolinking, etc. for < 10 libs? not sure yet

i also responded to ur internal WP comment ! let's chat further if needed :)

Kudo commented 1 week ago

apply review comments to the pr and split android implementation to #44526 as requested. though GH doesn't support stacked prs, the two commits are cloned:

facebook-github-bot commented 1 week ago

@philIip has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.