-
Notifications
You must be signed in to change notification settings - Fork 26
feat: add method to deallocate reactNativeFactory instance for iOS #137
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
base: main
Are you sure you want to change the base?
feat: add method to deallocate reactNativeFactory instance for iOS #137
Conversation
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.
Thanks for working on this! 🚀
Overall looks good, left couple of questions.
ios/ReactNativeBrownfield.swift
Outdated
return | ||
} | ||
|
||
guard let factory = reactNativeFactory else { return } |
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.
guard let factory = reactNativeFactory else { return } | |
guard let reactNativeFactory else { return } |
ios/ReactNativeBrownfield.swift
Outdated
private func checkFactoryInitialized(launchOptions: [AnyHashable: Any]? = nil) { | ||
if reactNativeFactory == nil { | ||
delegate.dependencyProvider = RCTAppDependencyProvider() | ||
self.reactNativeFactory = RCTReactNativeFactory(delegate: delegate) | ||
} | ||
} |
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.
Can we create w getter on the factory that initizalizes it if it's nil? It should remove the need to call this function
factory.bridge?.invalidate() | ||
|
||
NotificationCenter.default.removeObserver(self) | ||
onBundleLoaded = nil | ||
|
||
reactNativeFactory = nil |
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.
Did you test the scenario of what happens when react native view is still shown while calling this? Let's make sure it doesn't crash the app.
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.
Maybe there is a way to detect current react native surfaces in the app and show a warning if the user tries to do this with views still on screen
98b8d14
to
9264071
Compare
c11f9c0
to
5bef4b4
Compare
@okwasniewski, I refactored the factory a bit and added some additional checks due to Cursor Bugbot comments. When it comes to the Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-09-24.at.22.52.53.mp4 |
Summary
RCTHost
).rootViewFactory
to avoid retaining the old runtime.startReactNative()
API remains;view()
now self‑heals if RN was previously stopped.Test plan
stopReactNative
called before RN was initialize)