Skip to content

Questions about Silence of Maintainers #519

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

Closed
andersonaddo opened this issue Mar 3, 2020 · 19 comments
Closed

Questions about Silence of Maintainers #519

andersonaddo opened this issue Mar 3, 2020 · 19 comments

Comments

@andersonaddo
Copy link

react-native-push-notification has been abandoned for a while, so this package is pretty much the only well-done, free and maintained package for RN push notifications now.
It seems like you guys are maintaining the project, because commits are still coming in. But y'alls are so silent in regards to issues and PRs. How come?

@puremana
Copy link
Contributor

puremana commented Mar 4, 2020

They aren't maintaining it, hence why all of the PRs to fix and make this useable. It is depressing to look at 50% of the PRs getting closed from stalebot just because the owners don't want to review. I hope someone steps up and becomes more active but I really don't see that happening looking at this history of issues.

@andersonaddo
Copy link
Author

Hm.
That's very unfortunate - that means there's no free and maintained package for cross-platform React Native push notifications. That's strange, since it's a very important feature for many apps.
React Native Community have a maintained package for iOS notifications, hopefully they'll take up one for Android (or, even better, cross platform). I know they were having discussions about that in the past.

@puremana
Copy link
Contributor

puremana commented Mar 4, 2020

One solution could be for someone to fork this, make the changes and actively review and respond to PRs and issues. All you would need is to set docs and publish this as a package.

(Looking at the license I see nothing limiting you from doing this)

@grantvanhelsing
Copy link

Is this package not useable for production apps? I'm running into problems already and I have barely started.

@andersonaddo
Copy link
Author

Oh it's usable.
I know there are some issues with linking, but that's about it. Proper looking requires going through a few GitHub issues to see what problems people have run into.

@roni-castro
Copy link

I think there are some alternatives if you don't intend to have local notifications, like react-native-firebase

@andersonaddo
Copy link
Author

Yes, but the package doesn't handle notifications in general well anymore because they cut support for it recently.

@puremana
Copy link
Contributor

puremana commented Mar 5, 2020

Yes, but the package doesn't handle notifications in general well anymore because they cut support for it recently.

Yeah, they have a paid package now - invertase/react-native-firebase#2566 (comment)

Is this package not useable for production apps? I'm running into problems already and I have barely started.

You can but you more or less have to look through issues and PR's to piece together a working package. For me this included #496, #502 and #517. There may be more depending on what you're intending to do. My implementation was fairly basic.

@tattivitorino
Copy link

@puremana not to mention the #525 issue which I'm facing right now! I receive the notification on Android but it has no title nor Body.
:((((
Having a hard time here!

@puremana
Copy link
Contributor

@tattivitorino try implementing this PR #496, I had the same issue

@tattivitorino
Copy link

@puremana do you think it's safe and ok to use this https://www.npmjs.com/package/patch-package to implement this PR? I've never done it!

@puremana
Copy link
Contributor

@puremana do you think it's safe and ok to use this https://www.npmjs.com/package/patch-package to implement this PR? I've never done it!

Looks good to me

@tattivitorino
Copy link

@puremana thank you! the PR #496 fixed the title and body in the foreground notification, but the icon is not showing! Any insights?

@puremana
Copy link
Contributor

@puremana thank you! the PR #496 fixed the title and body in the foreground notification, but the icon is not showing! Any insights?

Is the icon not showing for both the foreground and background. or just the background? This may help - #517

@luisfuertes
Copy link

luisfuertes commented Apr 7, 2020

+1

@andersonaddo
Copy link
Author

@luisfuertes I suggest you make your own issue for this.
I'd prefer if people only comment on this thread if they're talking about the maintenance of this repo.

@luisfuertes
Copy link

@andersonaddo its ok

@stale
Copy link

stale bot commented May 9, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the 🏚 stale label May 9, 2020
@stale
Copy link

stale bot commented May 16, 2020

The issue has been closed for inactivity.

@stale stale bot closed this as completed May 16, 2020
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 17, 2020
Some (rather rude! yet still disregarded and closed-as-stale)
comments [1] on wix/react-native-notifications report that it's a
shame to see a decrease in activity from the maintainers, since it's
the only viable cross-platform library for RN push notifications.

But we don't even use it in a cross-platform way; we uprooted the
last of the linking logic on Android in 01b33ad. It also seems
strange to have two different libraries doing work to support push
notifications on iOS. See also discussion [2].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   `react-native-notifications` [3] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [4] from RN
   v0.60 to complete our setup. Some parts were missing, whether
   because we didn't need that functionality before, or it wasn't
   available in earlier RN versions.

3. Change the bit under the comment "Called when a notification is
   delivered to a foreground app." to correctly call the completion
   handler with UNNotificationPresentationOptions instead of
   UNAuthorizationOptions. N.B.: Also, remove the options that ask
   it to play a sound and show an alert, as these aren't part of the
   current behavior. That just leaves updating the badge count.

4. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

5. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

6. Do various housekeeping things like removing the libdef.

7. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [5],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

8. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[3]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[4]: https://reactnative.dev/docs/0.60/pushnotificationios
[5]: react-native-push-notification/ios#24 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 13, 2020
Some (rather rude! yet still disregarded and closed-as-stale)
comments [1] on wix/react-native-notifications report that it's a
shame to see a decrease in activity from the maintainers, since it's
the only viable cross-platform library for RN push notifications.

But we don't even use it in a cross-platform way; we uprooted the
last of the linking logic on Android in 01b33ad. It also seems
strange to have two different libraries doing work to support push
notifications on iOS. See also discussion [2].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   `react-native-notifications` [3] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [4] from RN
   v0.60 to complete our setup. Some parts were missing, whether
   because we didn't need that functionality before, or it wasn't
   available in earlier RN versions.

3. Change the bit under the comment "Called when a notification is
   delivered to a foreground app." to correctly call the completion
   handler with UNNotificationPresentationOptions instead of
   UNAuthorizationOptions. N.B.: Also, remove the options that ask
   it to play a sound and show an alert, as these aren't part of the
   current behavior. That just leaves updating the badge count.

4. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

5. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

6. Do various housekeeping things like removing the libdef.

7. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [5],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

8. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[3]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[4]: https://reactnative.dev/docs/0.60/pushnotificationios
[5]: react-native-push-notification/ios#24 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 16, 2020
Some (rather rude! yet still disregarded and closed-as-stale)
comments [1] on wix/react-native-notifications report that it's a
shame to see a decrease in activity from the maintainers, since it's
the only viable cross-platform library for RN push notifications.

But we don't even use it in a cross-platform way; we uprooted the
last of the linking logic on Android in 01b33ad. It also seems
strange to have two different libraries doing work to support push
notifications on iOS. See also discussion [2].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   `react-native-notifications` [3] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [4] from RN
   v0.60 to complete our setup. Some parts were missing, whether
   because we didn't need that functionality before, or it wasn't
   available in earlier RN versions.

3. Change the bit under the comment "Called when a notification is
   delivered to a foreground app." to correctly call the completion
   handler with UNNotificationPresentationOptions instead of
   UNAuthorizationOptions. N.B.: Also, remove the options that ask
   it to play a sound and show an alert, as these aren't part of the
   current behavior. That just leaves updating the badge count.

4. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

5. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

6. Do various housekeeping things like removing the libdef.

7. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [5],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

8. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[3]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[4]: https://reactnative.dev/docs/0.60/pushnotificationios
[5]: react-native-push-notification/ios#24 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Nov 10, 2020
Some (rather rude! yet still disregarded and closed-as-stale)
comments [1] on wix/react-native-notifications report that it's a
shame to see a decrease in activity from the maintainers, since it's
the only viable cross-platform library for RN push notifications.

But we don't even use it in a cross-platform way; we uprooted the
last of the linking logic on Android in 01b33ad. It also seems
strange to have two different libraries doing work to support push
notifications on iOS. See also discussion [2].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   `react-native-notifications` [3] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [4] from RN
   v0.60 to complete our setup. Some parts were missing, whether
   because we didn't need that functionality before, or it wasn't
   available in earlier RN versions.

3. Change the bit under the comment "Called when a notification is
   delivered to a foreground app." to correctly call the completion
   handler with UNNotificationPresentationOptions instead of
   UNAuthorizationOptions. N.B.: Also, remove the options that ask
   it to play a sound and show an alert, as these aren't part of the
   current behavior. That just leaves updating the badge count.

4. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

5. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

6. Do various housekeeping things like removing the libdef.

7. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [5],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

8. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[3]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[4]: https://reactnative.dev/docs/0.60/pushnotificationios
[5]: react-native-push-notification/ios#24 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 2, 2020
wix/react-native-notifications doesn't seem to be maintained [1].
expo-notifications also works on both platforms, but we've decided
it's not for us [2].

But we don't even use react-native-notifications in a cross-platform
way; we uprooted the last of the linking logic on Android in
01b33ad. It also seems non-optimal to have two different libraries
doing work to support push notifications on iOS. See also
discussion [3].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   wix/react-native-notifications [4] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [5] from RN
   v0.62 to complete our setup. We hadn't yet done everything in
   these instructions, whether because we didn't need that
   functionality before, or it wasn't available in earlier RN
   versions. We did some of this setup in the previous commit.

3. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

4. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

5. Do various housekeeping things like removing the libdef.

6. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [6],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

7. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and that they navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/1061130
[3]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[4]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[5]: https://reactnative.dev/docs/0.62/pushnotificationios
[6]: react-native-push-notification/ios#24 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Feb 8, 2021
wix/react-native-notifications doesn't seem to be maintained [1].
expo-notifications also works on both platforms, but we've decided
it's not for us [2].

But we don't even use react-native-notifications in a cross-platform
way; we uprooted the last of the linking logic on Android in
01b33ad. It also seems non-optimal to have two different libraries
doing work to support push notifications on iOS. See also
discussion [3].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   wix/react-native-notifications [4] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [5] from RN
   v0.62 to complete our setup. We hadn't yet done everything in
   these instructions, whether because we didn't need that
   functionality before, or it wasn't available in earlier RN
   versions. We did some of this setup in the previous commit.

3. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

4. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

5. Do various housekeeping things like removing the libdef.

6. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [6],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

7. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and that they navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/1061130
[3]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[4]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[5]: https://reactnative.dev/docs/0.62/pushnotificationios
[6]: react-native-push-notification/ios#24 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue May 10, 2021
wix/react-native-notifications doesn't seem to be maintained [1].
expo-notifications also works on both platforms, but we've decided
it's not for us [2].

But we don't even use react-native-notifications in a cross-platform
way; we uprooted the last of the linking logic on Android in
01b33ad. It also seems non-optimal to have two different libraries
doing work to support push notifications on iOS. See also
discussion [3].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   wix/react-native-notifications [4] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [5] from RN
   v0.62 to complete our setup. We hadn't yet done everything in
   these instructions, whether because we didn't need that
   functionality before, or it wasn't available in earlier RN
   versions. We did some of this setup in the previous commit.

3. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

4. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

5. Do various housekeeping things like removing the libdef.

6. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [6],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

7. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and that they navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/1061130
[3]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[4]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[5]: https://reactnative.dev/docs/0.62/pushnotificationios
[6]: react-native-push-notification/ios#24 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 17, 2021
wix/react-native-notifications doesn't seem to be maintained [1].
expo-notifications also works on both platforms, but we've decided
it's not for us [2].

But we don't even use react-native-notifications in a cross-platform
way; we uprooted the last of the linking logic on Android in
01b33ad. It also seems non-optimal to have two different libraries
doing work to support push notifications on iOS. See also
discussion [3].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   wix/react-native-notifications [4] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [5] from RN
   v0.62 to complete our setup. We hadn't yet done everything in
   these instructions, whether because we didn't need that
   functionality before, or it wasn't available in earlier RN
   versions. We did some of this setup in the previous commit.

3. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

4. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

5. Do various housekeeping things like removing the libdef.

6. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [6],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

7. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and that they navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/1061130
[3]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[4]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[5]: https://reactnative.dev/docs/0.62/pushnotificationios
[6]: react-native-push-notification/ios#24 (comment)
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 18, 2021
wix/react-native-notifications doesn't seem to be maintained [1].
expo-notifications also works on both platforms, but we've decided
it's not for us [2].

But we don't even use react-native-notifications in a cross-platform
way; we uprooted the last of the linking logic on Android in
01b33ad. It also seems non-optimal to have two different libraries
doing work to support push notifications on iOS. See also
discussion [3].

So, let PushNotificationIOS from react-native take responsibility
for what this library has been doing. Then, since *that's*
deprecated, an upcoming commit will have us using that same code but
from react-native-community, where it's maintained.

These were my steps:

1. Use the setup instructions for our version of
   wix/react-native-notifications [4] to tear it down.

2. Use the setup instructions for `PushNotificationIOS` [5] from RN
   v0.62 to complete our setup. We hadn't yet done everything in
   these instructions, whether because we didn't need that
   functionality before, or it wasn't available in earlier RN
   versions. We did some of this setup in the previous commit.

3. Make tiny, NFC adjustments, mostly to indentation, to smooth the
   transition to the react-native-community module.

4. Change the call sites to use PushNotificationIOS, and update some
   types and comments. One part that stands out is the removal of
   the "consumeBackgroundQueue" hack from c0e2233. Nothing further
   is necessary, it just works. :)

5. Do various housekeeping things like removing the libdef.

6. On iOS, test that notifications still appear in the closed and
   background states and that, from either state, they navigate to
   the corresponding narrow. All works as expected, with one
   "gotcha": from a cold start, in debug mode, sometimes
   notifications don't navigate. There's an open issue for this [6],
   and it seems it doesn't affect release builds. In debug mode, I
   was able to solve it by disabling "Debug JS Remotely", following
   one comment there. In any case, `getInitialNotification` is what
   we've already been using PushNotificationIOS for, for a long
   time, so this hiccup is not new.

7. On Android, test that notifications appear (regardless of
   closed/background/foreground state) and that they navigate to the
   corresponding narrow.

[1]: wix/react-native-notifications#519 (comment)
[2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/1061130
[3]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.20push.20notifications/near/825122
[4]: https://github.com/wix/react-native-notifications/blob/882775fb5/docs/installation.md
[5]: https://reactnative.dev/docs/0.62/pushnotificationios
[6]: react-native-push-notification/ios#24 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants