-
Notifications
You must be signed in to change notification settings - Fork 2
Fix compatibility issues with React Native 0.80+ and iOS initialization #64
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?
Conversation
Reviewer's GuideUpdates Podspec to handle New Architecture dependencies, defers iOS plugin state setup until after Rownd.configure(), and replaces deprecated YellowBox with LogBox in the JS context for RN 0.80+ compatibility. Sequence diagram for deferred RowndPlugin state initialization in iOSsequenceDiagram
participant App
participant RowndPlugin
participant Rownd
participant EventEmitter
App->>RowndPlugin: init()
Note right of RowndPlugin: state is not initialized
App->>RowndPlugin: configure(appKey)
RowndPlugin->>Rownd: configure(launchOptions, appKey)
Rownd-->>RowndPlugin: configuration complete
RowndPlugin->>Rownd: getInstance().state().subscribe
Rownd-->>RowndPlugin: ObservableState<RowndState>
RowndPlugin->>EventEmitter: sendEvent("update_state", state)
Class diagram for updated RowndPlugin state managementclassDiagram
class RowndPlugin {
- state: ObservableState<RowndState>?
- stateCancellable: AnyCancellable?
+ init()
+ configure(appKey, resolver, rejecter)
}
class ObservableState {
+ $current: RowndState
+ subscribe(callback)
}
class RowndState {
+ toDictionary()
}
RowndPlugin --> ObservableState
ObservableState --> RowndState
Class diagram for GlobalContext LogBox updateclassDiagram
class GlobalContext {
+ state: GlobalState
+ dispatch: React.Dispatch<TAction>
}
class LogBox {
+ ignoreLogs(warnings: string[])
}
GlobalContext ..> LogBox: uses
GlobalContext ..> GlobalState
GlobalContext ..> TAction
Flow diagram for Podspec dependency management updateflowchart TD
A["Podspec loads"] --> B{Is RCT_NEW_ARCH_ENABLED == '1'?}
B -- Yes --> C["Set compiler flags for New Architecture"]
C --> D["Call install_modules_dependencies(s)"]
B -- No --> E["Skip New Architecture dependency setup"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- In RowndPlugin consider adding a deinit to cancel stateCancellable so you don’t leak subscriptions when the plugin is deallocated.
- You may want to document or guard behavior for repeated calls to configure() since subsequent invocations currently won’t reinitialize the state subscription.
- Double-check that you’ve replaced all YellowBox imports with LogBox in GlobalContext.tsx and removed any lingering references to the old API.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In RowndPlugin consider adding a deinit to cancel stateCancellable so you don’t leak subscriptions when the plugin is deallocated.
- You may want to document or guard behavior for repeated calls to configure() since subsequent invocations currently won’t reinitialize the state subscription.
- Double-check that you’ve replaced all YellowBox imports with LogBox in GlobalContext.tsx and removed any lingering references to the old API.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary
This PR addresses three critical compatibility and stability issues:
install_modules_dependenciesfor proper New Architecture dependency management (fixes RCT-Folly compatibility issues)RowndPluginstate initialization until afterconfigure()is called (fixes crash on initialization)YellowBoxwithLogBox(fixes runtime errors)Changes
1. Podspec: Use
install_modules_dependenciesfor New ArchitectureFile:
rownd-react-native.podspecinstall_modules_dependencieshelper from React Native's scriptsReact-Codegen,RCT-Folly,RCTRequired,RCTTypeSafety,ReactCommon/turbomodule/core) withinstall_modules_dependencies(s)References:
2. Swift: Defer state initialization until after configuration
File:
ios/RowndPlugin.swift@ObservedObject private var statetoprivate var state: ObservableState<RowndState>? = nilto allow deferred initializationinit()toconfigure()methodRownd.configure()is called, preventing crashes when accessing Rownd state before it's configuredProblem: Previously, the plugin attempted to access Rownd state during initialization, which could cause crashes if Rownd wasn't configured yet.
3. JavaScript: Replace deprecated YellowBox with LogBox
File:
src/components/GlobalContext.tsxYellowBoximport withLogBoxYellowBox.ignoreWarnings()calls toLogBox.ignoreLogs()Testing
Related Issues
Fixes compatibility issues with:
RCT-Follywith New Architecture #63Versions
Summary by Sourcery
Ensure compatibility with React Native 0.80+ by updating the podspec for new architecture, deferring Swift plugin initialization to prevent crashes, and replacing deprecated YellowBox with LogBox in JavaScript.
Bug Fixes:
Build: