feat: implement ReactNativeFactory #46298

okwasniewski · 2024-09-02T10:52:52Z

Summary:

This PR implements ReactNativeFactory to encapsulate further the logic of creating an instance of React Native for iOS.

This will remove the strong coupling on the RCTAppDelegate and allow us to support Scene Delegate in the future.

The goal is to have a following API:

self.reactNativeFactory = [[RCTReactNativeFactory alloc] initWithDelegate:self];

UIView *rootView = [self.reactNativeFactory.rootViewFactory viewWithModuleName:self.moduleName
                                                               initialProperties:self.initialProps
                                                                   launchOptions:launchOptions];
                                                                   
// Standard iOS stuff here

Changelog:

[IOS] [ADDED] - implement ReactNativeFactory

Test Plan:

Test out all the methods of AppDelegate

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

cipolleschi

The direction looks the right one to me. I left a couple of comments with internal considerations, but it looks promising.

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

packages/rn-tester/RNTester/AppDelegate.mm

Summary: This PR introduces `RCTUIConfiguratorProtocol` for better separation of concerns inside of RCTAppDelegate. It's also a prerequisite for #46298 Discussed with cipolleschi ## Changelog: [IOS] [ADDED] - introduce RCTUIConfiguratorProtocol Pull Request resolved: #47139 Test Plan: - CI Green - Test if methods can be overriden Reviewed By: blakef Differential Revision: D65063839 Pulled By: cipolleschi fbshipit-source-id: b63766e245d57f369ab94bd8047d5de9a3447b3e

Summary: This PR introduces `RCTArchConfiguratorProtocol` for better separation of concerns inside of RCTAppDelegate. It's also a prerequisite for #46298 Discussed with cipolleschi ## Changelog: [IOS] [ADDED] - introduce RCTArchConfiguratorProtocol Pull Request resolved: #47306 Test Plan: - CI Green - Test if methods can be overriden Reviewed By: realsoelynn Differential Revision: D65212703 Pulled By: cipolleschi fbshipit-source-id: 9850fec31c421f0c6230e7e23d7a208d823d828f

cipolleschi

Sorry for the long wait @okwasniewski.
I love the direction of this!

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

cipolleschi

Pressed on approve by mistake.
There are a couple of things that are missing.

okwasniewski · 2024-12-05T09:03:38Z

@cipolleschi Sorry I kind of left it in-progress, as I was experimenting with the new approach, of course everything you pointed out needs to be fixed! Thanks for checking it out and making sure the direction is good.

We are finishing a client project, so I will have some time to get back to this work soon

okwasniewski · 2024-12-18T10:24:56Z

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

-#import <ReactCommon/RCTTurboModuleManager.h>
-#import "RCTAppDelegate.h"
-
-@interface RCTAppDelegate () <RCTTurboModuleManagerDelegate>
-@end


@cipolleschi I'm not sure how to handle this. This should be defined on the ReactNativeFactoryDelegate now but removing this might be a breaking change

You added the conformance already here so, technically, the RCTAppDelegate conforms to the RCTTurboModuleManagerDelegate.

If the breaking change is for the missing file/import, we can land it and ship it in 0.78.

cipolleschi · 2024-12-18T13:31:33Z

uhm... dynamic frameworks are failing, could you have a look at why and fix it, please?

cipolleschi

Great job. I left a few other comments that probably needs to be addressed.

Can you also check whether the packages/helloworld/ios app requires any change? This mimics the template in our CI.

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

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

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

okwasniewski · 2024-12-18T14:08:49Z

uhm... dynamic frameworks are failing, could you have a look at why and fix it, please?

Yeah.. wrong import, should be good now

cipolleschi · 2024-12-18T15:43:59Z

And now we have a conflict with your other PR.. 🤣

wip: work on new implementation

okwasniewski · 2024-12-18T15:53:50Z

And now we have a conflict with your other PR.. 🤣

Rebased ✅ At least now we have a clear history why it was deleted 😄

cipolleschi

Thanks for the amazing work!

cipolleschi · 2024-12-19T14:13:26Z

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

-#import <ReactCommon/RCTTurboModuleManager.h>
-#import "RCTAppDelegate.h"
-
-@interface RCTAppDelegate () <RCTTurboModuleManagerDelegate>
-@end


You added the conformance already here so, technically, the RCTAppDelegate conforms to the RCTTurboModuleManagerDelegate.

If the breaking change is for the missing file/import, we can land it and ship it in 0.78.

facebook-github-bot · 2024-12-19T14:14:16Z

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

react-native-bot · 2024-12-31T15:48:53Z

This pull request was successfully merged by @okwasniewski in 081be01

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

facebook-github-bot · 2024-12-31T15:48:53Z

@cipolleschi merged this pull request in 081be01.

Summary: Recently, I've introduced `RCTReactNativeFactory` in this PR: #46298, which is a good successor for `RCTAppDelegate`. ### Why? `RCTAppDelegate` introduced strong coupling between React Native and AppDelegate pattern. From iOS 13+ there is a newer equivalent (Scene Delegate) which is not possible to achieve with current architecture. The proposed solution involves migration to a `RCTReactNativeFactory` a class that encapsulates initialization logic of React Native. This migration will make brownfield initialization easier by making it more flexible and simpler to integrate into already established apps. ### Deprecation plan The plan I've discussed with cipolleschi involves: - Deprecation of `RCTAppDelegate` in 0.79 (current main) - Migration off `RCTAppDelegate` to SceneDelegate + `RCTReactNativeFactory` in 0.80 ## Changelog: [IOS] [DEPRECATED] - deprecate RCTAppDelegate Pull Request resolved: #49078 Test Plan: Not needed Reviewed By: cortinico Differential Revision: D69061022 Pulled By: cipolleschi fbshipit-source-id: b02a0ff3f26be9320da749f38c9cf083804f9f30

Summary: Recently, I've introduced `RCTReactNativeFactory` in this PR: facebook#46298, which is a good successor for `RCTAppDelegate`. ### Why? `RCTAppDelegate` introduced strong coupling between React Native and AppDelegate pattern. From iOS 13+ there is a newer equivalent (Scene Delegate) which is not possible to achieve with current architecture. The proposed solution involves migration to a `RCTReactNativeFactory` a class that encapsulates initialization logic of React Native. This migration will make brownfield initialization easier by making it more flexible and simpler to integrate into already established apps. ### Deprecation plan The plan I've discussed with cipolleschi involves: - Deprecation of `RCTAppDelegate` in 0.79 (current main) - Migration off `RCTAppDelegate` to SceneDelegate + `RCTReactNativeFactory` in 0.80 ## Changelog: [IOS] [DEPRECATED] - deprecate RCTAppDelegate Pull Request resolved: facebook#49078 Test Plan: Not needed Reviewed By: cortinico Differential Revision: D69061022 Pulled By: cipolleschi fbshipit-source-id: b02a0ff3f26be9320da749f38c9cf083804f9f30

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: Callstack Partner: Callstack Partner labels Sep 2, 2024

okwasniewski commented Sep 2, 2024

View reviewed changes

packages/react-native/Libraries/AppDelegate/RCTAppDelegate+Protected.h Outdated Show resolved Hide resolved

cipolleschi reviewed Sep 2, 2024

View reviewed changes

packages/react-native/Libraries/AppDelegate/RCTAppDelegate+Protected.h Outdated Show resolved Hide resolved

packages/rn-tester/RNTester/AppDelegate.mm Outdated Show resolved Hide resolved

okwasniewski force-pushed the feat/ReactNativeFactory branch from 7754580 to 969d347 Compare September 12, 2024 14:42

okwasniewski mentioned this pull request Oct 21, 2024

feat(iOS): introduce RCTUIConfiguratorProtocol #47139

Closed

okwasniewski mentioned this pull request Oct 30, 2024

feat: introduce RCTArchConfiguratorProtocol #47306

Closed

okwasniewski force-pushed the feat/ReactNativeFactory branch from 969d347 to 13d0fa4 Compare November 18, 2024 15:34

MaxAst mentioned this pull request Nov 21, 2024

update view controller to use react native's new architecture MaxAst/expo-share-extension#50

Merged

cipolleschi approved these changes Dec 4, 2024

View reviewed changes

cipolleschi requested changes Dec 4, 2024

View reviewed changes

okwasniewski force-pushed the feat/ReactNativeFactory branch 9 times, most recently from 7f64781 to e69d9dc Compare December 18, 2024 10:23

okwasniewski commented Dec 18, 2024

View reviewed changes

okwasniewski force-pushed the feat/ReactNativeFactory branch 2 times, most recently from 400d438 to 7f60df3 Compare December 18, 2024 10:27

okwasniewski marked this pull request as ready for review December 18, 2024 10:38

okwasniewski requested a review from cipolleschi December 18, 2024 10:40

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Dec 18, 2024

cipolleschi requested changes Dec 18, 2024

View reviewed changes

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h Show resolved Hide resolved

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm Outdated Show resolved Hide resolved

packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h Show resolved Hide resolved

okwasniewski force-pushed the feat/ReactNativeFactory branch from 7f60df3 to 06e3513 Compare December 18, 2024 14:08

okwasniewski force-pushed the feat/ReactNativeFactory branch from 06e3513 to 5bb5a94 Compare December 18, 2024 15:11

feat: implement ReactNativeFactory

4c456ee

wip: work on new implementation

okwasniewski force-pushed the feat/ReactNativeFactory branch from 5bb5a94 to 4c456ee Compare December 18, 2024 15:52

cipolleschi approved these changes Dec 19, 2024

View reviewed changes

facebook-github-bot closed this in 081be01 Dec 31, 2024

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

okwasniewski mentioned this pull request Jan 31, 2025

chore(iOS): deprecate RCTAppDelegate #49078

Closed

Bruno125 mentioned this pull request Mar 4, 2025

extraModulesForBridge not called with new architecture enabled in RN 0.78 #49819

Closed

kkafar mentioned this pull request Apr 16, 2025

Crash on + [ReactNativeSupport reloadApp] call with react-native@0.79 wix/Detox#4760

Closed

2 tasks

feat: implement ReactNativeFactory #46298

feat: implement ReactNativeFactory #46298

Uh oh!

Conversation

okwasniewski commented Sep 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Changelog:

Test Plan:

Uh oh!

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

okwasniewski commented Dec 5, 2024

Uh oh!

okwasniewski Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

cipolleschi Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

cipolleschi commented Dec 18, 2024

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

okwasniewski commented Dec 18, 2024

Uh oh!

cipolleschi commented Dec 18, 2024

Uh oh!

okwasniewski commented Dec 18, 2024

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

cipolleschi Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Dec 19, 2024

Uh oh!

react-native-bot commented Dec 31, 2024

Uh oh!

facebook-github-bot commented Dec 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

okwasniewski commented Sep 2, 2024 •

edited

Loading