Skip to content

Commit

Permalink
notifications [nfc]: Split listen into listenAndroid and `listenI…
Browse files Browse the repository at this point in the history
…OS`.

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 4008643:

"""
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 8ee1793 is easily preserved: these helper functions
still ensure that the appropriate functions are called to remove the
listeners on `unlistenAll`.
  • Loading branch information
chrisbobbe committed May 10, 2021
1 parent 906da3d commit eda22d0
Showing 1 changed file with 14 additions and 15 deletions.
29 changes: 14 additions & 15 deletions src/notification/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,15 @@ export class NotificationListener {
}

/** Private. */
listen(name: string, handler: (...empty) => void | Promise<void>) {
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<void>) {
NotificationsIOS.addEventListener(name, handler);
this.unsubs.push(() => NotificationsIOS.removeEventListener(name, handler));
}

/** Private. */
listenAndroid(name: string, handler: (...empty) => void | Promise<void>) {
const subscription = DeviceEventEmitter.addListener(name, handler);
this.unsubs.push(() => subscription.remove());
}

/** Private. */
Expand Down Expand Up @@ -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') {
Expand Down

0 comments on commit eda22d0

Please sign in to comment.