From 3283202248a36dbda553745afc46a3e3e2ab41a6 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Mon, 11 Mar 2024 09:40:33 -0700 Subject: [PATCH] Remove createRootView(initialProps) from ReactDelegate (#43365) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/43365 Changelog: [Android][Changed] Simplified ReactActivityDelegate to expose a single createRootView method. Reviewed By: cortinico Differential Revision: D54434648 fbshipit-source-id: 305c54910a1eac25664507972879c7c6bd633466 --- .../ReactAndroid/api/ReactAndroid.api | 3 --- .../facebook/react/ReactActivityDelegate.java | 22 +++++++------------ .../com/facebook/react/ReactDelegate.java | 13 ++--------- .../com/facebook/react/ReactRootView.java | 2 +- .../defaults/DefaultReactActivityDelegate.kt | 8 ------- .../react/ReactActivityDelegateTest.kt | 10 +++------ 6 files changed, 14 insertions(+), 44 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 24fe604b68b..462410ae4ff 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -108,7 +108,6 @@ public class com/facebook/react/ReactActivityDelegate { public fun (Lcom/facebook/react/ReactActivity;Ljava/lang/String;)V protected fun composeLaunchOptions ()Landroid/os/Bundle; protected fun createRootView ()Lcom/facebook/react/ReactRootView; - protected fun createRootView (Landroid/os/Bundle;)Lcom/facebook/react/ReactRootView; protected fun getContext ()Landroid/content/Context; protected fun getLaunchOptions ()Landroid/os/Bundle; public fun getMainComponentName ()Ljava/lang/String; @@ -1954,8 +1953,6 @@ public class com/facebook/react/defaults/DefaultReactActivityDelegate : com/face public fun (Lcom/facebook/react/ReactActivity;Ljava/lang/String;Z)V public synthetic fun (Lcom/facebook/react/ReactActivity;Ljava/lang/String;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun (Lcom/facebook/react/ReactActivity;Ljava/lang/String;ZZ)V - protected fun createRootView ()Lcom/facebook/react/ReactRootView; - protected fun createRootView (Landroid/os/Bundle;)Lcom/facebook/react/ReactRootView; protected fun isFabricEnabled ()Z } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java index b305b390945..92b63c91607 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java @@ -31,8 +31,12 @@ public class ReactActivityDelegate { private @Nullable PermissionListener mPermissionListener; private @Nullable Callback mPermissionsCallback; - private ReactDelegate mReactDelegate; + private @Nullable ReactDelegate mReactDelegate; + /** + * Prefer using ReactActivity when possible, as it hooks up all Activity lifecycle methods by + * default. It also implements DefaultHardwareBackBtnHandler, which ReactDelegate requires. + */ @Deprecated public ReactActivityDelegate(@Nullable Activity activity, @Nullable String mainComponentName) { mActivity = activity; @@ -56,21 +60,11 @@ public class ReactActivityDelegate { } protected @Nullable Bundle composeLaunchOptions() { - Bundle composedLaunchOptions = getLaunchOptions(); - if (isFabricEnabled()) { - if (composedLaunchOptions == null) { - composedLaunchOptions = new Bundle(); - } - } - return composedLaunchOptions; + return getLaunchOptions(); } protected ReactRootView createRootView() { - return new ReactRootView(getContext()); - } - - protected ReactRootView createRootView(Bundle initialProps) { - return new ReactRootView(getContext()); + return Assertions.assertNotNull(mReactDelegate).createRootView(); } /** @@ -108,7 +102,7 @@ public class ReactActivityDelegate { getPlainActivity(), getReactNativeHost(), mainComponentName, launchOptions) { @Override protected ReactRootView createRootView() { - return ReactActivityDelegate.this.createRootView(launchOptions); + return ReactActivityDelegate.this.createRootView(); } }; } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactDelegate.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactDelegate.java index 1f89ceee0f6..7b83e71d5d8 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactDelegate.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactDelegate.java @@ -11,7 +11,6 @@ import android.app.Activity; import android.content.Intent; import android.os.Bundle; import android.view.KeyEvent; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.facebook.infer.annotation.Assertions; import com.facebook.react.config.ReactFeatureFlags; @@ -75,7 +74,7 @@ public class ReactDelegate { mFabricEnabled = fabricEnabled; mActivity = activity; mMainComponentName = appKey; - mLaunchOptions = composeLaunchOptions(launchOptions); + mLaunchOptions = launchOptions; mDoubleTapReloadRecognizer = new DoubleTapReloadRecognizer(); mReactNativeHost = reactNativeHost; } @@ -181,6 +180,7 @@ public class ReactDelegate { } } + // Not used in bridgeless protected ReactRootView createRootView() { ReactRootView reactRootView = new ReactRootView(mActivity); reactRootView.setIsFabric(isFabricEnabled()); @@ -233,13 +233,4 @@ public class ReactDelegate { protected boolean isFabricEnabled() { return mFabricEnabled; } - - private @NonNull Bundle composeLaunchOptions(Bundle composedLaunchOptions) { - if (isFabricEnabled()) { - if (composedLaunchOptions == null) { - composedLaunchOptions = new Bundle(); - } - } - return composedLaunchOptions; - } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java index 766836fd0b1..a01f7213e16 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java @@ -452,7 +452,7 @@ public class ReactRootView extends FrameLayout implements RootView, ReactRoot { /** * Schedule rendering of the react component rendered by the JS application from the given JS * module (@{param moduleName}) using provided {@param reactInstanceManager} to attach to the JS - * context of that manager. Extra parameter {@param launchOptions} can be used to pass initial + * context of that manager. Extra parameter {@param initialProperties} can be used to pass initial * properties for the react component. */ @ThreadConfined(UI) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactActivityDelegate.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactActivityDelegate.kt index 07f7ce270b2..4cb4263c41c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactActivityDelegate.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactActivityDelegate.kt @@ -7,10 +7,8 @@ package com.facebook.react.defaults -import android.os.Bundle import com.facebook.react.ReactActivity import com.facebook.react.ReactActivityDelegate -import com.facebook.react.ReactRootView /** * A utility class that allows you to simplify the setup of a [ReactActivityDelegate] for new apps @@ -43,10 +41,4 @@ public open class DefaultReactActivityDelegate( ) : this(activity, mainComponentName, fabricEnabled) override fun isFabricEnabled(): Boolean = fabricEnabled - - override fun createRootView(): ReactRootView = - ReactRootView(context).apply { setIsFabric(fabricEnabled) } - - override fun createRootView(bundle: Bundle?): ReactRootView = - ReactRootView(context).apply { setIsFabric(fabricEnabled) } } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/ReactActivityDelegateTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/ReactActivityDelegateTest.kt index f0eb097b697..44d33df9c45 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/ReactActivityDelegateTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/ReactActivityDelegateTest.kt @@ -28,9 +28,7 @@ class ReactActivityDelegateTest { get() = composeLaunchOptions() } - assertNotNull(delegate.inspectLaunchOptions) - // False because oncurrentRoot is hardcoded to true for Fabric inside renderApplication - assertFalse(delegate.inspectLaunchOptions!!.containsKey("concurrentRoot")) + assertNull(delegate.inspectLaunchOptions) } @Test @@ -60,9 +58,7 @@ class ReactActivityDelegateTest { } assertNotNull(delegate.inspectLaunchOptions) - // False because oncurrentRoot is hardcoded to true for Fabric inside renderApplication - assertFalse(delegate.inspectLaunchOptions!!.containsKey("concurrentRoot")) - assertTrue(delegate.inspectLaunchOptions!!.containsKey("test-property")) - assertEquals("test-value", delegate.inspectLaunchOptions!!.getString("test-property")) + assertTrue(delegate.inspectLaunchOptions?.containsKey("test-property") ?: false) + assertEquals("test-value", delegate.inspectLaunchOptions?.getString("test-property")) } }