Skip to content

Commit eb3fc1d

Browse files
committed
ios notifications: Stop using wix/react-native-notifications.
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)
1 parent 11ef3fe commit eb3fc1d

File tree

11 files changed

+101
-158
lines changed

11 files changed

+101
-158
lines changed

.flowconfig

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ node_modules/warning/.*
2626
# resolved, or write our own libdef.
2727
.*/node_modules/react-native-image-picker/.*
2828

29-
.*/node_modules/react-native-notifications/.*
3029
.*/node_modules/react-native-tab-view/.*
3130
.*/node_modules/react-native-safe-area/.*
3231
.*/node_modules/flow-coverage-report/.*

flow-typed/npm/react-native-notifications_vx.x.x.js

Lines changed: 0 additions & 53 deletions
This file was deleted.

ios/Podfile.lock

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,6 @@ PODS:
273273
- React-Core
274274
- react-native-netinfo (5.9.10):
275275
- React-Core
276-
- react-native-notifications (1.5.0):
277-
- React
278276
- react-native-photo-view (1.5.2):
279277
- React
280278
- react-native-safe-area-context (3.2.0):
@@ -446,7 +444,6 @@ DEPENDENCIES:
446444
- "react-native-cameraroll (from `../node_modules/@react-native-community/cameraroll`)"
447445
- react-native-image-picker (from `../node_modules/react-native-image-picker`)
448446
- "react-native-netinfo (from `../node_modules/@react-native-community/netinfo`)"
449-
- react-native-notifications (from `../node_modules/react-native-notifications`)
450447
- react-native-photo-view (from `../node_modules/react-native-photo-view`)
451448
- react-native-safe-area-context (from `../node_modules/react-native-safe-area-context`)
452449
- react-native-simple-toast (from `../node_modules/react-native-simple-toast`)
@@ -558,8 +555,6 @@ EXTERNAL SOURCES:
558555
:path: "../node_modules/react-native-image-picker"
559556
react-native-netinfo:
560557
:path: "../node_modules/@react-native-community/netinfo"
561-
react-native-notifications:
562-
:path: "../node_modules/react-native-notifications"
563558
react-native-photo-view:
564559
:path: "../node_modules/react-native-photo-view"
565560
react-native-safe-area-context:
@@ -676,7 +671,6 @@ SPEC CHECKSUMS:
676671
react-native-cameraroll: 88f4e62d9ecd0e1f253abe4f685474f2ea14bfa2
677672
react-native-image-picker: c6d75c4ab2cf46f9289f341242b219cb3c1180d3
678673
react-native-netinfo: 30fb89fa913c342be82a887b56e96be6d71201dd
679-
react-native-notifications: ce37363008fe2d6a226da4e721eace23b6ae3ad9
680674
react-native-photo-view: 63e9e61da873531f931008b545d8d10c5373ddf8
681675
react-native-safe-area-context: f0906bf8bc9835ac9a9d3f97e8bde2a997d8da79
682676
react-native-simple-toast: bf002828cf816775a6809f7a9ec3907509bce11f

ios/ZulipMobile-Bridging-Header.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,3 @@
77
#import <React/RCTEventEmitter.h>
88
#import <React/RCTBundleURLProvider.h>
99
#import <React/RCTPushNotificationManager.h>
10-
#import <RNNotifications.h>

ios/ZulipMobile/AppDelegate.m

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
#import <React/RCTLinkingManager.h>
77
#import <asl.h>
88
#import <React/RCTLog.h>
9-
#import <RNNotifications.h>
9+
#import <UserNotifications/UserNotifications.h>
1010
#import <React/RCTPushNotificationManager.h>
1111
#import <UMCore/UMModuleRegistry.h>
1212
#import <UMReactNativeAdapter/UMNativeModulesProxy.h>
@@ -96,30 +96,30 @@ - (BOOL)application:(UIApplication *)application
9696
// Required to register for notifications
9797
- (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings
9898
{
99-
[RNNotifications didRegisterUserNotificationSettings:notificationSettings];
99+
[RCTPushNotificationManager didRegisterUserNotificationSettings:notificationSettings];
100100
}
101101

102+
// Required for the register event.
102103
- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken
103104
{
104-
[RNNotifications didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
105+
[RCTPushNotificationManager didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
105106
}
106107

108+
// Required for the registrationError event.
107109
- (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error {
108-
[RNNotifications didFailToRegisterForRemoteNotificationsWithError:error];
110+
[RCTPushNotificationManager didFailToRegisterForRemoteNotificationsWithError:error];
109111
}
110112

111-
// Required for the notification event.
113+
// Required for the notification event. You must call the completion handler after handling the remote notification.
112114
- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo
113115
fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler
114116
{
115-
[RNNotifications didReceiveRemoteNotification:userInfo];
116117
[RCTPushNotificationManager didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler];
117118
}
118119

119120
// Required for the localNotification event.
120121
- (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification
121122
{
122-
[RNNotifications didReceiveLocalNotification:notification];
123123
[RCTPushNotificationManager didReceiveLocalNotification:notification];
124124
}
125125

package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@
6363
"react-native-document-picker": "^3.2.4",
6464
"react-native-gesture-handler": "^1.0.12",
6565
"react-native-image-picker": "~2.3.4",
66-
"react-native-notifications": "^1.2.0",
6766
"react-native-photo-view": "alwx/react-native-photo-view#c58fd6b30",
6867
"react-native-reanimated": "^1.0.0",
6968
"react-native-safe-area-context": "^3.1.7",

react-native.config.js

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,6 @@ module.exports = {
2222
ios: null,
2323
},
2424
},
25-
'react-native-notifications': {
26-
platforms: {
27-
// We don't use this Wix library in the Android build. See 01b33ad31.
28-
android: null,
29-
},
30-
},
3125
'react-native-screens': {
3226
platforms: {
3327
// We haven't enabled `react-native-screens` yet, that's

src/boot/AppEventHandlers.js

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,7 @@ import type { Dispatch, Orientation as OrientationT } from '../types';
1010
import { createStyleSheet } from '../styles';
1111
import { connect } from '../react-redux';
1212
import { getUnreadByHuddlesMentionsAndPMs } from '../selectors';
13-
import {
14-
handleInitialNotification,
15-
NotificationListener,
16-
notificationOnAppActive,
17-
} from '../notification';
13+
import { handleInitialNotification, NotificationListener } from '../notification';
1814
import { ShareReceivedListener, handleInitialShare } from '../sharing';
1915
import { appOnline, appOrientation } from '../actions';
2016
import PresenceHeartbeat from '../presence/PresenceHeartbeat';
@@ -110,9 +106,6 @@ class AppEventHandlers extends PureComponent<Props> {
110106
if (state === 'background' && Platform.OS === 'android') {
111107
NativeModules.BadgeCountUpdaterModule.setBadgeCount(unreadCount);
112108
}
113-
if (state === 'active') {
114-
notificationOnAppActive();
115-
}
116109
};
117110

118111
notificationListener = new NotificationListener(this.props.dispatch);

src/notification/extract.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,9 @@ export const fromAPNsImpl = (rawData: JSONableDict): Notification | void => {
108108
// required, with a structure defined by Apple; all other entries are
109109
// available to the application.
110110
//
111-
// The react-native-notifications library filters out `aps`, parses it, and
112-
// hands us the rest as "data". Pretty much any iOS notifications library
113-
// should do the same, but we don't rely on that.
111+
// PushNotificationsIOS filters out `aps`, parses it, and hands us the rest
112+
// as "data". Pretty much any iOS notifications library should do
113+
// the same, but we don't rely on that.
114114

115115
const data: JSONableInputDict = (() => {
116116
if ('aps' in rawData) {

0 commit comments

Comments
 (0)