-
Notifications
You must be signed in to change notification settings - Fork 24.7k
proposal: BundleConsumer interface for TurboModules #50788
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
## Summary To execute the bundle on Worklet Runtimes we need to somehow access it. Unfortunately, currently there are no APIs for that and we need to patch React Native to expose them. Once we settle for a good API both internally and externally I will upstream these changes to facebook/react-native#50788 (review) ## Test plan Fabric example works on: - [x] iOS debug - [x] iOS prod - [x] android debug - [x] android prod
fun <TInterface> getModulesConformingToInterfaceNames(clazz: Class<TInterface>): List<String> { | ||
val moduleNames = mutableListOf<String>() | ||
|
||
for (moduleProvider in moduleProviders) { | ||
val moduleInfos = packageModuleInfos[moduleProvider]?.values ?: continue | ||
for (moduleInfo in moduleInfos) { | ||
val module = moduleProvider.getModule(moduleInfo.name) | ||
if (clazz.isInstance(module)) { | ||
moduleNames.add(moduleInfo.name) | ||
} | ||
} | ||
} | ||
return moduleNames | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is obviously unacceptable, because this code actually creates the modules and discards them. I tried to work around this, but the class names of TurboModules are also not stored fully, i.e. instead of classname being com.facebook.react.SomeClass
only SomeClass
is kept, therefore Class.forName(...)
doesn't work either.
I was wondering if maybe the user could optionally add the Class to ReactModuleInfo
so we could reflect in runtime if it's satisfying some interface.
Is there a particular reason why is the Class implementing the TurboModule not available at this point?
cc @RSNara since I noticed you'd been working a bit on TurboModules some time ago.
val sourceURL = assetURL.removePrefix("assets://") | ||
val script = BigStringBufferWrapper(assetManager, sourceURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved string stripping from C++ here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an Obj-C wrapper of a shared_ptr to the BigStringBuffer containing the string. This way it can be passed to TurboModules as NSObject.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is Java hybrid object wrapping a shared_ptr to the BigStringBuffer containing the string. This way it can be passed to TurboModules as a Java Object and the pointer can be obtained from C++.
void JReactInstance::loadJSBundleFromAssets( | ||
jni::alias_ref<JAssetManager::javaobject> assetManager, | ||
const std::string& assetURL) { | ||
const int kAssetsLength = 9; // strlen("assets://"); | ||
auto sourceURL = assetURL.substr(kAssetsLength); | ||
|
||
auto manager = extractAssetManager(assetManager); | ||
auto script = loadScriptFromAssets(manager, sourceURL); | ||
instance_->loadScript(std::move(script), sourceURL); | ||
} | ||
|
||
void JReactInstance::loadJSBundleFromFile( | ||
const std::string& fileName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to BigStringBufferWrapper.cpp
.
if (conformsToProtocols.includes('RCTBundleConsumer')) { | ||
bundleConsumerModules.add(moduleName); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converesly to Android, I could get classes conforming to an interface through available APIs in the codegen.
I don't like it that it's in generateCustomURLHandlers.js
but I assume the sole usage of the API isn't bad:
"codegenConfig": {
"ios": {
"modulesConformingToProtocol": {
"RCTBundleConsumer": [
"MyModule"
]
}
}
}
|
||
#import <Foundation/Foundation.h> | ||
|
||
#ifdef __cplusplus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a hard time compiling it without these #ifdef __cplusplus
macros.
Summary:
With the development of
react-native-worklets
library I want to align it more with the tools inside React Native ecosystem and reduce the amount of hacks needed to provide multithreading JavaScript solutions. One of the steps there is to obtain multi-threading (byte)code via Metro, instead of Babel tricks andevalWithSourceMap
. For this sake it would be very handy to have an interface a Turbo Module could implement, that would allow it to get the bundle and all hot-reload updates. This would also benefit other libraries that are featuring multi-threading JavaScript.I have been dogfooding these changes successfully within Reanimated repo.
I don't fully grasp the directory tree in the repo, therefore some new files might appear to be located in random directories.
The PR could be split into three parts, I guess ideally if it's accepted I'd also split it into 3 parts.
feat: TurboModuleManager API to check if a TurboModule implements a given interface
If a TurboModule wants to obtain the bundle, it should implement a BundleConsumer interface. However, the bundle is obtained early, before all library TurboModules are created. Therefore we don't know which TurboModules should we instantiate to provide them with the bundle.
I managed to handle it with codegen on iOS, but on Android it didn't seem to be an option. I think on Android an optional field in
ReactModuleInfo
could be added that would hold the class of the TurboModule.refactor: pass the JSBundle as
std::shared_ptr<const BigStringBuffer>
instead ofstd::unique_ptr<DataBigString>
The idea behind bundle consuming modules is that they would all share the bundle, so we'd not duplicate it in memory. For this purpose we pass the evaluatable
std::shared_ptr<const BigStringBuffer>
instead ofstd::shared_ptr<DataBigString>
, because the latter has to be converted toBigStringBuffer
before evaluating it.feat: BundleConsumer interface for TurboModules
An interface for TurboModules, that allows them to obtain the JSBundle and sourceURL. These modules would be instantiated before the JSBundle is evaluated and this is when they would obtain the JSBundle and sourceURL. Since the bundle is passed as a
const std::shared_ptr<const BigStringBuffer>
they cannot influence React Native's own evaluation of the bundle.Changelog:
[GENERAL] [ADDED] - Add BundleConsumer interface for TurboModules to access the JS bundle
Test Plan:
I made
SampleTurboModule
inRNTester
to implement BundleConsumer interface and it worked fine.I'm also dogfooding these changes in Reanimated repo, see the relevant patches.