From eda22d0d44fe6998175371757f66b5d4bd4fd2ee Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 16 Jun 2020 14:33:14 -0700 Subject: [PATCH] notifications [nfc]: Split `listen` into `listenAndroid` and `listenIOS`. In an upcoming commit, we'll abandon `react-native-notifications` (which we'd only been using on iOS) in favor of PushNotificationIOS, provided by React Native (and then we'll take that same code from react-native-community, where it's maintained now). Once we've switched to PushNotificationIOS, none of the events we can listen to on iOS will match the Android events' names. That matching was pointed out in 400864361: """ And the names we use over the `DeviceEventEmitter` channel on Android are chosen to match the names the Wix library offers on its own event source... """ That meant that there were a few possible cases where we could call `this.listen` and pass a single name that wasn't conditioned on the platform. We took advantage of that possibility in one or two places, but we'll no longer be able to, unless we change the Android names to match these (keep reading for one reason not to do this). I'm happy to now have the `PushNotificationEventName` type from PushNotificationIOS (whether from RN or RNC) so we don't accidentally listen for events on iOS that won't be emitted. I'm optimistic that PushNotificationIOS may eventually have types that associate those names with the iOS-specific data shapes that are passed to their corresponding listeners. E.g., if I'm listening for the 'notification' event, my listener must only accept a PushNotificationIOS object. If such types are eventually provided, we'll want to use them -- but keep them away from Android codepaths. Having `this.listenIOS` and `this.listenAndroid` be separate, as well as keeping the events' names in different namespaces, will help with that. One gain from 8ee1793c1 is easily preserved: these helper functions still ensure that the appropriate functions are called to remove the listeners on `unlistenAll`. --- src/notification/index.js | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/notification/index.js b/src/notification/index.js index 01daada9e28..7fb9b762c18 100644 --- a/src/notification/index.js +++ b/src/notification/index.js @@ -184,14 +184,15 @@ export class NotificationListener { } /** Private. */ - listen(name: string, handler: (...empty) => void | Promise) { - if (Platform.OS === 'ios') { - NotificationsIOS.addEventListener(name, handler); - this.unsubs.push(() => NotificationsIOS.removeEventListener(name, handler)); - } else { - const subscription = DeviceEventEmitter.addListener(name, handler); - this.unsubs.push(() => subscription.remove()); - } + listenIOS(name: string, handler: (...empty) => void | Promise) { + NotificationsIOS.addEventListener(name, handler); + this.unsubs.push(() => NotificationsIOS.removeEventListener(name, handler)); + } + + /** Private. */ + listenAndroid(name: string, handler: (...empty) => void | Promise) { + const subscription = DeviceEventEmitter.addListener(name, handler); + this.unsubs.push(() => subscription.remove()); } /** Private. */ @@ -245,22 +246,20 @@ export class NotificationListener { if (Platform.OS === 'android') { // On Android, the object passed to the handler is constructed in // FcmMessage.kt, and will always be a Notification. - this.listen('notificationOpened', this.handleNotificationOpen); + this.listenAndroid('notificationOpened', this.handleNotificationOpen); + this.listenAndroid('remoteNotificationsRegistered', this.handleDeviceToken); } else { // On iOS, `note` should be an IOSNotifications object. The notification // data it returns from `getData` is unvalidated -- it comes almost // straight off the wire from the server. - this.listen('notificationOpened', (note: { getData(): JSONableDict }) => { + this.listenIOS('notificationOpened', (note: { getData(): JSONableDict }) => { const data = fromAPNs(note.getData()); if (data) { this.handleNotificationOpen(data); } }); - } - - this.listen('remoteNotificationsRegistered', this.handleDeviceToken); - if (Platform.OS === 'ios') { - this.listen('remoteNotificationsRegistrationFailed', this.handleIOSRegistrationFailure); + this.listenIOS('remoteNotificationsRegistered', this.handleDeviceToken); + this.listenIOS('remoteNotificationsRegistrationFailed', this.handleIOSRegistrationFailure); } if (Platform.OS === 'android') {