Set concurrentRoot to true whenever Fabric is used in renderApplicati…

…on (#42821) Summary: Pull Request resolved: #42821 As the title says, we want `concurrentRoot iif fabric`. This makes sure we invoke renderApplication correctly. Changelog: [Internal] [Changed] - Set concurrentRoot to true whenever Fabric is used in renderApplication Reviewed By: sammy-SC Differential Revision: D53353017 fbshipit-source-id: 8de88adf528eb71f233233bd85c2c6ef9430fb16
facebook · Feb 7, 2024 · 30d186c · 30d186c
1 parent 665c400
commit 30d186c
Show file tree

Hide file tree

Showing 8 changed files with 10 additions and 26 deletions.
diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h
@@ -42,7 +42,6 @@ NS_ASSUME_NONNULL_BEGIN
  *   - (UIViewController *)createRootViewController;
  *   - (void)setRootView:(UIView *)rootView toRootViewController:(UIViewController *)rootViewController;
  * New Architecture:
- *   - (BOOL)concurrentRootEnabled
  *   - (BOOL)turboModuleEnabled;
  *   - (BOOL)fabricEnabled;
  *   - (NSDictionary *)prepareInitialProps

diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm
@@ -39,8 +39,6 @@
 #import <react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.h>
 #import <react/runtime/JSRuntimeFactory.h>
 
-static NSString *const kRNConcurrentRoot = @"concurrentRoot";
-
 @interface RCTAppDelegate () <
     RCTTurboModuleManagerDelegate,
     RCTComponentViewFactoryComponentProvider,
@@ -53,9 +51,6 @@ @interface RCTAppDelegate () <
 static NSDictionary *updateInitialProps(NSDictionary *initialProps, BOOL isFabricEnabled)
 {
   NSMutableDictionary *mutableProps = [initialProps mutableCopy] ?: [NSMutableDictionary new];
-  // Hardcoding the Concurrent Root as it it not recommended to
-  // have the concurrentRoot turned off when Fabric is enabled.
-  mutableProps[kRNConcurrentRoot] = @(isFabricEnabled);
   return mutableProps;
 }
 

diff --git a/packages/react-native/Libraries/ReactNative/renderApplication.js b/packages/react-native/Libraries/ReactNative/renderApplication.js
@@ -83,16 +83,13 @@ export default function renderApplication<Props: Object>(
     );
   }
 
-  if (fabric && !useConcurrentRoot) {
-    console.warn(
-      'Using Fabric without concurrent root is deprecated. Please enable concurrent root for this application.',
-    );
-  }
+  // We want to have concurrentRoot always enabled when you're on Fabric.
+  const useConcurrentRootOverride = fabric;
 
   performanceLogger.startTimespan('renderApplication_React_render');
   performanceLogger.setExtra(
     'usedReactConcurrentRoot',
-    useConcurrentRoot ? '1' : '0',
+    useConcurrentRootOverride ? '1' : '0',
   );
   performanceLogger.setExtra('usedReactFabric', fabric ? '1' : '0');
   performanceLogger.setExtra(
@@ -103,7 +100,7 @@ export default function renderApplication<Props: Object>(
     element: renderable,
     rootTag,
     useFabric: Boolean(fabric),
-    useConcurrentRoot: Boolean(useConcurrentRoot),
+    useConcurrentRoot: Boolean(useConcurrentRootOverride),
   });
   performanceLogger.stopTimespan('renderApplication_React_render');
 }
diff --git a/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java b/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java
@@ -61,7 +61,6 @@ public ReactActivityDelegate(
       if (composedLaunchOptions == null) {
         composedLaunchOptions = new Bundle();
       }
-      composedLaunchOptions.putBoolean("concurrentRoot", true);
     }
     return composedLaunchOptions;
   }

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
@@ -240,7 +240,6 @@ protected boolean isFabricEnabled() {
       if (composedLaunchOptions == null) {
         composedLaunchOptions = new Bundle();
       }
-      composedLaunchOptions.putBoolean("concurrentRoot", true);
     }
     return composedLaunchOptions;
   }

diff --git a/...ages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/...ages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java
@@ -340,9 +340,7 @@ public JavaScriptExecutorFactory getJavaScriptExecutorFactory() {
           ReactRootView rootView = new ReactRootView(currentActivity);
           boolean isFabric = ReactFeatureFlags.enableFabricRenderer;
           rootView.setIsFabric(isFabric);
-          Bundle launchOptions = new Bundle();
-          launchOptions.putBoolean("concurrentRoot", isFabric);
-          rootView.startReactApplication(ReactInstanceManager.this, appKey, launchOptions);
+          rootView.startReactApplication(ReactInstanceManager.this, appKey, new Bundle());
           return rootView;
         }
 

diff --git a/...ve/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessDevSupportManager.java b/...ve/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessDevSupportManager.java
@@ -135,11 +135,8 @@ public JavaScriptExecutorFactory getJavaScriptExecutorFactory() {
       public View createRootView(String appKey) {
         Activity currentActivity = getCurrentActivity();
         if (currentActivity != null && !reactHost.isSurfaceWithModuleNameAttached(appKey)) {
-          Bundle launchOptions = new Bundle();
-          launchOptions.putBoolean("concurrentRoot", true);
-
           ReactSurfaceImpl reactSurface =
-              ReactSurfaceImpl.createWithView(currentActivity, appKey, launchOptions);
+              ReactSurfaceImpl.createWithView(currentActivity, appKey, new Bundle());
           reactSurface.attach(reactHost);
           reactSurface.start();
 

diff --git a/...s/react-native/ReactAndroid/src/test/java/com/facebook/react/ReactActivityDelegateTest.kt b/...s/react-native/ReactAndroid/src/test/java/com/facebook/react/ReactActivityDelegateTest.kt
@@ -29,8 +29,8 @@ class ReactActivityDelegateTest {
         }
 
     assertNotNull(delegate.inspectLaunchOptions)
-    assertTrue(delegate.inspectLaunchOptions!!.containsKey("concurrentRoot"))
-    assertTrue(delegate.inspectLaunchOptions!!.getBoolean("concurrentRoot"))
+    // False because oncurrentRoot is hardcoded to true for Fabric inside renderApplication
+    assertFalse(delegate.inspectLaunchOptions!!.containsKey("concurrentRoot"))
   }
 
   @Test
@@ -60,8 +60,8 @@ class ReactActivityDelegateTest {
         }
 
     assertNotNull(delegate.inspectLaunchOptions)
-    assertTrue(delegate.inspectLaunchOptions!!.containsKey("concurrentRoot"))
-    assertTrue(delegate.inspectLaunchOptions!!.getBoolean("concurrentRoot"))
+    // 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"))
   }