Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PushNotificationIOS.requestPermissions won't resolve if no event listeners are attached #13012

Closed
frantic opened this issue Mar 18, 2017 · 5 comments
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.

Comments

@frantic
Copy link
Contributor

frantic commented Mar 18, 2017

Description

RCTPushNotificationManager uses startObserving to register for RCTRegisterUserNotificationSettings. According to the docs, the startObserving method won't be called until somebody subscribes to NotificationManagerIOS.

This means there is a scenario when the developer can call requestPermissions without subscribing to notifications first, but since RCTPushNotificationManager relies on NSNotificationCenter subscribtion, the result will never be returned.

This should be fairly straightforward to solve for somebody new to React Native. Feel free to send a PR!

Reproduction

Have this in your JS file

PushNotificationIOS.requestPermissions().then(console.log);

You'll see iOS permissions dialog, however pressing "Allow" won't resolve the promise and nothing will be printed to the console.

Solution

Change the code to this, then uninstall and run the app again (needed to reset iOS permissions cache):

PushNotificationIOS.addEventListener('localNotification', () => {});
PushNotificationIOS.requestPermissions().then(console.log);

Now "Allow" button will work as expected, the promise will be resolved and you'll see log entry.

Additional Information

  • React Native version: 0.42.0
  • Platform: iOS
  • Operating System: macOS
@frantic frantic added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Platform: iOS iOS applications. labels Mar 18, 2017
@binoy14
Copy link

binoy14 commented Mar 20, 2017

@frantic If you are okay with it. I would like to take a stab at this.

@frantic
Copy link
Contributor Author

frantic commented Mar 21, 2017

@binoy14 sure!

@binoy14
Copy link

binoy14 commented Mar 21, 2017

@frantic Can you point me in the right direction? I am not sure which file to edit and what exactly to do.

@frantic
Copy link
Contributor Author

frantic commented Mar 22, 2017

@binoy14 how familiar are you with React Native? I'd recommend starting with building a simple app that uses local notifications. Then you can try reproduce this bug locally on your computer. Once you have the repro case, check out PushNotificationsIOS native module (Objective-C) and see if you can move some event subscriptions out of startObserving and into methods that actually depend on them.

thotegowda pushed a commit to thotegowda/react-native that referenced this issue May 7, 2017
…eners are attached

Summary:
Resolves facebook#13012

RCTPushNotificationManager uses startObserving to register for RCTRegisterUserNotificationSettings. According to the docs, the startObserving method won't be called until somebody subscribes to NotificationManagerIOS.

This means there is a scenario when the developer can call requestPermissions without subscribing to notifications first, but since RCTPushNotificationManager relies on NSNotificationCenter subscribtion, the result will never be returned.

When requesting permissions  the promise will resolve:
`PushNotificationIOS.requestPermissions().then(console.log);` without the need for calling `PushNotificationIOS.addEventListener()` first.
Closes facebook#13263

Differential Revision: D4851767

Pulled By: javache

fbshipit-source-id: 2be8621e072ae1086014594bc986ca5590b5eb61
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 18, 2018
@cpojer
Copy link
Contributor

cpojer commented Feb 13, 2019

This issue has been moved to react-native-push-notification/ios#3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
4 participants