Skip to content

Commit

Permalink
Hardcode concurrentRootEnabled to true when Fabric is enabled (#36107)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #36107

Having concurrentRoot disabled when Fabric is enabled is not recommended.
This simplifies the setup and makes sure that both are either enabled or disabled.

Changelog:
[Android] [Breaking] - Hardcode concurrentRootEnabled to true when Fabric is enabled

Reviewed By: cipolleschi

Differential Revision: D43127625

fbshipit-source-id: 88e5e800b55d5df228fb072bedf8533b0ab6c20d
  • Loading branch information
cortinico authored and facebook-github-bot committed Feb 10, 2023
1 parent 681b35d commit 81e7f2f
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public class ReactActivityDelegate {
private @Nullable PermissionListener mPermissionListener;
private @Nullable Callback mPermissionsCallback;
private ReactDelegate mReactDelegate;
private boolean mConcurrentRootEnabled;

@Deprecated
public ReactActivityDelegate(Activity activity, @Nullable String mainComponentName) {
Expand All @@ -59,7 +58,7 @@ public ReactActivityDelegate(ReactActivity activity, @Nullable String mainCompon

private @NonNull Bundle composeLaunchOptions() {
Bundle composedLaunchOptions = getLaunchOptions();
if (isConcurrentRootEnabled()) {
if (isFabricEnabled()) {
if (composedLaunchOptions == null) {
composedLaunchOptions = new Bundle();
}
Expand Down Expand Up @@ -212,14 +211,12 @@ protected Activity getPlainActivity() {
}

/**
* Override this method to enable Concurrent Root on the surface for this Activity. See:
* https://reactjs.org/blog/2022/03/29/react-v18.html
* Override this method if you wish to selectively toggle Fabric for a specific surface. This will
* also control if Concurrent Root (React 18) should be enabled or not.
*
* <p>This requires to be rendering on Fabric (i.e. on the New Architecture).
*
* @return Wether you want to enable Concurrent Root for this surface or not.
* @return true if Fabric is enabled for this Activity, false otherwise.
*/
protected boolean isConcurrentRootEnabled() {
protected boolean isFabricEnabled() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,18 @@ import com.facebook.react.ReactRootView
* A utility class that allows you to simplify the setup of a [ReactActivityDelegate] for new apps
* in Open Source.
*
* Specifically, with this class you can simply control if Fabric and Concurrent Root are enabled
* for an Activity using the boolean flags in the constructor.
* Specifically, with this class you can simply control if Fabric is enabled for an Activity using
* the boolean flag in the constructor.
*
* @param fabricEnabled Whether Fabric should be enabled for the RootView of this Activity.
* @param concurrentRootEnabled Whether ConcurrentRoot (aka React 18) should be enabled for the
* RootView of this Activity.
*/
open class DefaultReactActivityDelegate(
activity: ReactActivity,
mainComponentName: String,
private val fabricEnabled: Boolean = false,
private val concurrentRootEnabled: Boolean = false
) : ReactActivityDelegate(activity, mainComponentName) {

/**
* Override this method to enable Concurrent Root on the surface for this Activity. See:
* https://reactjs.org/blog/2022/03/29/react-v18.html
*
* This requires to be rendering on Fabric (i.e. on the New Architecture).
*
* @return Whether you want to enable Concurrent Root for this surface or not.
*/
override fun isConcurrentRootEnabled(): Boolean {
return concurrentRootEnabled
}
override fun isFabricEnabled(): Boolean = fabricEnabled

override fun createRootView(): ReactRootView =
ReactRootView(context).apply { setIsFabric(fabricEnabled) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import org.robolectric.RobolectricTestRunner
class ReactActivityDelegateTest {

@Test
fun delegateWithConcurrentRoot_populatesInitialPropsCorrectly() {
fun delegateWithFabricEnabled_populatesInitialPropsCorrectly() {
val delegate =
object : ReactActivityDelegate(null, "test-delegate") {
override fun isConcurrentRootEnabled() = true
override fun isFabricEnabled() = true
public val inspectLaunchOptions: Bundle?
get() = getLaunchOptions()
}
Expand All @@ -31,10 +31,10 @@ class ReactActivityDelegateTest {
}

@Test
fun delegateWithoutConcurrentRoot_hasNullInitialProperties() {
fun delegateWithoutFabricEnabled_hasNullInitialProperties() {
val delegate =
object : ReactActivityDelegate(null, "test-delegate") {
override fun isConcurrentRootEnabled() = false
override fun isFabricEnabled() = false
public val inspectLaunchOptions: Bundle?
get() = getLaunchOptions()
}
Expand All @@ -43,10 +43,10 @@ class ReactActivityDelegateTest {
}

@Test
fun delegateWithConcurrentRoot_composesInitialPropertiesCorrectly() {
fun delegateWithFabricEnabled_composesInitialPropertiesCorrectly() {
val delegate =
object : ReactActivityDelegate(null, "test-delegate") {
override fun isConcurrentRootEnabled() = true
override fun isFabricEnabled() = true
override fun getLaunchOptions(): Bundle =
Bundle().apply { putString("test-property", "test-value") }
public val inspectLaunchOptions: Bundle?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ public static class RNTesterActivityDelegate extends DefaultReactActivityDelegat
private final @Nullable ReactActivity mActivity;

public RNTesterActivityDelegate(ReactActivity activity, String mainComponentName) {
super(
activity,
mainComponentName,
DefaultNewArchitectureEntryPoint.getFabricEnabled(), // fabricEnabled
DefaultNewArchitectureEntryPoint.getConcurrentReactEnabled() // concurrentRootEnabled
);
super(activity, mainComponentName, DefaultNewArchitectureEntryPoint.getFabricEnabled());
this.mActivity = activity;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ protected ReactActivityDelegate createReactActivityDelegate() {
this,
getMainComponentName(),
// If you opted-in for the New Architecture, we enable the Fabric Renderer.
DefaultNewArchitectureEntryPoint.getFabricEnabled(), // fabricEnabled
// If you opted-in for the New Architecture, we enable Concurrent React (i.e. React 18).
DefaultNewArchitectureEntryPoint.getConcurrentReactEnabled() // concurrentRootEnabled
);
DefaultNewArchitectureEntryPoint.getFabricEnabled());
}
}

0 comments on commit 81e7f2f

Please sign in to comment.