-
Notifications
You must be signed in to change notification settings - Fork 24.8k
Closed
Labels
Ran CommandsOne of our bots successfully processed a command.One of our bots successfully processed a command.Resolution: LockedThis issue was locked by the bot.This issue was locked by the bot.
Description
I've been following #2332 and am glad that it looks like it will be in, but I still think there is an issue. I'm running iOS 8+
I'm seeing the push registration callback twice.
My appdelegate looks like this...
- (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings {
[RCTPushNotificationManager application:application didRegisterUserNotificationSettings:notificationSettings];
}
- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken
{
[RCTPushNotificationManager application:application didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
}
- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)notification
{
[RCTPushNotificationManager application:application didReceiveRemoteNotification:notification];
}
The JS code ends up calling requestPermissions
(RCTPushNotificationManager.m) which does
[[UIApplication sharedApplication] registerUserNotificationSettings:notificationSettings];
[[UIApplication sharedApplication] registerForRemoteNotifications];
Then there is the appdelgate callback to didRegisterUserNotificationSettings and then didRegisterForRemoteNotificationsWithDeviceToken
(both)
The first one calls back into RCTPushNotificationManager, which does this.
+ (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(__unused UIUserNotificationSettings *)notificationSettings
{
[application registerForRemoteNotifications];
}
You'll note there is a second call to registerForRemoteNotifications
My iOS guy says he would remove the call to registerForRemoteNotifications
in requestPermissions
Anyone else seeing this?
Metadata
Metadata
Assignees
Labels
Ran CommandsOne of our bots successfully processed a command.One of our bots successfully processed a command.Resolution: LockedThis issue was locked by the bot.This issue was locked by the bot.
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
DannyvanderJagt commentedon Oct 19, 2015
I have tried to replicate your problem but the register callback keeps firing once.
Which version of RN are you using?
If you did use v0.12 and lower did you apply the fix from #2332 ?
bleonard commentedon Dec 3, 2015
I'll try it again in the more recent releases.
chirag04 commentedon Jan 8, 2016
I'm also seeing this on 0.17.
registerPermissions calls
react-native/Libraries/PushNotificationIOS/RCTPushNotificationManager.m
Line 160 in b9c5f75
and
didRegisterUserNotificationSettings
also callsregisterForRemoteNotifications
react-native/Libraries/PushNotificationIOS/RCTPushNotificationManager.m
Line 78 in b9c5f75
cc @ericvicenti @nicklockwood
insraq commentedon Feb 1, 2016
I've tried this on RN 0.19 and can reproduce this issue.
yueshuaijie commentedon Feb 2, 2016
I use 0.19, have this problem, too.
my code:
{
[RCTPushNotificationManager didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
}
MichaelCereda commentedon Feb 3, 2016
i'm currently on 0.18 and i see this problem too, to avoid too many useless calls to my server i solved (not really) the issue adding a timeout+lock similar to this
insraq commentedon Feb 3, 2016
@MichaelCereda You could just comment out the the call to
registerForRemoteNotifications
inrequestPermission
. That's way easier. Just patchRCTPushNotificationManager.m
react-native/Libraries/PushNotificationIOS/RCTPushNotificationManager.m
Line 160 in b9c5f75
MichaelCereda commentedon Feb 3, 2016
thanks @insraq i just used this method because i don't want to patch react-native itself (and lose the changes in case of an update) :)
thank you anyways.
insraq commentedon Feb 11, 2016
Can we have a PR for this?
jacob-israel-turner commentedon Feb 23, 2016
I'm having this same issue, a PR would be nice. Is the best solution to simply remove the
registerForRemoteNotifications
call inrequestPermissions
?Remove duplicate register call
Remove duplicate register call
1 remaining item
sgonyea commentedon Apr 9, 2016
The change that was merged in for this has actually broken push notifications in 0.23. I can't register for push unless I revert this change, in 0.23.
sgonyea commentedon Apr 9, 2016
The breaking change in question is: 5c41865
Granted, putting it back DOES invoke multiple calls. But that beats no calls :)
sgonyea commentedon Apr 9, 2016
The issue is basically that I can request to send push notifications (that works!) but I can't seem to get the Push Token via JS. So although the user's approved it, I can't send the token back to the server. 😢
insraq commentedon Apr 13, 2016
@sgonyea I've test 0.23 on my device. It works without any issue. Have you set up the event handlers in AppDelegate.m according to this page? https://facebook.github.io/react-native/docs/pushnotificationios.html
The code for event hook has changed as I recall. If you follow the old instruction, you might need to update your code.
tomazahlin commentedon Sep 1, 2016
Having the same issue, any news on this?
charpeni commentedon Nov 14, 2016
@facebook-github-bot label Icebox
charpeni commentedon Nov 14, 2016
Hi there! This issue is being closed because it has been inactive for a while.
But don't worry, it will live on with ProductPains! Check out its new home: https://productpains.com/post/react-native/registering-push-notifications-twice
ProductPains helps the community prioritize the most important issues thanks to its voting feature.
It is easy to use - just login with GitHub.
Also, if this issue is a bug, please consider sending a PR with a fix.
We're a small team and rely on the community for bug fixes of issues that don't affect fb apps.
charpeni commentedon Nov 14, 2016
@facebook-github-bot close
facebook-github-bot commentedon Nov 14, 2016
@charpeni tells me to close this issue. If you think it should still be opened let us know why.