chore: remove old architecture event listener callbacks #546
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.
|
| private val inAppMessagingModule: ModuleMessagingInApp? | ||
| get() = kotlin.runCatching { CustomerIO.instance().inAppMessaging() }.getOrNull() | ||
| val inAppEventListener = ReactInAppEventListener() | ||
| val inAppEventListener = ReactInAppEventListener.shared |
There was a problem hiding this comment.
Because we will be removing NativeMessagingInAppModuleImpl with removal of old arch support in next PR, we'll now be accessing singleton instance using ReactInAppEventListener.shared in both React Native SDK and Expo Plugin. Expo plugin will require an update after this change though but won't be breaking for customers as Expo Plugin is now fixed to one React Native SDK version.
android/src/main/java/io/customer/reactnative/sdk/messaginginapp/ReactInAppEventListener.kt
Outdated
Show resolved
Hide resolved
| Build.SUPPORTED_ABIS?.firstOrNull() | ||
| ?.lowercase() | ||
| ?.contains("armeabi") == true | ||
| } |
There was a problem hiding this comment.
Good job cleaning this up!
mahmoud-elmorabea
left a comment
mahmoud-elmorabea
left a comment
There was a problem hiding this comment.
Looks great 💯 Thanks for accommodating suggestions 😇
| const eventEmitter = new NativeEventEmitter(native); | ||
| return eventEmitter.addListener(InAppEventListenerEventName, emitter); | ||
| // Return a no-op subscription to maintain backwards compatibility | ||
| return { |
There was a problem hiding this comment.
If new-arch is now mandatory, can we throw an explicit error here so customers know they must enable TurboModules? otherwise it would be silent failures for them?
There was a problem hiding this comment.
Apps using old arch won't compile so the failure wouldn't be silent.
There was a problem hiding this comment.
On old architecture:
- First call: onCioLogEvent() throws → logs warning → sets isInitialized = true
- Subsequent calls: Early return at line 23 prevents any retry
Once marked initialized, the system never attempts to reconnect, is this the intent here?
There was a problem hiding this comment.
Old arch is removed in next PRs in stack
part of: MBL-1476, MBL-1477
Summary
Removes legacy event listener methods (
addListener,removeListeners) from React Native SDK, helping complete the cleanup of support for old architecture. This simplifies the codebase and improves alignment with new architecture (TurboModules + Fabric).Changes
NativeEventEmitterfallback fromcustomerio-inapp.tsandnative-logger-listener.tsaddListenerandremoveListenersfromTurboModulespecsNativeCustomerIOLoggingModuleandNativeMessagingInAppModuleReactInAppEventListenerto use a singleton pattern with lazy initialization (ReactInAppEventListener.shared) for better compatibility with Expo's plugin setupReactInAppEventListener.sharedsingleton instancearmeabi/armeabi-v7aas the crash was fixed in React Native core and these workarounds are no longer necessaryPR Stack:
Note
Remove old-architecture event listener support and fallbacks, adopt a singleton in-app event listener, and simplify Android logging/messaging modules and TurboModule specs.
ReactInAppEventListenerconverted to a lazy singleton (instance), updated usage inNativeMessagingInAppModuleImplandNativeMessagingInAppModule.setLogEventEmitterwiring.NativeEventEmitterfallbacks and old-arch listener methods; listeners now use TurboModule events only with guarded registration and a no-op subscription fallback on failure.addListener/removeListenersfromNativeCustomerIOLoggingandNativeCustomerIOMessagingInApp; specs expose onlyonCioLogEvent/onInAppEventReceivedanddismissMessage.Written by Cursor Bugbot for commit d01b4b7. This will update automatically on new commits. Configure here.