From 74e6c95572bc2fdfd32ebe53d0d997926ffcff4f Mon Sep 17 00:00:00 2001 From: David Vacca Date: Wed, 21 Jun 2023 19:26:40 -0700 Subject: [PATCH] Refactor integration of BridgelessReactPackage into ReactHost (#38010) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/38010 This diff refactors the integration of ReactPackages into ReactHost and ReactHostDelegate. As part of this diff I'm also modifying ReactHostDelegate to depend on TurboModuleManagerDelegate.Builder instead of TurboModuleManagerDelegateBuilder. This is necessary to be able to create BridgelessReactPackage inside ReactInstance bypass-github-export-checks changelog: [internal] internal Reviewed By: luluwu2032 Differential Revision: D46410795 fbshipit-source-id: 221f0f5ce06b7c57410dc4d351d1a1eae29f2733 --- .../react/bridgeless/ReactHostDelegate.kt | 12 ++++------- .../react/bridgeless/ReactInstance.java | 20 ++++++++++++++++--- .../defaults/DefaultReactHostDelegate.kt | 11 +++------- .../react/bridgeless/ReactHostDelegateTest.kt | 8 ++++---- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostDelegate.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostDelegate.kt index 3d9ab63ad5a..1a409a98aab 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostDelegate.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostDelegate.kt @@ -9,12 +9,11 @@ package com.facebook.react.bridgeless import com.facebook.infer.annotation.ThreadSafe import com.facebook.react.ReactPackage +import com.facebook.react.ReactPackageTurboModuleManagerDelegate import com.facebook.react.bridge.JSBundleLoader -import com.facebook.react.bridge.ReactApplicationContext import com.facebook.react.common.annotations.UnstableReactNativeAPI import com.facebook.react.fabric.ReactNativeConfig import com.facebook.react.turbomodule.core.TurboModuleManager -import com.facebook.react.turbomodule.core.TurboModuleManagerDelegate /** * [ReactHostDelegate] is an interface that defines parameters required to initialize React Native. @@ -49,7 +48,7 @@ interface ReactHostDelegate { val jSBundleLoader: JSBundleLoader /** TODO: combine getTurboModuleManagerDelegate inside [ReactPackage] */ - fun getTurboModuleManagerDelegate(context: ReactApplicationContext): TurboModuleManagerDelegate + val turboModuleManagerDelegateBuilder: ReactPackageTurboModuleManagerDelegate.Builder /** * Callback that can be used by React Native host applications to react to exceptions thrown by @@ -68,17 +67,14 @@ interface ReactHostDelegate { override val jSMainModulePath: String, override val jSBundleLoader: JSBundleLoader, override val jSEngineInstance: JSEngineInstance, + override val turboModuleManagerDelegateBuilder: + ReactPackageTurboModuleManagerDelegate.Builder, override val reactPackages: List = emptyList(), override val bindingsInstaller: BindingsInstaller? = null, - private val turboModuleManagerDelegate: - (context: ReactApplicationContext) -> TurboModuleManagerDelegate, private val reactNativeConfig: ReactNativeConfig = ReactNativeConfig.DEFAULT_CONFIG, private val exceptionHandler: (error: Exception) -> Unit = {} ) : ReactHostDelegate { - override fun getTurboModuleManagerDelegate(context: ReactApplicationContext) = - turboModuleManagerDelegate(context) - override fun getReactNativeConfig(turboModuleManager: TurboModuleManager) = reactNativeConfig override fun handleInstanceException(error: Exception) = exceptionHandler(error) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactInstance.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactInstance.java index 85dbe4abe06..7f5c3acf492 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactInstance.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactInstance.java @@ -16,6 +16,7 @@ import com.facebook.infer.annotation.ThreadConfined; import com.facebook.infer.annotation.ThreadSafe; import com.facebook.jni.HybridData; import com.facebook.proguard.annotations.DoNotStrip; +import com.facebook.react.BridgelessReactPackage; import com.facebook.react.ReactPackage; import com.facebook.react.ViewManagerOnDemandReactPackage; import com.facebook.react.bridge.JSBundleLoader; @@ -78,6 +79,7 @@ final class ReactInstance { private final ReactHostDelegate mDelegate; private final BridgelessReactContext mBridgelessReactContext; + private final List mReactPackages; private final ReactQueueConfiguration mQueueConfiguration; private final TurboModuleManager mTurboModuleManager; @@ -186,8 +188,20 @@ final class ReactInstance { // Set up TurboModules Systrace.beginSection( Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstance.initialize#initTurboModules"); + + mReactPackages = mDelegate.getReactPackages(); + mReactPackages.add( + new BridgelessReactPackage( + bridgelessReactContext.getDevSupportManager(), + bridgelessReactContext.getDefaultHardwareBackBtnHandler())); + TurboModuleManagerDelegate turboModuleManagerDelegate = - mDelegate.getTurboModuleManagerDelegate(mBridgelessReactContext); + mDelegate + .getTurboModuleManagerDelegateBuilder() + .setPackages(mReactPackages) + .setReactApplicationContext(mBridgelessReactContext) + .build(); + mTurboModuleManager = new TurboModuleManager( // Use unbuffered RuntimeExecutor to install binding @@ -442,7 +456,7 @@ final class ReactInstance { private @Nullable ViewManager createViewManager(String viewManagerName) { if (mDelegate != null) { - List packages = mDelegate.getReactPackages(); + List packages = mReactPackages; if (packages != null) { synchronized (packages) { for (ReactPackage reactPackage : packages) { @@ -465,7 +479,7 @@ final class ReactInstance { private @NonNull Collection getViewManagerNames() { Set uniqueNames = new HashSet<>(); if (mDelegate != null) { - List packages = mDelegate.getReactPackages(); + List packages = mReactPackages; if (packages != null) { synchronized (packages) { for (ReactPackage reactPackage : packages) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactHostDelegate.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactHostDelegate.kt index c56417f35f8..f789bdd03ac 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactHostDelegate.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactHostDelegate.kt @@ -9,8 +9,8 @@ package com.facebook.react.defaults import com.facebook.jni.annotations.DoNotStrip import com.facebook.react.ReactPackage +import com.facebook.react.ReactPackageTurboModuleManagerDelegate import com.facebook.react.bridge.JSBundleLoader -import com.facebook.react.bridge.ReactApplicationContext import com.facebook.react.bridgeless.BindingsInstaller import com.facebook.react.bridgeless.JSEngineInstance import com.facebook.react.bridgeless.ReactHostDelegate @@ -18,7 +18,6 @@ import com.facebook.react.bridgeless.hermes.HermesInstance import com.facebook.react.common.annotations.UnstableReactNativeAPI import com.facebook.react.fabric.ReactNativeConfig import com.facebook.react.turbomodule.core.TurboModuleManager -import com.facebook.react.turbomodule.core.TurboModuleManagerDelegate /** * A utility class that allows you to simplify the initialization of React Native by setting up a @@ -46,15 +45,11 @@ class DefaultReactHostDelegate( override val reactPackages: List = emptyList(), override val jSEngineInstance: JSEngineInstance = HermesInstance(), override val bindingsInstaller: BindingsInstaller = DefaultBindingsInstaller(), - private val turboModuleManagerDelegate: - (context: ReactApplicationContext) -> TurboModuleManagerDelegate, private val reactNativeConfig: ReactNativeConfig = ReactNativeConfig.DEFAULT_CONFIG, - private val exceptionHandler: (Exception) -> Unit = {} + private val exceptionHandler: (Exception) -> Unit = {}, + override val turboModuleManagerDelegateBuilder: ReactPackageTurboModuleManagerDelegate.Builder ) : ReactHostDelegate { - override fun getTurboModuleManagerDelegate(context: ReactApplicationContext) = - turboModuleManagerDelegate(context) - override fun getReactNativeConfig(turboModuleManager: TurboModuleManager) = reactNativeConfig override fun handleInstanceException(error: Exception) = exceptionHandler(error) diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostDelegateTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostDelegateTest.kt index 4bc2ed9c6b2..fc89a8fc46e 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostDelegateTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostDelegateTest.kt @@ -7,11 +7,11 @@ package com.facebook.react.bridgeless +import com.facebook.react.ReactPackageTurboModuleManagerDelegate import com.facebook.react.bridge.JSBundleLoader import com.facebook.react.bridgeless.hermes.HermesInstance import com.facebook.react.common.annotations.UnstableReactNativeAPI import com.facebook.react.defaults.DefaultReactHostDelegate -import com.facebook.react.turbomodule.core.TurboModuleManagerDelegate import com.facebook.testutils.shadows.ShadowSoLoader import org.assertj.core.api.Assertions.assertThat import org.junit.Test @@ -32,8 +32,8 @@ class ReactHostDelegateTest { @Test fun testDefaultReactHostDelegateCreation() { val jsBundleLoader: JSBundleLoader = Mockito.mock(JSBundleLoader::class.java) - val turboModuleManagerDelegateMock: TurboModuleManagerDelegate = - Mockito.mock(TurboModuleManagerDelegate::class.java) + val turboModuleManagerDelegateBuilderMock: ReactPackageTurboModuleManagerDelegate.Builder = + Mockito.mock(ReactPackageTurboModuleManagerDelegate.Builder::class.java) val hermesInstance: JSEngineInstance = Mockito.mock(HermesInstance::class.java) val jsMainModulePathMocked = "mockedJSMainModulePath" val delegate = @@ -41,7 +41,7 @@ class ReactHostDelegateTest { jSMainModulePath = jsMainModulePathMocked, jSBundleLoader = jsBundleLoader, jSEngineInstance = hermesInstance, - turboModuleManagerDelegate = { turboModuleManagerDelegateMock }) + turboModuleManagerDelegateBuilder = turboModuleManagerDelegateBuilderMock) assertThat(delegate.jSMainModulePath).isEqualTo(jsMainModulePathMocked) }