-
Notifications
You must be signed in to change notification settings - Fork 36
(feat): add method to deallocate reactNativeFactory instance #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
1919271
1fdc967
0b28d4f
52c889c
7be2698
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@callstack/react-native-brownfield': minor | ||
| --- | ||
|
|
||
| feat: add method to deallocate reactNativeFactory instance |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,16 +28,13 @@ final class ExpoHostRuntime { | |
| public func startReactNative(onBundleLoaded: (() -> Void)?) { | ||
| guard reactNativeFactory == nil else { return } | ||
|
|
||
| let factory = ExpoReactNativeFactory(delegate: delegate) | ||
| delegate.dependencyProvider = RCTAppDependencyProvider() | ||
|
|
||
| reactNativeFactory = factory | ||
|
|
||
| let appDelegate = ExpoAppDelegate() | ||
| // below: https://github.com/expo/expo/pull/39418/changes/5abd332b55b2ee7daee848284ed5f7fe1639452e | ||
| // has removed bindReactNativeFactory method from ExpoAppDelegate | ||
| #if !EXPO_SDK_GTE_55 // this define comes from the Brownfield Expo config plugin | ||
| appDelegate.bindReactNativeFactory(factory) | ||
| delegate.dependencyProvider = RCTAppDependencyProvider() | ||
| reactNativeFactory = ExpoReactNativeFactory(delegate: delegate) | ||
| appDelegate.bindReactNativeFactory(reactNativeFactory) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The CI fails because now we are passing We can do a quick: if let reactNativeFactory {
appDelegate.bindReactNativeFactory(reactNativeFactory)
}OR guard let reactNativeFactory else { return }
appDelegate.bindReactNativeFactory(reactNativeFactory) |
||
| #endif | ||
| expoDelegate = appDelegate | ||
|
|
||
|
|
@@ -46,6 +43,24 @@ final class ExpoHostRuntime { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Stops React Native and releases the underlying factory instance. | ||
| */ | ||
| public func stopReactNative() { | ||
| if !Thread.isMainThread { | ||
| DispatchQueue.main.async { [weak self] in self?.stopReactNative() } | ||
| return | ||
| } | ||
|
|
||
| #if !EXPO_SDK_GTE_55 | ||
| guard let factory = reactNativeFactory else { return } | ||
| factory.bridge?.invalidate() | ||
| #endif | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check is here because there is no bridge instance on Expo > 55?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This API, will only work on less than 55. In that too, it may not work for new arch on SDK 54. Do we have any solution for SDK 55 and new arch on 54? |
||
|
|
||
| reactNativeFactory = nil | ||
| expoDelegate = nil | ||
| } | ||
|
|
||
| /** | ||
| * Path to JavaScript root. | ||
| * Default value: ".expo/.virtual-metro-entry" | ||
|
|
@@ -125,19 +140,19 @@ final class ExpoHostRuntime { | |
| // below: https://github.com/expo/expo/commit/2013760c46cde1404872d181a691da72fbf207a4 | ||
| // has moved the recreateRootView method to ExpoReactNativeFactory | ||
| #if EXPO_SDK_GTE_55 // this define comes from the Brownfield Expo config plugin | ||
| return (reactNativeFactory as? ExpoReactNativeFactory)?.recreateRootView( | ||
| withBundleURL: bundleURL, | ||
| moduleName: moduleName, | ||
| initialProps: initialProps, | ||
| launchOptions: launchOptions | ||
| ) | ||
| return (reactNativeFactory as? ExpoReactNativeFactory)?.recreateRootView( | ||
| withBundleURL: bundleURL, | ||
| moduleName: moduleName, | ||
| initialProps: initialProps, | ||
| launchOptions: launchOptions | ||
| ) | ||
| #else | ||
| return expoDelegate?.recreateRootView( | ||
| withBundleURL: bundleURL, | ||
| moduleName: moduleName, | ||
| initialProps: initialProps, | ||
| launchOptions: launchOptions | ||
| ) | ||
| return expoDelegate?.recreateRootView( | ||
| withBundleURL: bundleURL, | ||
| moduleName: moduleName, | ||
| initialProps: initialProps, | ||
| launchOptions: launchOptions | ||
| ) | ||
| #endif | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,7 @@ final class ReactNativeHostRuntime { | |
| delegate.bundlePath = bundlePath | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Bundle instance to lookup the JavaScript bundle. | ||
| * Default value: Bundle.main | ||
|
|
@@ -68,6 +69,7 @@ final class ReactNativeHostRuntime { | |
| delegate.bundle = bundle | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Dynamic bundle URL provider called on every bundle load. | ||
| * When set, this overrides the default bundleURL() behavior in the delegate. | ||
|
|
@@ -79,17 +81,12 @@ final class ReactNativeHostRuntime { | |
| delegate.bundleURLOverride = bundleURLOverride | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * React Native factory instance created when starting React Native. | ||
| * Default value: nil | ||
| */ | ||
| private var reactNativeFactory: RCTReactNativeFactory? = nil | ||
| /** | ||
| * Root view factory used to create React Native views. | ||
| */ | ||
| lazy private var rootViewFactory: RCTRootViewFactory? = { | ||
| return reactNativeFactory?.rootViewFactory | ||
| }() | ||
|
|
||
| /** | ||
| * Starts React Native with default parameters. | ||
|
|
@@ -98,12 +95,24 @@ final class ReactNativeHostRuntime { | |
| startReactNative(onBundleLoaded: nil) | ||
| } | ||
|
|
||
| /** | ||
| * Stops React Native and releases the underlying factory instance. | ||
| */ | ||
| public func stopReactNative() { | ||
| if !Thread.isMainThread { | ||
| DispatchQueue.main.async { [weak self] in self?.stopReactNative() } | ||
| return | ||
| } | ||
|
|
||
| reactNativeFactory = nil | ||
| } | ||
|
Comment on lines
+101
to
+108
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you accidentally remove the call to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I guess that was a consequence of this topic: #301 (comment) should it be restored then?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah no, they should not be restored. All good. One thing, can you also remove this from
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, will be done in a next commit |
||
|
|
||
| public func view( | ||
| moduleName: String, | ||
| initialProps: [AnyHashable: Any]?, | ||
| launchOptions: [AnyHashable: Any]? = nil | ||
| ) -> UIView? { | ||
| rootViewFactory?.view( | ||
| reactNativeFactory?.rootViewFactory.view( | ||
| withModuleName: moduleName, | ||
| initialProperties: initialProps, | ||
| launchOptions: launchOptions | ||
|
|
@@ -145,7 +154,7 @@ final class ReactNativeHostRuntime { | |
| guard reactNativeFactory == nil else { return } | ||
|
|
||
| delegate.dependencyProvider = RCTAppDependencyProvider() | ||
| self.reactNativeFactory = RCTReactNativeFactory(delegate: delegate) | ||
| reactNativeFactory = RCTReactNativeFactory(delegate: delegate) | ||
|
|
||
| if let onBundleLoaded { | ||
| jsBundleLoadObserver.observeOnce(onBundleLoaded: onBundleLoaded) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two functions should be present for Expo55 as well, we can keep them outside of the
#if !EXPO_SDK_GTE_55block.