Skip to content

Support customizeRootView from RCTRootViewFactory - #44775

Closed
Kudo wants to merge 1 commit into
react:mainfrom
Kudo:@kudo/RCTRootViewFactory-customizeRootView
Closed

Support customizeRootView from RCTRootViewFactory#44775
Kudo wants to merge 1 commit into
react:mainfrom
Kudo:@kudo/RCTRootViewFactory-customizeRootView

Conversation

@Kudo

@KudoKudo commented Jun 4, 2024

Copy link
Copy Markdown
Contributor

Summary:

The new customizeRootView does not have the feature parity as createRootViewWithBridge where reusing RCTRootViewFactory to create a root view, it does not call customizeRootView. This PR moves the customizeRootView support from RCTAppDelegate into RCTRootViewFactory and improves the customizeRootView support.

Changelog:

[IOS] [CHANGED] - Support customizeRootView from RCTRootViewFactory

Test Plan:

Add customizeRootView to packages/rn-tester/RNTester/AppDelegate.mm and test whether RNTester has blue background color in both new arch and old arch mode.

- (void)customizeRootView:(RCTRootView *)rootView
{
rootView.backgroundColor = [UIColor blueColor];
}

@facebook-github-botfacebook-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. Contributor A React Native contributor. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jun 4, 2024
@analysis-bot

analysis-bot commented Jun 4, 2024

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a19,675,021-14
androidhermesarmeabi-v7an/a--
androidhermesx86n/a--
androidhermesx86_64n/a--
androidjscarm64-v8a23,044,524-1
androidjscarmeabi-v7an/a--
androidjscx86n/a--
androidjscx86_64n/a--

Base commit: bb61e07
Branch: main

@cipolleschi

Copy link
Copy Markdown
Contributor

Thanks for the PR, but I don't think we need this, @Kudo. We moved the customizeRootView method after the rootView is created, so that it is invoked independently from whether method is used to create the view.

Perhaps I'm missing something, but why we need to bring this customization into the RootView Factory?

@Kudo

Kudo commented Jun 4, 2024

Copy link
Copy Markdown
ContributorAuthor

@cipolleschi thanks for feedback. this is to resolve the issue when enable new architecture with react-native-bootsplash and expo-updates. there is a workaround patch in react-conf-app but i think it would be good to have the feature in RCTRootViewFactory, so that developers reuse RCTRootViewFactory can fully support customizeRootView.

we could apply the patch inside expo-modules-core if you think RCTRootViewFactory is not the right place.

more details: expo-updates will delay the timing to initialize react instance and create an empty root view rather than the RCTRootView. we need to show splash screen twice. one before empty root view and the other after we use RCTRootViewFactory to create the formal RCTRootView. react-native-bootsplash uses customizeRootView to setup and show splash screen twice. that's why we need customizeRootView to be supported from RCTRootViewFactory (or our derived EXReactRootViewFactory)

@Kudo
Kudoforce-pushed the @kudo/RCTRootViewFactory-customizeRootView branch from 4d2aedb to 8b54ed7CompareJune 5, 2024 07:47

@cipolleschicipolleschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution and the explanation!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@cipolleschi merged this pull request in 8956869.

@github-actions

Copy link
Copy Markdown

This pull request was successfully merged by @Kudo in 8956869.

When will my fix make it into a release? | How to file a pick request?

@Kudo
Kudo deleted the @kudo/RCTRootViewFactory-customizeRootView branch June 6, 2024 11:19
kosmydel pushed a commit to kosmydel/react-native that referenced this pull request Jun 11, 2024
Summary:
The new `customizeRootView` does not have the feature parity as `createRootViewWithBridge` where reusing RCTRootViewFactory to create a root view, it does not call `customizeRootView`. This PR moves the `customizeRootView` support from RCTAppDelegate into RCTRootViewFactory and improves the customizeRootView support.
## Changelog:
[IOS] [CHANGED] - Support `customizeRootView` from `RCTRootViewFactory`
Pull Request resolved: react#44775
Test Plan:
Add customizeRootView to **packages/rn-tester/RNTester/AppDelegate.mm** and test whether RNTester has blue background color in both new arch and old arch mode.
```objc
- (void)customizeRootView:(RCTRootView *)rootView
{
rootView.backgroundColor = [UIColor blueColor];
}
```
Reviewed By: dmytrorykun
Differential Revision: D58179693
Pulled By: cipolleschi
fbshipit-source-id: 0fac9a1bd5b2583a2700b3a3d2c80d0f608c4481
Titozzz pushed a commit that referenced this pull request Jun 18, 2024
Summary:
The new `customizeRootView` does not have the feature parity as `createRootViewWithBridge` where reusing RCTRootViewFactory to create a root view, it does not call `customizeRootView`. This PR moves the `customizeRootView` support from RCTAppDelegate into RCTRootViewFactory and improves the customizeRootView support.
[IOS] [CHANGED] - Support `customizeRootView` from `RCTRootViewFactory`
Pull Request resolved: #44775
Test Plan:
Add customizeRootView to **packages/rn-tester/RNTester/AppDelegate.mm** and test whether RNTester has blue background color in both new arch and old arch mode.
```objc
- (void)customizeRootView:(RCTRootView *)rootView
{
rootView.backgroundColor = [UIColor blueColor];
}
```
Reviewed By: dmytrorykun
Differential Revision: D58179693
Pulled By: cipolleschi
fbshipit-source-id: 0fac9a1bd5b2583a2700b3a3d2c80d0f608c4481
Titozzz pushed a commit that referenced this pull request Jun 18, 2024
Summary:
The new `customizeRootView` does not have the feature parity as `createRootViewWithBridge` where reusing RCTRootViewFactory to create a root view, it does not call `customizeRootView`. This PR moves the `customizeRootView` support from RCTAppDelegate into RCTRootViewFactory and improves the customizeRootView support.
[IOS] [CHANGED] - Support `customizeRootView` from `RCTRootViewFactory`
Pull Request resolved: #44775
Test Plan:
Add customizeRootView to **packages/rn-tester/RNTester/AppDelegate.mm** and test whether RNTester has blue background color in both new arch and old arch mode.
```objc
- (void)customizeRootView:(RCTRootView *)rootView
{
rootView.backgroundColor = [UIColor blueColor];
}
```
Reviewed By: dmytrorykun
Differential Revision: D58179693
Pulled By: cipolleschi
fbshipit-source-id: 0fac9a1bd5b2583a2700b3a3d2c80d0f608c4481
@zoontek

Copy link
Copy Markdown
Contributor

@cipolleschi@Kudo Does a 0.74.x version with this fix is planned?

@Kudo

Kudo commented Jun 28, 2024

Copy link
Copy Markdown
ContributorAuthor

yes, it was already picked reactwg/react-native-releases#312, would be available from next version

@cipolleschicipolleschi mentioned this pull request Jun 28, 2024
Kudo added a commit to expo/expo that referenced this pull request Jul 6, 2024
# Why
react-native 0.74.3 added `customizeRootView` support from
react/react-native#44775. we should clone the
code from `RCTAppDelegate.createRCTRootViewFactory` to our
`EXAppDelegateWrapper.createRCTRootViewFactory`
that could possibly fix react-native-bootsplash integration for
zoontek/react-native-bootsplash#590
# How
- reference
https://github.com/facebook/react-native/blob/8c8c77b974ce3d4c3036361e56276a6ff2b02208/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm#L238-L262
to copy to `EXAppDelegateWrapper.createRCTRootViewFactory`
- since we are still on 0.74.2, we should use `respondsToSelector` to
check the existing property from RCTRootViewFactoryConfiguration.
Kudo added a commit to expo/expo that referenced this pull request Jul 6, 2024
react-native 0.74.3 added `customizeRootView` support from
react/react-native#44775. we should clone the
code from `RCTAppDelegate.createRCTRootViewFactory` to our
`EXAppDelegateWrapper.createRCTRootViewFactory`
that could possibly fix react-native-bootsplash integration for
zoontek/react-native-bootsplash#590
- reference
https://github.com/facebook/react-native/blob/8c8c77b974ce3d4c3036361e56276a6ff2b02208/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm#L238-L262
to copy to `EXAppDelegateWrapper.createRCTRootViewFactory`
- since we are still on 0.74.2, we should use `respondsToSelector` to
check the existing property from RCTRootViewFactoryConfiguration.
(cherry picked from commit 7c7499a)
@cipolleschicipolleschi mentioned this pull request Jul 22, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.ContributorA React Native contributor.MergedThis PR has been merged.p: ExpoPartner: ExpoPartnerShared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Kudo@analysis-bot@cipolleschi@facebook-github-bot@zoontek