react-native-onesignal
react-native-onesignal copied to clipboard
Add getInitialNotification() on iOS
TL;DR
This PR adds the ability to call getInitialNotification()
on react-native-onesignal.
Description
The aim is to provide an API that is similar to React-Native's Linking and PushNotificationIOS modules. Both of these stock modules allow to register event handlers, as well as providing the initial data that triggered the app launch.
From my point of view, triggering a onOpened
event in the case of an app launch from a notification if flawed. You have to race to register your listener before the event is dispatched, and this means you're introducing a luck factor in your app initialization.
I cannot rely on luck nor do I wish to race in an already complicated Javascript-timed environment.
So this PR is a neat way of doing things. You can call getInitialNotification()
and get the data from the notification that opened the app. You can call it soon, late, it doesn't matter, it's available 24/7.
It also comes in handy in case you need to store the initial notification or need to deal with it at a later point. No more redux needed to save your notification for later use.
This addresses #332, for real this time.
Android notice
I'm no Android developer, and I did not implement the function in the native Android module. If someone feels like Android could benefit from this as well, don't hesitate to contribute!
Trying to call getInitialNotification()
on Android will simply console.log a message saying it's iOS only.
Usage
import OneSignal from 'react-native-onesignal'
OneSignal.getInitialNotification().then((notification) => {
this.onNotificationOpened(notification)
}).catch((error) => { console.error(error) })
// or
OneSignal.getInitialNotification().then((notification) => {
this.onNotificationOpened(notification)
})
// or
try {
const notification = await OneSignal.getInitialNotification()
console.log(notification)
} catch (error) {
console.error(error)
}
Breaking changes
If the iOS app is opened by a notification, the native module will no longer trigger the onOpened
callback.
This choice has been made to prevent cases where the old way was working [insert random number here]% of the time. A working event trigger AND using getInitialNotification()
could make you handle the initial notification twice. So we're avoiding this right of the bat.
You can easily update your code by adding getInitialNotification()
in addition to your listener, and basically give it the same callback. This ensures you handle the initial notification properly, and your listener handles any future notifications.
Travis build failed because the Android build failed, and I didn't change anything on Android side. Can someone please review this PR?
Hi, could you elaborate on this functionality a little bit?
@avishayil well I’m not sure what else can I say to explain what this PR is about... What part don’t you get?
Appearantly your PR documentation appears correctly now for me. Maybe GitHub problems. @jkasten2 why was this hadn't been approved yet?
This PR is not necessary as the SDK already waits for an observer to be added before it triggers the opened
event. The race condition scenario described in the initial PR should not be a concern for this reason.
@habovh if you were adding the opened
observer yet weren’t getting all opened events, that would definitely be a bug. We’ve actually fixed a few bugs related to this in recent versions of the SDK so you may want to check.
Notifications can be opened while the app is already running in the background, so I am also fundamentally opposed to the idea of using a special function to get these notifications.
@Nightsd01 thanks for taking the time to review. If the SDK waits for a listener to be registered, I agree that the race condition scenario I describe is a bug. I wasn't aware that the SDK waits for listener to be registered prior triggering the opened
event, mainly because it simply didn't work in my case.
If the SDK waits as promised, then I understand that getInitialNotification()
should not be used to emulate the same behaviour, hence making my PR irrelevant.
By the way, my PR didn't prevent the already-running app receive and handle notifications using the opened
event!
However, it then could be useful to have this getInitialNotification()
available in order to be able to check if the app was opened from a notification, and to recall the data associated with the said notification programatically at any given time. Just an idea here.
Need this. The OneSignal React-Native iOS SDK is really incomplete compared to the Android one...