-
Notifications
You must be signed in to change notification settings - Fork 15
feat!: remove support for React Native old architecture #553
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
Conversation
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
2202936 to
8bd725c
Compare
| emitOnCioLogEvent(data) | ||
| } | ||
| } | ||
| } |
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.
Removed armeabi crash protection in logging module
The previous new architecture logging module (src/newarch/.../NativeCustomerIOLoggingModule.kt) included explicit protection to skip TurboModule event emission on armeabi/armeabi-v7a devices with the comment "avoid known native (C++) crashes on unsupported ABIs". This isABIArmeabi check and runOnSupportedAbi wrapper have been removed in the consolidated module. While the code is wrapped in runWithTryCatch, Java try-catch cannot intercept native C++ crashes (SIGSEGV). On affected architectures, logging events could now trigger crashes that were previously avoided. This may be acceptable given React Native 0.83's new architecture focus, but the protection was explicitly added for a reason.
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.
That's correct. At some point we might remove this try-catch too since it doesn't really help with C++ crashes and isn't something we can control. For now, we'll keep the simple try-catch and re-evaluate once this version is rolled out more widely and we see how it behaves.
mahmoud-elmorabea
left a comment
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.
💯
Shahroz16
left a comment
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.
should we probably make it a breaking change? as we are removing the support for old arch?
closes: Remove Support for React Native Old Architecture
Summary
This PR removes all remaining code and logic related to React Native's old architecture, simplifying the SDK codebase to fully support and rely on New Architecture (TurboModules + Fabric). This aligns with React Native's direction of deprecating old architecture and simplifies maintenance, and future feature work.
Changes
0.834.13.0to4.14.0Notes
References
Note
Fully removes React Native old-architecture paths and consolidates the SDK on new-architecture modules and Fabric UI components.
BaseReactPackage(no old/new split); always applycom.facebook.react; removes new-arch toggles/sourceSets; updatescioSDKVersionAndroidto4.14.0NativeCustomerIO*, Logging, In-App, Push) to TurboModules-only; replaces legacy impl helpers with direct modules; aligns APIs (setProfileAttributes/setDeviceAttributes) and event emission via codegenEventEmitterRCTViewComponentViewand proper protocol; removes legacy view manageraddListener/removeListeners; JS listeners now attach via codegen EventEmitter with try-catch fallbacks (no NativeEventEmitter)0.83and React19.2; lockfiles bumped (Hermes, Metro, eslint configs, etc.) and minor iOS script quoting fixWritten by Cursor Bugbot for commit 8bd725c. This will update automatically on new commits. Configure here.