From 8b8b85bb1f55a8090360fec84ec4f56f372ede6b Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Tue, 9 Apr 2024 08:54:33 -0700 Subject: [PATCH] Fix Connect to Metro after Reload in Bridgeless mode (#43994) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/43994 We received [this issue](https://github.com/facebook/react-native/issues/43764) from OSS where an app can't connect to Metro on reloads in the following scenario: * Start the App when metro does not run. * Observe the error screen * Start Metro * Press Reload * Observe the error message again While the desired behavior should be to connect to Metro now that this is running. The root cause of the problem is that the RCTHost is initialized with a value of the `bundleURL` that is `nil`. Upon reload, the RCTHost is **not** recreated: the instance is restarted, but with the previous `bundleURL`, which is still `nil`. The solution is to initialize the `RCTHost` with a closure that re-evaluate the `bundleURL` whenever it is invoked and to evaluate it only on `start`, to keep the initialization path light. This way, when the app is started with Metro not running, the `bundleURL` is `nil`. But when it is reloaded with Metro starting, the `bundleURL` is properly initialized. Note that the changes in this diff are not breaking as I reimplemented (and deprecated) the old initializer so that they should work in the same way. ## Changelog: [iOS][Fixed] - Let RCTHost be initialized with a function to provide the `bundleURL` so that it can connect to metro on Reload when the url changes. Reviewed By: dmytrorykun Differential Revision: D55916135 fbshipit-source-id: 6927b2154870245f28f42d26bd0209b28c9518f2 --- .../Libraries/AppDelegate/RCTAppDelegate.mm | 17 ++++++---- .../AppDelegate/RCTRootViewFactory.h | 10 ++++-- .../AppDelegate/RCTRootViewFactory.mm | 32 +++++++++++++------ .../ios/ReactCommon/RCTHost+Internal.h | 2 -- .../platform/ios/ReactCommon/RCTHost.h | 10 +++++- .../platform/ios/ReactCommon/RCTHost.mm | 29 +++++++++++++---- 6 files changed, 74 insertions(+), 26 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm index 2c56734cbf4..0ced5888f91 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm @@ -248,13 +248,18 @@ - (RCTRootViewFactory *)createRCTRootViewFactory { - RCTRootViewFactoryConfiguration *configuration = - [[RCTRootViewFactoryConfiguration alloc] initWithBundleURL:self.bundleURL - newArchEnabled:self.fabricEnabled - turboModuleEnabled:self.turboModuleEnabled - bridgelessEnabled:self.bridgelessEnabled]; - __weak __typeof(self) weakSelf = self; + RCTBundleURLBlock bundleUrlBlock = ^{ + RCTAppDelegate *strongSelf = weakSelf; + return strongSelf.bundleURL; + }; + + RCTRootViewFactoryConfiguration *configuration = + [[RCTRootViewFactoryConfiguration alloc] initWithBundleURLBlock:bundleUrlBlock + newArchEnabled:self.fabricEnabled + turboModuleEnabled:self.turboModuleEnabled + bridgelessEnabled:self.bridgelessEnabled]; + configuration.createRootViewWithBridge = ^UIView *(RCTBridge *bridge, NSString *moduleName, NSDictionary *initProps) { return [weakSelf createRootViewWithBridge:bridge moduleName:moduleName initProps:initProps]; diff --git a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.h b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.h index 21533feaad9..1926dd81747 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.h +++ b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.h @@ -24,6 +24,7 @@ typedef UIView *_Nonnull ( typedef RCTBridge *_Nonnull ( ^RCTCreateBridgeWithDelegateBlock)(id delegate, NSDictionary *launchOptions); typedef NSURL *_Nullable (^RCTSourceURLForBridgeBlock)(RCTBridge *bridge); +typedef NSURL *_Nullable (^RCTBundleURLBlock)(void); typedef NSArray> *_Nonnull (^RCTExtraModulesForBridgeBlock)(RCTBridge *bridge); typedef NSDictionary *_Nonnull (^RCTExtraLazyModuleClassesForBridge)(RCTBridge *bridge); typedef BOOL (^RCTBridgeDidNotFindModuleBlock)(RCTBridge *bridge, NSString *moduleName); @@ -41,7 +42,7 @@ typedef BOOL (^RCTBridgeDidNotFindModuleBlock)(RCTBridge *bridge, NSString *modu @property (nonatomic, assign, readonly) BOOL turboModuleEnabled; /// Return the bundle URL for the main bundle. -@property (nonatomic) NSURL *bundleURL; +@property (nonatomic, nonnull) RCTBundleURLBlock bundleURLBlock; /** * Use this method to initialize a new instance of `RCTRootViewFactoryConfiguration` by passing a `bundleURL` @@ -52,10 +53,15 @@ typedef BOOL (^RCTBridgeDidNotFindModuleBlock)(RCTBridge *bridge, NSString *modu * pointing to a path inside the app resources, e.g. `file://.../main.jsbundle`. * */ +- (instancetype)initWithBundleURLBlock:(RCTBundleURLBlock)bundleURLBlock + newArchEnabled:(BOOL)newArchEnabled + turboModuleEnabled:(BOOL)turboModuleEnabled + bridgelessEnabled:(BOOL)bridgelessEnabled NS_DESIGNATED_INITIALIZER; + - (instancetype)initWithBundleURL:(NSURL *)bundleURL newArchEnabled:(BOOL)newArchEnabled turboModuleEnabled:(BOOL)turboModuleEnabled - bridgelessEnabled:(BOOL)bridgelessEnabled; + bridgelessEnabled:(BOOL)bridgelessEnabled __deprecated; /** * Block that allows to override logic of creating root view instance. diff --git a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm index f8acdcd8aed..6e93e4835f3 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm @@ -57,9 +57,23 @@ static NSDictionary *updateInitialProps(NSDictionary *initialProps, BOOL isFabri newArchEnabled:(BOOL)newArchEnabled turboModuleEnabled:(BOOL)turboModuleEnabled bridgelessEnabled:(BOOL)bridgelessEnabled +{ + return [self + initWithBundleURLBlock:^{ + return bundleURL; + } + newArchEnabled:newArchEnabled + turboModuleEnabled:turboModuleEnabled + bridgelessEnabled:bridgelessEnabled]; +} + +- (instancetype)initWithBundleURLBlock:(RCTBundleURLBlock)bundleURLBlock + newArchEnabled:(BOOL)newArchEnabled + turboModuleEnabled:(BOOL)turboModuleEnabled + bridgelessEnabled:(BOOL)bridgelessEnabled { if (self = [super init]) { - _bundleURL = bundleURL; + _bundleURLBlock = bundleURLBlock; _fabricEnabled = newArchEnabled; _turboModuleEnabled = turboModuleEnabled; _bridgelessEnabled = bridgelessEnabled; @@ -214,13 +228,13 @@ static NSDictionary *updateInitialProps(NSDictionary *initialProps, BOOL isFabri } __weak __typeof(self) weakSelf = self; - _reactHost = [[RCTHost alloc] initWithBundleURL:[self bundleURL] - hostDelegate:nil - turboModuleManagerDelegate:_turboModuleManagerDelegate - jsEngineProvider:^std::shared_ptr() { - return [weakSelf createJSRuntimeFactory]; - } - launchOptions:launchOptions]; + _reactHost = [[RCTHost alloc] initWithBundleURLProvider:self->_configuration.bundleURLBlock + hostDelegate:nil + turboModuleManagerDelegate:_turboModuleManagerDelegate + jsEngineProvider:^std::shared_ptr() { + return [weakSelf createJSRuntimeFactory]; + } + launchOptions:launchOptions]; [_reactHost setBundleURLProvider:^NSURL *() { return [weakSelf bundleURL]; }]; @@ -276,7 +290,7 @@ static NSDictionary *updateInitialProps(NSDictionary *initialProps, BOOL isFabri - (NSURL *)bundleURL { - return self->_configuration.bundleURL; + return self->_configuration.bundleURLBlock(); } @end diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost+Internal.h b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost+Internal.h index 4432aa0cce4..e1708784a45 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost+Internal.h +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost+Internal.h @@ -9,8 +9,6 @@ #import "RCTContextContainerHandling.h" -typedef NSURL * (^RCTHostBundleURLProvider)(void); - @interface RCTHost (Internal) - (void)registerSegmentWithId:(NSNumber *)segmentId path:(NSString *)path; diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.h b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.h index 8f3c0460f38..87f6de0b7fa 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.h +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.h @@ -21,6 +21,8 @@ NS_ASSUME_NONNULL_BEGIN @protocol RCTTurboModuleManagerDelegate; +typedef NSURL *_Nullable (^RCTHostBundleURLProvider)(void); + // Runtime API @protocol RCTHostDelegate @@ -45,11 +47,17 @@ typedef std::shared_ptr (^RCTHostJSEngineProv @interface RCTHost : NSObject +- (instancetype)initWithBundleURLProvider:(RCTHostBundleURLProvider)provider + hostDelegate:(id)hostDelegate + turboModuleManagerDelegate:(id)turboModuleManagerDelegate + jsEngineProvider:(RCTHostJSEngineProvider)jsEngineProvider + launchOptions:(nullable NSDictionary *)launchOptions NS_DESIGNATED_INITIALIZER; + - (instancetype)initWithBundleURL:(NSURL *)bundleURL hostDelegate:(id)hostDelegate turboModuleManagerDelegate:(id)turboModuleManagerDelegate jsEngineProvider:(RCTHostJSEngineProvider)jsEngineProvider - launchOptions:(nullable NSDictionary *)launchOptions NS_DESIGNATED_INITIALIZER; + launchOptions:(nullable NSDictionary *)launchOptions __deprecated; @property (nonatomic, weak, nullable) id runtimeDelegate; diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm index c5d9ae26b50..b72949a52f9 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm @@ -79,15 +79,31 @@ class RCTHostHostTargetDelegate : public facebook::react::jsinspector_modern::Ho _RCTInitializeJSThreadConstantInternal(); } -/** - Host initialization should not be resource intensive. A host may be created before any intention of using React Native - has been expressed. - */ - (instancetype)initWithBundleURL:(NSURL *)bundleURL hostDelegate:(id)hostDelegate turboModuleManagerDelegate:(id)turboModuleManagerDelegate jsEngineProvider:(RCTHostJSEngineProvider)jsEngineProvider launchOptions:(nullable NSDictionary *)launchOptions +{ + return [self + initWithBundleURLProvider:^{ + return bundleURL; + } + hostDelegate:hostDelegate + turboModuleManagerDelegate:turboModuleManagerDelegate + jsEngineProvider:jsEngineProvider + launchOptions:launchOptions]; +} + +/** + Host initialization should not be resource intensive. A host may be created before any intention of using React Native + has been expressed. + */ +- (instancetype)initWithBundleURLProvider:(RCTHostBundleURLProvider)provider + hostDelegate:(id)hostDelegate + turboModuleManagerDelegate:(id)turboModuleManagerDelegate + jsEngineProvider:(RCTHostJSEngineProvider)jsEngineProvider + launchOptions:(nullable NSDictionary *)launchOptions { if (self = [super init]) { _hostDelegate = hostDelegate; @@ -99,7 +115,6 @@ class RCTHostHostTargetDelegate : public facebook::react::jsinspector_modern::Ho _launchOptions = [launchOptions copy]; __weak RCTHost *weakSelf = self; - auto bundleURLGetter = ^NSURL *() { RCTHost *strongSelf = weakSelf; @@ -124,7 +139,6 @@ class RCTHostHostTargetDelegate : public facebook::react::jsinspector_modern::Ho return strongSelf->_bundleURLProvider(); }; - [self _setBundleURL:bundleURL]; [_bundleManager setBridgelessBundleURLGetter:bundleURLGetter andSetter:bundleURLSetter andDefaultGetter:defaultBundleURLGetter]; @@ -170,6 +184,9 @@ class RCTHostHostTargetDelegate : public facebook::react::jsinspector_modern::Ho - (void)start { + if (_bundleURLProvider) { + [self _setBundleURL:_bundleURLProvider()]; + } auto &inspectorFlags = jsinspector_modern::InspectorFlags::getInstance(); if (inspectorFlags.getEnableModernCDPRegistry() && !_inspectorPageId.has_value()) { _inspectorTarget =