-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| #import <Foundation/Foundation.h> | ||
| #import <React/NSBigStringBuffer.h> | ||
|
|
||
| /** | ||
| * Provides the interface needed to register a Bundle Consumer module. | ||
| */ | ||
| @protocol RCTBundleConsumer <NSObject> | ||
|
|
||
| @property (nonatomic, strong, readwrite) NSBigStringBuffer *scriptBuffer; | ||
|
|
||
| @property (nonatomic, strong, readwrite) NSString *sourceURL; | ||
|
|
||
| @end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| #import <Foundation/Foundation.h> | ||
|
|
||
| #ifdef __cplusplus | ||
| #import <jsireact/JSIExecutor.h> | ||
| #import <memory> | ||
|
|
||
| using namespace facebook; | ||
| using namespace facebook::react; | ||
| #endif // __cplusplus | ||
|
|
||
| @interface NSBigStringBuffer : NSObject | ||
| #ifdef __cplusplus | ||
|
|
||
| { | ||
| std::shared_ptr<const BigStringBuffer> _buffer; | ||
| } | ||
|
|
||
| - (instancetype)initWithSharedPtr:(const std::shared_ptr<const BigStringBuffer> &)buffer; | ||
| - (const std::shared_ptr<const BigStringBuffer> &)getBuffer; | ||
| #endif // __cplusplus | ||
|
|
||
| @end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| #import "NSBigStringBuffer.h" | ||
|
|
||
| @implementation NSBigStringBuffer | ||
|
|
||
| - (instancetype)initWithSharedPtr:(const std::shared_ptr<const BigStringBuffer>&)buffer { | ||
| if (self = [super init]) { | ||
| _buffer = buffer; | ||
| } | ||
| return self; | ||
| } | ||
|
|
||
| - (const std::shared_ptr<const BigStringBuffer>&)getBuffer { | ||
| return _buffer; | ||
| } | ||
|
|
||
| @end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,6 +149,21 @@ public abstract class ReactPackageTurboModuleManagerDelegate : TurboModuleManage | |
| return resolvedModule as TurboModule | ||
| } | ||
|
|
||
| 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 | ||
| } | ||
|
Comment on lines
+152
to
+165
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I was wondering if maybe the user could optionally add the Class to 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. |
||
|
|
||
| override fun unstable_isModuleRegistered(moduleName: String): Boolean { | ||
| for (moduleProvider in moduleProviders) { | ||
| val moduleInfo: ReactModuleInfo? = packageModuleInfos[moduleProvider]?.get(moduleName) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| package com.facebook.react.bridge | ||
|
|
||
| import com.facebook.react.fabric.BigStringBufferWrapper | ||
|
|
||
| public interface BundleConsumer { | ||
| public fun setScriptWrapper(scriptWrapper: BigStringBufferWrapper) | ||
| public fun setSourceURL(sourceURL: String) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| package com.facebook.react.fabric | ||
|
|
||
| import android.annotation.SuppressLint | ||
| import android.content.res.AssetManager | ||
| import com.facebook.jni.HybridData | ||
| import com.facebook.proguard.annotations.DoNotStripAny | ||
|
|
||
| /** TODO: Description */ | ||
| @SuppressLint("MissingNativeLoadLibrary") | ||
| @DoNotStripAny | ||
| public class BigStringBufferWrapper { | ||
|
|
||
| private val mHybridData: HybridData | ||
|
|
||
| public constructor(fileName: String) { | ||
| mHybridData = initHybridFromFile(fileName) | ||
| } | ||
|
|
||
| public constructor(assetManager: AssetManager, assetURL: String) { | ||
| mHybridData = initHybridFromAssets(assetManager, assetURL) | ||
| } | ||
|
|
||
| private external fun initHybridFromFile(fileName: String): HybridData | ||
|
|
||
| private external fun initHybridFromAssets( | ||
| assetManager: AssetManager, | ||
| assetURL: String | ||
| ): HybridData | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ import com.facebook.react.DebugCorePackage | |
| import com.facebook.react.ReactPackage | ||
| import com.facebook.react.ViewManagerOnDemandReactPackage | ||
| import com.facebook.react.bridge.Arguments | ||
| import com.facebook.react.bridge.BundleConsumer | ||
| import com.facebook.react.bridge.JSBundleLoader | ||
| import com.facebook.react.bridge.JSBundleLoaderDelegate | ||
| import com.facebook.react.bridge.JavaScriptContextHolder | ||
|
|
@@ -42,6 +43,7 @@ import com.facebook.react.common.annotations.FrameworkAPI | |
| import com.facebook.react.common.annotations.UnstableReactNativeAPI | ||
| import com.facebook.react.devsupport.StackTraceHelper | ||
| import com.facebook.react.devsupport.interfaces.DevSupportManager | ||
| import com.facebook.react.fabric.BigStringBufferWrapper | ||
| import com.facebook.react.fabric.ComponentFactory | ||
| import com.facebook.react.fabric.FabricUIManager | ||
| import com.facebook.react.fabric.FabricUIManagerBinding | ||
|
|
@@ -285,6 +287,15 @@ internal class ReactInstance( | |
| } | ||
| } | ||
|
|
||
| fun beforeLoad(scriptWrapper: BigStringBufferWrapper, sourceURL: String){ | ||
| val bundleConsumers = turboModuleManager.getModulesConformingToInterfaceNames(BundleConsumer::class.java) | ||
| for(name in bundleConsumers) { | ||
| val module = turboModuleManager.getModule(name) as BundleConsumer | ||
| module.setScriptWrapper(scriptWrapper); | ||
| module.setSourceURL(sourceURL); | ||
| } | ||
| } | ||
|
|
||
| fun loadJSBundle(bundleLoader: JSBundleLoader) { | ||
| Systrace.beginSection(Systrace.TRACE_TAG_REACT, "ReactInstance.loadJSBundle") | ||
| bundleLoader.loadScript( | ||
|
|
@@ -295,11 +306,18 @@ internal class ReactInstance( | |
| loadSynchronously: Boolean | ||
| ) { | ||
| context.setSourceURL(sourceURL) | ||
| loadJSBundleFromFile(fileName, sourceURL) | ||
|
|
||
| val script = BigStringBufferWrapper(fileName); | ||
|
|
||
| beforeLoad(script, sourceURL); | ||
| loadJSBundle(script, sourceURL) | ||
| } | ||
|
|
||
| override fun loadSplitBundleFromFile(fileName: String, sourceURL: String) { | ||
| loadJSBundleFromFile(fileName, sourceURL) | ||
| val script = BigStringBufferWrapper(fileName) | ||
|
|
||
| beforeLoad(script, sourceURL); | ||
| loadJSBundle(script, sourceURL) | ||
| } | ||
|
|
||
| override fun loadScriptFromAssets( | ||
|
|
@@ -308,7 +326,12 @@ internal class ReactInstance( | |
| loadSynchronously: Boolean | ||
| ) { | ||
| context.setSourceURL(assetURL) | ||
| loadJSBundleFromAssets(assetManager, assetURL) | ||
|
|
||
| val sourceURL = assetURL.removePrefix("assets://") | ||
| val script = BigStringBufferWrapper(assetManager, sourceURL) | ||
|
Comment on lines
+330
to
+331
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved string stripping from C++ here. |
||
|
|
||
| beforeLoad(script, assetURL); | ||
| loadJSBundle(script, assetURL) | ||
| } | ||
|
|
||
| override fun setSourceURLs(deviceURL: String, remoteURL: String) { | ||
|
|
@@ -420,7 +443,7 @@ internal class ReactInstance( | |
| reactHostInspectorTarget: ReactHostInspectorTarget? | ||
| ): HybridData | ||
|
|
||
| private external fun loadJSBundleFromFile(fileName: String, sourceURL: String) | ||
| private external fun loadJSBundle(scriptWrapper: BigStringBufferWrapper, sourceURL: String) | ||
|
|
||
| private external fun loadJSBundleFromAssets(assetManager: AssetManager, assetURL: String) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| #include "BigStringBufferWrapper.h" | ||
| #include <cxxreact/JSBigString.h> | ||
| #include <cxxreact/RecoverableError.h> | ||
|
|
||
| using namespace facebook::jni; | ||
|
|
||
| namespace facebook::react { | ||
| jni::local_ref<BigStringBufferWrapper::jhybriddata> | ||
| BigStringBufferWrapper::initHybridFromFile( | ||
| jni::alias_ref<jhybridobject> jThis, | ||
| std::string fileName) { | ||
| std::unique_ptr<const JSBigFileString> script; | ||
| RecoverableError::runRethrowingAsRecoverable<std::system_error>( | ||
| [&fileName, &script]() { script = JSBigFileString::fromPath(fileName); }); | ||
| auto buffer = std::make_shared<BigStringBuffer>(std::move(script)); | ||
| return makeCxxInstance(buffer); | ||
| } | ||
|
|
||
| jni::local_ref<BigStringBufferWrapper::jhybriddata> | ||
| BigStringBufferWrapper::initHybridFromAssets( | ||
| jni::alias_ref<jhybridobject> jThis, | ||
| jni::alias_ref<JAssetManager::javaobject> assetManager, | ||
| const std::string& sourceURL) { | ||
| auto manager = extractAssetManager(assetManager); | ||
| auto script = loadScriptFromAssets(manager, sourceURL); | ||
| auto buffer = std::make_shared<BigStringBuffer>(std::move(script)); | ||
| return makeCxxInstance(buffer); | ||
| } | ||
|
|
||
| BigStringBufferWrapper::BigStringBufferWrapper( | ||
| const std::shared_ptr<const BigStringBuffer>& script) | ||
| : script_(script) {} | ||
|
|
||
| const std::shared_ptr<const BigStringBuffer> BigStringBufferWrapper::getScript() | ||
| const { | ||
| return script_; | ||
| } | ||
|
|
||
| void BigStringBufferWrapper::registerNatives() { | ||
| registerHybrid( | ||
| {makeNativeMethod( | ||
| "initHybridFromFile", BigStringBufferWrapper::initHybridFromFile), | ||
| makeNativeMethod( | ||
| "initHybridFromAssets", | ||
| BigStringBufferWrapper::initHybridFromAssets)}); | ||
| } | ||
| } // namespace facebook::react |
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 __cplusplusmacros.