From 839373600823bb156a012e6f138cf59105cf7c83 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. [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 | 15 ++++++--- .../AppDelegate/RCTRootViewFactory.h | 14 ++++++-- .../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, 77 insertions(+), 25 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm index cee2c9017bb3b9..6d159ef598a286 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm @@ -233,13 +233,18 @@ - (Class)getModuleClassFromName:(const char *)name - (RCTRootViewFactory *)createRCTRootViewFactory { + __weak __typeof(self) weakSelf = self; + RCTBundleURLBlock bundleUrlBlock = ^{ + RCTAppDelegate *strongSelf = weakSelf; + return strongSelf.bundleURL; + }; + RCTRootViewFactoryConfiguration *configuration = - [[RCTRootViewFactoryConfiguration alloc] initWithBundleURL:self.bundleURL - newArchEnabled:self.fabricEnabled - turboModuleEnabled:self.turboModuleEnabled - bridgelessEnabled:self.bridgelessEnabled]; + [[RCTRootViewFactoryConfiguration alloc] initWithBundleURLBlock:bundleUrlBlock + newArchEnabled:self.fabricEnabled + turboModuleEnabled:self.turboModuleEnabled + bridgelessEnabled:self.bridgelessEnabled]; - __weak __typeof(self) weakSelf = self; 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 a6649c283878de..502203fcdf9c1a 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.h +++ b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.h @@ -23,6 +23,11 @@ typedef UIView *_Nonnull ( ^RCTCreateRootViewWithBridgeBlock)(RCTBridge *bridge, NSString *moduleName, NSDictionary *initProps); 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); #pragma mark - RCTRootViewFactory Configuration @interface RCTRootViewFactoryConfiguration : NSObject @@ -37,7 +42,7 @@ typedef RCTBridge *_Nonnull ( @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` @@ -48,10 +53,15 @@ typedef RCTBridge *_Nonnull ( * 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 4edd8fef3e4218..901b1c9996bd7b 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm @@ -57,9 +57,23 @@ - (instancetype)initWithBundleURL:(NSURL *)bundleURL 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; @@ -213,13 +227,13 @@ - (void)createReactHostIfNeeded:(NSDictionary *)launchOptions } __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]; }]; @@ -248,7 +262,7 @@ - (NSURL *)sourceURLForBridge:(RCTBridge *)bridge - (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 4432aa0cce498d..e1708784a45340 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 8f3c0460f38ed8..87f6de0b7fa045 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 791bdf5f838bbe..924564364256fa 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 @@ + (void)initialize _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 @@ - (instancetype)initWithBundleURL:(NSURL *)bundleURL _launchOptions = [launchOptions copy]; __weak RCTHost *weakSelf = self; - auto bundleURLGetter = ^NSURL *() { RCTHost *strongSelf = weakSelf; @@ -124,7 +139,6 @@ - (instancetype)initWithBundleURL:(NSURL *)bundleURL return strongSelf->_bundleURLProvider(); }; - [self _setBundleURL:bundleURL]; [_bundleManager setBridgelessBundleURLGetter:bundleURLGetter andSetter:bundleURLSetter andDefaultGetter:defaultBundleURLGetter]; @@ -170,6 +184,9 @@ - (instancetype)initWithBundleURL:(NSURL *)bundleURL - (void)start { + if (_bundleURLProvider) { + [self _setBundleURL:_bundleURLProvider()]; + } auto &inspectorFlags = jsinspector_modern::InspectorFlags::getInstance(); if (inspectorFlags.getEnableModernCDPRegistry() && !_inspectorPageId.has_value()) { _inspectorTarget =