Skip to content

Commit 782c7f6

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 7a622ac commit 782c7f6

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
@@ -270,8 +270,6 @@ PODS:
270270
- React
271271
- react-native-netinfo (5.9.5):
272272
- React
273-
- react-native-notifications (1.5.0):
274-
- React
275273
- react-native-photo-view (1.5.2):
276274
- React
277275
- react-native-safari-view (1.0.0):
@@ -443,7 +441,6 @@ DEPENDENCIES:
443441
- "react-native-cameraroll (from `../node_modules/@react-native-community/cameraroll`)"
444442
- react-native-image-picker (from `../node_modules/react-native-image-picker`)
445443
- "react-native-netinfo (from `../node_modules/@react-native-community/netinfo`)"
446-
- react-native-notifications (from `../node_modules/react-native-notifications`)
447444
- react-native-photo-view (from `../node_modules/react-native-photo-view`)
448445
- react-native-safari-view (from `../node_modules/react-native-safari-view`)
449446
- react-native-safe-area (from `../node_modules/react-native-safe-area`)
@@ -554,8 +551,6 @@ EXTERNAL SOURCES:
554551
:path: "../node_modules/react-native-image-picker"
555552
react-native-netinfo:
556553
:path: "../node_modules/@react-native-community/netinfo"
557-
react-native-notifications:
558-
:path: "../node_modules/react-native-notifications"
559554
react-native-photo-view:
560555
:path: "../node_modules/react-native-photo-view"
561556
react-native-safari-view:
@@ -674,7 +669,6 @@ SPEC CHECKSUMS:
674669
react-native-cameraroll: 06e60780a4e6e7bb9a588eca72506744cf6e133b
675670
react-native-image-picker: 3d3f85baabca60a00b75fb8facc1376db7bbdafa
676671
react-native-netinfo: a53b00d949b6456913aaf507d9dba90c4008c611
677-
react-native-notifications: ce37363008fe2d6a226da4e721eace23b6ae3ad9
678672
react-native-photo-view: 63e9e61da873531f931008b545d8d10c5373ddf8
679673
react-native-safari-view: 955d7160d159241b8e9395d12d10ea0ef863dcdd
680674
react-native-safe-area: 5fce5242419932bc05656f31bc5f0716e30be0f6

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>
@@ -99,30 +99,30 @@ - (BOOL)application:(UIApplication *)application continueUserActivity:(NSUserAct
9999
// Required to register for notifications
100100
- (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings
101101
{
102-
[RNNotifications didRegisterUserNotificationSettings:notificationSettings];
102+
[RCTPushNotificationManager didRegisterUserNotificationSettings:notificationSettings];
103103
}
104104

105+
// Required for the register event.
105106
- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken
106107
{
107-
[RNNotifications didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
108+
[RCTPushNotificationManager didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
108109
}
109110

111+
// Required for the registrationError event.
110112
- (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error {
111-
[RNNotifications didFailToRegisterForRemoteNotificationsWithError:error];
113+
[RCTPushNotificationManager didFailToRegisterForRemoteNotificationsWithError:error];
112114
}
113115

114-
// Required for the notification event.
116+
// Required for the notification event. You must call the completion handler after handling the remote notification.
115117
- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo
116118
fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler
117119
{
118-
[RNNotifications didReceiveRemoteNotification:userInfo];
119120
[RCTPushNotificationManager didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler];
120121
}
121122

122123
// Required for the localNotification event.
123124
- (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification
124125
{
125-
[RNNotifications didReceiveLocalNotification:notification];
126126
[RCTPushNotificationManager didReceiveLocalNotification:notification];
127127
}
128128

package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
"react-native-document-picker": "^3.2.4",
6565
"react-native-gesture-handler": "^1.0.12",
6666
"react-native-image-picker": "^2.3.3",
67-
"react-native-notifications": "^1.2.0",
6867
"react-native-photo-view": "alwx/react-native-photo-view#c58fd6b30",
6968
"react-native-reanimated": "^1.0.0",
7069
"react-native-safari-view": "2.0.0",

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
@@ -11,11 +11,7 @@ import type { Dispatch, Orientation as OrientationT } from '../types';
1111
import { createStyleSheet } from '../styles';
1212
import { connect } from '../react-redux';
1313
import { getUnreadByHuddlesMentionsAndPMs } from '../selectors';
14-
import {
15-
handleInitialNotification,
16-
NotificationListener,
17-
notificationOnAppActive,
18-
} from '../notification';
14+
import { handleInitialNotification, NotificationListener } from '../notification';
1915
import { ShareReceivedListener, handleInitialShare } from '../sharing';
2016
import { appOnline, appOrientation, initSafeAreaInsets } from '../actions';
2117
import PresenceHeartbeat from '../presence/PresenceHeartbeat';
@@ -109,9 +105,6 @@ class AppEventHandlers extends PureComponent<Props> {
109105
if (state === 'background' && Platform.OS === 'android') {
110106
NativeModules.BadgeCountUpdaterModule.setBadgeCount(unreadCount);
111107
}
112-
if (state === 'active') {
113-
notificationOnAppActive();
114-
}
115108
};
116109

117110
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)