From c99d6f60778623ab6694239f2c2bf7c85ebd6d7c Mon Sep 17 00:00:00 2001 From: David Vacca Date: Thu, 25 May 2023 10:29:01 -0700 Subject: [PATCH] Reduce dependencies on ReactHostDelegate (#37564) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/37564 In this diff I'm refactoring ReactHostDelegate to reduce API dependencies. As part of this diff I'm also refactoring callsites to ensure everything works as expected. changelog: [internal] internal Reviewed By: cortinico, yungsters Differential Revision: D45662329 fbshipit-source-id: 1cab4331b2636b9a5d0d48085b68610153bcf0c5 --- .../facebook/react/bridgeless/ReactHost.java | 4 +-- .../react/bridgeless/ReactHostDelegate.kt | 28 ++++++++----------- .../react/bridgeless/ReactInstance.java | 6 ++-- .../react/bridgeless/hermes/HermesInstance.kt | 1 - .../react/bridgeless/ReactHostDelegateTest.kt | 6 ++-- .../react/bridgeless/ReactHostTest.java | 6 +--- 6 files changed, 21 insertions(+), 30 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHost.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHost.java index e822d6c7bd..83b8ab31a0 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHost.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHost.java @@ -945,7 +945,7 @@ public class ReactHost { // Since metro is running, fetch the JS bundle from the server return loadJSBundleFromMetro(); } - return Task.forResult(mReactHostDelegate.getJSBundleLoader(mContext)); + return Task.forResult(mReactHostDelegate.getJSBundleLoader()); }, mBGExecutor); } else { @@ -960,7 +960,7 @@ public class ReactHost { * throws an exception, the task will fault, and we'll go through the ReactHost error * reporting pipeline. */ - return Task.call(() -> mReactHostDelegate.getJSBundleLoader(mContext)); + return Task.call(() -> mReactHostDelegate.getJSBundleLoader()); } } 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 a138af7735..191f000e67 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 @@ -7,14 +7,13 @@ package com.facebook.react.bridgeless -import android.content.Context import com.facebook.infer.annotation.ThreadSafe import com.facebook.react.ReactPackage import com.facebook.react.bridge.JSBundleLoader import com.facebook.react.bridge.ReactApplicationContext +import com.facebook.react.bridge.ReactContext 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 /** TODO: add javadoc for class and methods */ @@ -27,36 +26,33 @@ interface ReactHostDelegate { val reactPackages: List - fun getJSBundleLoader(context: Context): JSBundleLoader + val jSEngineInstance: JSEngineInstance + + val jSBundleLoader: JSBundleLoader fun getTurboModuleManagerDelegate(context: ReactApplicationContext): TurboModuleManagerDelegate - fun getJSEngineInstance(context: ReactApplicationContext): JSEngineInstance - fun handleInstanceException(e: Exception) - // TODO: remove TurboModuleManager as a parameter - fun getReactNativeConfig(turboModuleManager: TurboModuleManager): ReactNativeConfig + fun getReactNativeConfig(context: ReactContext): ReactNativeConfig @UnstableReactNativeAPI class ReactHostDelegateBase( override val jSMainModulePath: String, + override val jSBundleLoader: JSBundleLoader, + override val jSEngineInstance: JSEngineInstance, + override val reactPackages: List = emptyList(), override val bindingsInstaller: BindingsInstaller? = null, - override val reactPackages: List, - private val jsBundleLoader: JSBundleLoader, - private val turboModuleManagerDelegate: TurboModuleManagerDelegate, - private val jsEngineInstance: JSEngineInstance, + private val turboModuleManagerDelegate: + (context: ReactApplicationContext) -> TurboModuleManagerDelegate, private val reactNativeConfig: ReactNativeConfig = ReactNativeConfig.DEFAULT_CONFIG, private val exceptionHandler: (Exception) -> Unit = {} ) : ReactHostDelegate { - override fun getJSBundleLoader(context: Context) = jsBundleLoader override fun getTurboModuleManagerDelegate(context: ReactApplicationContext) = - turboModuleManagerDelegate + turboModuleManagerDelegate(context) - override fun getJSEngineInstance(context: ReactApplicationContext) = jsEngineInstance - - override fun getReactNativeConfig(turboModuleManager: TurboModuleManager) = reactNativeConfig + override fun getReactNativeConfig(context: ReactContext) = reactNativeConfig override fun handleInstanceException(e: Exception) = exceptionHandler(e) } 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 1d8d270029..9552cee95a 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 @@ -150,7 +150,7 @@ final class ReactInstance { } }); - JSEngineInstance jsEngineInstance = mDelegate.getJSEngineInstance(mBridgelessReactContext); + JSEngineInstance jsEngineInstance = mDelegate.getJSEngineInstance(); BindingsInstaller bindingsInstaller = mDelegate.getBindingsInstaller(); // Notify JS if profiling is enabled boolean isProfiling = @@ -225,7 +225,7 @@ final class ReactInstance { mFabricUIManager = new FabricUIManager(mBridgelessReactContext, viewManagerRegistry, eventBeatManager); - ReactNativeConfig config = mDelegate.getReactNativeConfig(mTurboModuleManager); + ReactNativeConfig config = mDelegate.getReactNativeConfig(mBridgelessReactContext); // Misc initialization that needs to be done before Fabric init DisplayMetricsHolder.initDisplayMetricsIfNotInitialized(mBridgelessReactContext); @@ -383,7 +383,7 @@ final class ReactInstance { JavaTimerManager timerManager, JSTimerExecutor jsTimerExecutor, ReactJsExceptionHandler jReactExceptionsManager, - BindingsInstaller jBindingsInstaller, + @Nullable BindingsInstaller jBindingsInstaller, boolean isProfiling); @DoNotStrip diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/hermes/HermesInstance.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/hermes/HermesInstance.kt index 7db40bf228..2e0836a410 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/hermes/HermesInstance.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/hermes/HermesInstance.kt @@ -11,7 +11,6 @@ import com.facebook.jni.HybridData import com.facebook.jni.annotations.DoNotStrip import com.facebook.react.bridgeless.JSEngineInstance import com.facebook.soloader.SoLoader -import kotlin.jvm.JvmStatic class HermesInstance() : JSEngineInstance(initHybrid()!!) { 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 19aefb0df0..30ea505441 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 @@ -40,12 +40,12 @@ class ReactHostDelegateTest { val delegate = ReactHostDelegate.ReactHostDelegateBase( jsMainModulePathMocked, - jsBundleLoader = jsBundleLoader, + jSBundleLoader = jsBundleLoader, reactPackages = reactPackages, bindingsInstaller = bindingsInstallerMock, - jsEngineInstance = jsEngineInstanceMock, + jSEngineInstance = jsEngineInstanceMock, reactNativeConfig = reactNativeConfigMock, - turboModuleManagerDelegate = turboModuleManagerDelegateMock, + turboModuleManagerDelegate = { turboModuleManagerDelegateMock }, exceptionHandler = {}) assertThat(delegate.jSMainModulePath).isEqualTo(jsMainModulePathMocked) diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostTest.java b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostTest.java index 7ce2af4a1a..8c0218e07a 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostTest.java +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridgeless/ReactHostTest.java @@ -20,7 +20,6 @@ import android.app.Activity; import com.facebook.react.MemoryPressureRouter; import com.facebook.react.bridge.JSBundleLoader; import com.facebook.react.bridge.MemoryPressureListener; -import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.UIManager; import com.facebook.react.bridgeless.internal.bolts.TaskCompletionSource; import com.facebook.react.devsupport.interfaces.PackagerStatusCallback; @@ -32,7 +31,6 @@ import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentMatchers; import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.core.classloader.annotations.SuppressStaticInitializationFor; @@ -83,9 +81,7 @@ public class ReactHostTest { whenNew(MemoryPressureRouter.class).withAnyArguments().thenReturn(mMemoryPressureRouter); whenNew(BridgelessDevSupportManager.class).withAnyArguments().thenReturn(mDevSupportManager); - doReturn(mJSBundleLoader) - .when(mReactHostDelegate) - .getJSBundleLoader(ArgumentMatchers.any()); + doReturn(mJSBundleLoader).when(mReactHostDelegate).getJSBundleLoader(); mReactHost = new ReactHost(