Add function to customise RootView in Bridgeless #42088

cipolleschi · 2023-12-28T10:00:12Z

Summary:
This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode.
To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one.
The Warning is shown ONLY when the user do customise the rootView. For users which were not customising the Root View, the warning will not appear.

The documentation of the new method plus the warning should guide the users toward the right migration path.

Changelog

[iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate.

Differential Revision: D52442598

TestPlan

Tested locally on RNTester:

Bridge/Bridgeless	Light Theme	Dark Theme
Bridge Mode
Bridgeless

facebook-github-bot · 2023-12-28T10:00:30Z

This pull request was exported from Phabricator. Differential Revision: D52442598

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Differential Revision: D52442598

facebook-github-bot · 2023-12-28T10:06:14Z

This pull request was exported from Phabricator. Differential Revision: D52442598

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Differential Revision: D52442598

facebook-github-bot · 2023-12-28T10:07:22Z

This pull request was exported from Phabricator. Differential Revision: D52442598

analysis-bot · 2023-12-28T10:35:27Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	16,577,954	+0
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	19,955,039	-3
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: af8c56a
Branch: main

okwasniewski

Looks good, left few minor comments

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm

okwasniewski · 2023-12-28T10:42:58Z

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm

@@ -117,6 +118,7 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
    }
    rootView = [self createRootViewWithBridge:self.bridge moduleName:self.moduleName initProps:initProps];
  }
+  [self customizeRootView:(RCTRootView *)rootView];


nit: Can't we set the rootView variable type to RCTRootView? And cast it to UIView if needed

Nope, this won't change a lot.

The variable is defined as UIView *rootView because it is the common ancestor.

The surfaceHostingProxyRootView; is actually a @interface RCTSurfaceHostingProxyRootView : RCTSurfaceHostingView where RCTSurfaceHostingView is a @interface RCTSurfaceHostingView : UIView <RCTSurfaceDelegate>.
So, technically, it is not even a RCTRootView, but a proxied one, and it will still require casting.

The createRootViewWithBridge:moduleName:initProps: technically returns a UIView *, so changing if we made the rootView a RCTRootView * we would have either to change the createRootViewWithBridge (breaking change, we don't want to do it) or add the casting to RCTRootView.

So, in the end, we will always have to cast. :(

Okay, thanks for the explanation

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Differential Revision: D52442598

facebook-github-bot · 2023-12-28T13:24:53Z

This pull request was exported from Phabricator. Differential Revision: D52442598

facebook-github-bot · 2023-12-28T13:24:58Z

This pull request was exported from Phabricator. Differential Revision: D52442598

cipolleschi · 2023-12-28T10:04:03Z

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm

@@ -156,6 +164,20 @@ - (UIView *)createRootViewWithBridge:(RCTBridge *)bridge
  return rootView;
 }

+- (void)_logWarnIfOverridden


We will remove the log after 0.74

cipolleschi · 2023-12-28T13:19:28Z

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm

@@ -117,6 +118,7 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
    }
    rootView = [self createRootViewWithBridge:self.bridge moduleName:self.moduleName initProps:initProps];
  }
+  [self customizeRootView:(RCTRootView *)rootView];


Nope, this won't change a lot.

The variable is defined as UIView *rootView because it is the common ancestor.

The surfaceHostingProxyRootView; is actually a @interface RCTSurfaceHostingProxyRootView : RCTSurfaceHostingView where RCTSurfaceHostingView is a @interface RCTSurfaceHostingView : UIView <RCTSurfaceDelegate>.
So, technically, it is not even a RCTRootView, but a proxied one, and it will still require casting.

The createRootViewWithBridge:moduleName:initProps: technically returns a UIView *, so changing if we made the rootView a RCTRootView * we would have either to change the createRootViewWithBridge (breaking change, we don't want to do it) or add the casting to RCTRootView.

So, in the end, we will always have to cast. :(

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm

facebook-github-bot · 2023-12-28T15:37:43Z

This pull request was exported from Phabricator. Differential Revision: D52442598

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Differential Revision: D52442598

Saadnajmi

If I understand correctly, the issue with createRootViewWithBridge was that:

It only worked on the old architecture
It doesn't work with bridgeless

and this new method is just a new uniform way to do that? How does it differ from RCTAppSetupDefaultRootView?

cipolleschi · 2023-12-28T20:36:00Z

@Saadnajmi

If I understand correctly, the issue with createRootViewWithBridge was that:

It only worked on the old architecture

It doesn't work with bridgeless
and this new method is just a new uniform way to do that?

Correct. Before this change the only way to customise the RCTRootView in bridgeless/New Architecture in the future is to override (void)setRootView:(UIView *)rootView toRootViewController:(UIViewController *)rootViewController; and customise the rootView there.
But it is an abuse, as the purpose of that method is different. That method needs to be override when the rootViewController is a special one, with custom APIs to set the rootView, like the UISplitViewController.

How does it differ from RCTAppSetupDefaultRootView?

There are several differences:

Purpose: RCTAppSetupDefaultRootView is used to initialize the rootView. It will be an abuse to use it to customise the rootView.
Stability: RCTAppSetupDefaultRootView takes the RCTBridge as one of its parameters. This means that we are going to break it sooner or later. And we will have to force a migration to our users.
Visibility: RCTAppSetupDefaultRootView is part of the RCTAppSetupUtils.h header, which should be a private header. At some point, we will hide it from the final users, to allow us to change the internals of React Native without breaking everyone. Furthermore, it is a plain C function, it is not usable from the AppDelegate and there is no easy way to override it.

With the purpose of being future proof and to design clean, simple API with a well-defined purpose, it is better to have a method that does only one thing, and with a clear name which is unlikely that we will break.

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Differential Revision: D52442598

facebook-github-bot · 2023-12-28T21:14:34Z

This pull request was exported from Phabricator. Differential Revision: D52442598

facebook-github-bot · 2024-01-02T08:42:45Z

This pull request was exported from Phabricator. Differential Revision: D52442598

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Reviewed By: cortinico Differential Revision: D52442598

facebook-github-bot · 2024-01-02T09:25:31Z

This pull request was exported from Phabricator. Differential Revision: D52442598

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Reviewed By: cortinico Differential Revision: D52442598

facebook-github-bot · 2024-01-02T09:26:17Z

This pull request was exported from Phabricator. Differential Revision: D52442598

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Reviewed By: cortinico Differential Revision: D52442598

facebook-github-bot · 2024-01-02T09:53:26Z

This pull request was exported from Phabricator. Differential Revision: D52442598

Summary: This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Reviewed By: cortinico Differential Revision: D52442598

facebook-github-bot · 2024-01-02T10:13:45Z

This pull request was exported from Phabricator. Differential Revision: D52442598

facebook-github-bot · 2024-01-02T12:58:23Z

This pull request has been merged in db9c9ea.

Summary: Pull Request resolved: facebook#42088 This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode. To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one. *The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear. The documentation of the new method plus the warning should guide the users toward the right migration path. ## Changelog [iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate. Reviewed By: cortinico Differential Revision: D52442598 fbshipit-source-id: 8b99b67f4741ee61989a8659a3d74c1eba27bc5b

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Dec 28, 2023

cipolleschi force-pushed the export-D52442598 branch from 341625b to aa8c3d3 Compare December 28, 2023 10:06

cipolleschi force-pushed the export-D52442598 branch from aa8c3d3 to 8490fbd Compare December 28, 2023 10:07

okwasniewski reviewed Dec 28, 2023

View reviewed changes

cipolleschi force-pushed the export-D52442598 branch from 8490fbd to 509ceb0 Compare December 28, 2023 13:24

cipolleschi force-pushed the export-D52442598 branch from 509ceb0 to 5e9a3bb Compare December 28, 2023 13:24

cipolleschi commented Dec 28, 2023

View reviewed changes

cipolleschi force-pushed the export-D52442598 branch from 5e9a3bb to 9a3f95f Compare December 28, 2023 15:37

Saadnajmi reviewed Dec 28, 2023

View reviewed changes

cipolleschi force-pushed the export-D52442598 branch from 9a3f95f to 28e4c7e Compare December 28, 2023 21:14

cipolleschi force-pushed the export-D52442598 branch from 28e4c7e to edb9e9b Compare January 2, 2024 08:42

cipolleschi force-pushed the export-D52442598 branch from edb9e9b to eaa156a Compare January 2, 2024 09:25

cipolleschi force-pushed the export-D52442598 branch from eaa156a to c0420d6 Compare January 2, 2024 09:25

cipolleschi force-pushed the export-D52442598 branch from c0420d6 to 5a07a6c Compare January 2, 2024 09:53

cipolleschi force-pushed the export-D52442598 branch from 5a07a6c to f4471f1 Compare January 2, 2024 10:13

facebook-github-bot closed this in db9c9ea Jan 2, 2024

facebook-github-bot added the Merged This PR has been merged. label Jan 2, 2024

mlazari mentioned this pull request Apr 23, 2024

React Native 0.73 Onboarding Flash Loading & Blank Screen #44195

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add function to customise RootView in Bridgeless #42088

Add function to customise RootView in Bridgeless #42088

cipolleschi commented Dec 28, 2023 •

edited

Loading

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

analysis-bot commented Dec 28, 2023 •

edited

Loading

okwasniewski left a comment

okwasniewski Dec 28, 2023

cipolleschi Dec 28, 2023

okwasniewski Jan 2, 2024

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

cipolleschi Dec 28, 2023

cipolleschi Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

Saadnajmi left a comment

cipolleschi commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

Add function to customise RootView in Bridgeless #42088

Add function to customise RootView in Bridgeless #42088

Conversation

cipolleschi commented Dec 28, 2023 • edited Loading

Changelog

TestPlan

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

analysis-bot commented Dec 28, 2023 • edited Loading

okwasniewski left a comment

Choose a reason for hiding this comment

okwasniewski Dec 28, 2023

Choose a reason for hiding this comment

cipolleschi Dec 28, 2023

Choose a reason for hiding this comment

okwasniewski Jan 2, 2024

Choose a reason for hiding this comment

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

cipolleschi Dec 28, 2023

Choose a reason for hiding this comment

cipolleschi Dec 28, 2023

Choose a reason for hiding this comment

facebook-github-bot commented Dec 28, 2023

Saadnajmi left a comment

Choose a reason for hiding this comment

cipolleschi commented Dec 28, 2023

facebook-github-bot commented Dec 28, 2023

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

facebook-github-bot commented Jan 2, 2024

cipolleschi commented Dec 28, 2023 •

edited

Loading

analysis-bot commented Dec 28, 2023 •

edited

Loading