Skip to content

Commit

Permalink
ios deps: Use PushNotificationIOS from react-native-community, not RN…
Browse files Browse the repository at this point in the history
… core.

In a recent commit, we asked RN's PushNotificationIOS to take
responsibilities that were previously taken by
wix/react-native-notifications, and removed the Wix library.

Now, we account for the fact that PushNotificationsIOS from RN is
deprecated [1] and asks us to use
@react-native-community/push-notification-ios instead. So, do.

These were my steps:

1. Use the setup instructions for `PushNotificationIOS` [1] from RN
   v0.60 to tear it down.

2. Follow the setup instructions for
   @react-native-community/push-notification-ios at the latest,
   v1.5.0 [2]. The native code closely matches what was there
   before, which makes sense. There are a few additions, and notes
   on old iOS APIs.

3. Follow the simple "Migrating..." instructions [3] that say you
   just have to change the imports; no call site changes are
   necessary.

4. Change a few comments that refer to details of the directory
   structure or implementation of the library.

5. Test thoroughly, as in the previous commit, and observe the same
   results.

[1]: https://reactnative.dev/docs/0.60/pushnotificationios
[2]: https://github.com/react-native-community/push-notification-ios/blob/v1.5.0/README.md
[3]: https://github.com/react-native-community/push-notification-ios/blob/v1.5.0/README.md#migrating-from-the-core-react-native-module

Fixes: zulip#4115
  • Loading branch information
chrisbobbe committed Nov 10, 2020
1 parent 3ce528e commit 2c077cb
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 68 deletions.
3 changes: 0 additions & 3 deletions ios/Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ target 'ZulipMobile' do
pod 'React-jsi', :path => '../node_modules/react-native/ReactCommon/jsi'
pod 'React-jsiexecutor', :path => '../node_modules/react-native/ReactCommon/jsiexecutor'
pod 'React-jsinspector', :path => '../node_modules/react-native/ReactCommon/jsinspector'
# This is deprecated upstream; removing it is #4115. See
# https://reactnative.dev/docs/pushnotificationios.
pod 'React-RCTPushNotification', :path => '../node_modules/react-native/Libraries/PushNotificationIOS'
pod 'ReactCommon/callinvoker', :path => "../node_modules/react-native/ReactCommon"
pod 'ReactCommon/turbomodule/core', :path => "../node_modules/react-native/ReactCommon"
pod 'Yoga', :path => '../node_modules/react-native/ReactCommon/yoga', :modular_headers => true
Expand Down
25 changes: 7 additions & 18 deletions ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,6 @@ PODS:
- React-jsi (= 0.62.2)
- React-jsiexecutor (= 0.62.2)
- Yoga
- React-Core/RCTPushNotificationHeaders (0.62.2):
- Folly (= 2018.10.22.00)
- glog
- React-Core/Default
- React-cxxreact (= 0.62.2)
- React-jsi (= 0.62.2)
- React-jsiexecutor (= 0.62.2)
- Yoga
- React-Core/RCTSettingsHeaders (0.62.2):
- Folly (= 2018.10.22.00)
- glog
Expand Down Expand Up @@ -316,11 +308,6 @@ PODS:
- RCTTypeSafety (= 0.62.2)
- React-Core/RCTNetworkHeaders (= 0.62.2)
- ReactCommon/turbomodule/core (= 0.62.2)
- React-RCTPushNotification (0.62.2):
- FBReactNativeSpec (= 0.62.2)
- RCTTypeSafety (= 0.62.2)
- React-Core/RCTPushNotificationHeaders (= 0.62.2)
- ReactCommon/turbomodule/core (= 0.62.2)
- React-RCTSettings (0.62.2):
- FBReactNativeSpec (= 0.62.2)
- Folly (= 2018.10.22.00)
Expand Down Expand Up @@ -353,6 +340,8 @@ PODS:
- React
- RNCMaskedView (0.1.10):
- React
- RNCPushNotificationIOS (1.7.3):
- React-Core
- RNDeviceInfo (6.0.2):
- React
- RNGestureHandler (1.8.0):
Expand Down Expand Up @@ -453,7 +442,6 @@ DEPENDENCIES:
- React-RCTImage (from `../node_modules/react-native/Libraries/Image`)
- React-RCTLinking (from `../node_modules/react-native/Libraries/LinkingIOS`)
- React-RCTNetwork (from `../node_modules/react-native/Libraries/Network`)
- React-RCTPushNotification (from `../node_modules/react-native/Libraries/PushNotificationIOS`)
- React-RCTSettings (from `../node_modules/react-native/Libraries/Settings`)
- React-RCTText (from `../node_modules/react-native/Libraries/Text`)
- React-RCTVibration (from `../node_modules/react-native/Libraries/Vibration`)
Expand All @@ -462,6 +450,7 @@ DEPENDENCIES:
- rn-fetch-blob (from `../node_modules/rn-fetch-blob`)
- "RNCAsyncStorage (from `../node_modules/@react-native-community/async-storage`)"
- "RNCMaskedView (from `../node_modules/@react-native-community/masked-view`)"
- "RNCPushNotificationIOS (from `../node_modules/@react-native-community/push-notification-ios`)"
- RNDeviceInfo (from `../node_modules/react-native-device-info`)
- RNGestureHandler (from `../node_modules/react-native-gesture-handler`)
- RNReanimated (from `../node_modules/react-native-reanimated`)
Expand Down Expand Up @@ -575,8 +564,6 @@ EXTERNAL SOURCES:
:path: "../node_modules/react-native/Libraries/LinkingIOS"
React-RCTNetwork:
:path: "../node_modules/react-native/Libraries/Network"
React-RCTPushNotification:
:path: "../node_modules/react-native/Libraries/PushNotificationIOS"
React-RCTSettings:
:path: "../node_modules/react-native/Libraries/Settings"
React-RCTText:
Expand All @@ -591,6 +578,8 @@ EXTERNAL SOURCES:
:path: "../node_modules/@react-native-community/async-storage"
RNCMaskedView:
:path: "../node_modules/@react-native-community/masked-view"
RNCPushNotificationIOS:
:path: "../node_modules/@react-native-community/push-notification-ios"
RNDeviceInfo:
:path: "../node_modules/react-native-device-info"
RNGestureHandler:
Expand Down Expand Up @@ -681,14 +670,14 @@ SPEC CHECKSUMS:
React-RCTImage: e70be9b9c74fe4e42d0005f42cace7981c994ac3
React-RCTLinking: c1b9739a88d56ecbec23b7f63650e44672ab2ad2
React-RCTNetwork: 73138b6f45e5a2768ad93f3d57873c2a18d14b44
React-RCTPushNotification: d544b6b5bf8def493b87da896a1cc0f4b61291c7
React-RCTSettings: 6e3738a87e21b39a8cb08d627e68c44acf1e325a
React-RCTText: fae545b10cfdb3d247c36c56f61a94cfd6dba41d
React-RCTVibration: 4356114dbcba4ce66991096e51a66e61eda51256
ReactCommon: ed4e11d27609d571e7eee8b65548efc191116eb3
rn-fetch-blob: f525a73a78df9ed5d35e67ea65e79d53c15255bc
RNCAsyncStorage: 3c304d1adfaea02ec732ac218801cb13897aa8c0
RNCMaskedView: 5a8ec07677aa885546a0d98da336457e2bea557f
RNCPushNotificationIOS: edeeea93b7210d2f918fc0e46e1a2a189b5fdacc
RNDeviceInfo: bdd61e8b070d13a1dd9d022091981075ed4cde16
RNGestureHandler: 7a5833d0f788dbd107fbb913e09aa0c1ff333c39
RNReanimated: 89f5e0a04d1dd52fbf27e7e7030d8f80a646a3fc
Expand All @@ -713,6 +702,6 @@ SPEC CHECKSUMS:
Yoga: 3ebccbdd559724312790e7742142d062476b698e
YogaKit: f782866e155069a2cca2517aafea43200b01fd5a

PODFILE CHECKSUM: 225a865018bba07be314e8afa4e91319ab40adac
PODFILE CHECKSUM: bb21f5f2cffae3c57e18eab3696b63eff8e52585

COCOAPODS: 1.9.3
1 change: 0 additions & 1 deletion ios/ZulipMobile-Bridging-Header.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@
#import <React/RCTConvert.h>
#import <React/RCTEventEmitter.h>
#import <React/RCTBundleURLProvider.h>
#import <React/RCTPushNotificationManager.h>
3 changes: 2 additions & 1 deletion ios/ZulipMobile/AppDelegate.h
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#import <React/RCTBridgeDelegate.h>
#import <UIKit/UIKit.h>
#import <UMReactNativeAdapter/UMModuleRegistryAdapter.h>
#import <UserNotifications/UNUserNotificationCenter.h>

@interface AppDelegate : UIResponder <UIApplicationDelegate, RCTBridgeDelegate>
@interface AppDelegate : UIResponder <UIApplicationDelegate, RCTBridgeDelegate, UNUserNotificationCenterDelegate>

@property (nonatomic, strong) UMModuleRegistryAdapter *moduleRegistryAdapter;
@property (nonatomic, strong) UIWindow *window;
Expand Down
25 changes: 17 additions & 8 deletions ios/ZulipMobile/AppDelegate.m
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#import <asl.h>
#import <React/RCTLog.h>
#import <UserNotifications/UserNotifications.h>
#import <React/RCTPushNotificationManager.h>
#import <RNCPushNotificationIOS.h>
#import <UMCore/UMModuleRegistry.h>
#import <UMReactNativeAdapter/UMNativeModulesProxy.h>
#import <UMReactNativeAdapter/UMModuleRegistryAdapter.h>
Expand Down Expand Up @@ -99,34 +99,43 @@ - (BOOL)application:(UIApplication *)application continueUserActivity:(NSUserAct
// Required to register for notifications
- (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings
{
[RCTPushNotificationManager didRegisterUserNotificationSettings:notificationSettings];
[RNCPushNotificationIOS didRegisterUserNotificationSettings:notificationSettings];
}

// Required for the register event.
- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken
{
[RCTPushNotificationManager didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
[RNCPushNotificationIOS didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
}

// Required for the registrationError event.
- (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error {
[RCTPushNotificationManager didFailToRegisterForRemoteNotificationsWithError:error];
[RNCPushNotificationIOS didFailToRegisterForRemoteNotificationsWithError:error];
}

// Required for the notification event. You must call the completion handler after handling the remote notification.
- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo
fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler
{
[RCTPushNotificationManager didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler];
[RNCPushNotificationIOS didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler];
}

// Required for the localNotification event.
// iOS 10+ Required for localNotification event
- (void)userNotificationCenter:(UNUserNotificationCenter *)center
didReceiveNotificationResponse:(UNNotificationResponse *)response
withCompletionHandler:(void (^)(void))completionHandler
{
[RNCPushNotificationIOS didReceiveNotificationResponse:response];
completionHandler();
}

// iOS 4-10 Required for the localNotification event.
- (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification
{
[RCTPushNotificationManager didReceiveLocalNotification:notification];
[RNCPushNotificationIOS didReceiveLocalNotification:notification];
}

// Called when a notification is delivered to a foreground app.
// iOS 10.0+: Called when a notification is delivered to a foreground app.
-(void)userNotificationCenter:(UNUserNotificationCenter *)center
willPresentNotification:(UNNotification *)notification
withCompletionHandler:(void (^)(UNNotificationPresentationOptions options))completionHandler
Expand Down
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const transformModulesWhitelist = [
// @rnc/async-storage itself is precompiled, but its mock-helper is not
'@react-native-community/async-storage',
'@react-native-community/cameraroll',
'@react-native-community/push-notification-ios',
'@expo/react-native-action-sheet',
'react-navigation',
'@sentry/react-native',
Expand Down
12 changes: 12 additions & 0 deletions jest/jestSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,18 @@ jest.mock('react-native-reanimated', () => {

jest.mock('@react-native-community/async-storage', () => mockAsyncStorage);

// Without this, we get lots of these errors on importing the module:
// `Invariant Violation: Native module cannot be null.`
jest.mock('@react-native-community/push-notification-ios', () => ({
presentLocalNotification: jest.fn(),
scheduleLocalNotification: jest.fn(),
cancelAllLocalNotifications: jest.fn(),
removeAllDeliveredNotifications: jest.fn(),
getDeliveredNotifications: jest.fn(),
removeDeliveredNotifications: jest.fn(),
// etc. (incomplete)
}));

jest.mock('react-native-sound', () => () => ({
play: jest.fn(),
}));
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"@react-native-community/cameraroll": "^1.7.2",
"@react-native-community/masked-view": "^0.1.10",
"@react-native-community/netinfo": "^5.9.5",
"@react-native-community/push-notification-ios": "^1.5.0",
"@sentry/react-native": "^1.0.9",
"@unimodules/core": "~5.3.0",
"@zulip/shared": "^0.0.2",
Expand Down
68 changes: 31 additions & 37 deletions src/notification/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* @flow strict-local */
import { DeviceEventEmitter, NativeModules, Platform, PushNotificationIOS } from 'react-native';
import type { PushNotificationEventName } from 'react-native/Libraries/PushNotificationIOS/PushNotificationIOS';
import { DeviceEventEmitter, NativeModules, Platform } from 'react-native';
import PushNotificationIOS from '@react-native-community/push-notification-ios';
import type { PushNotificationEventName } from '@react-native-community/push-notification-ios';

import type { Notification } from './types';
import type { Auth, Dispatch, Identity, Narrow, User } from '../types';
Expand Down Expand Up @@ -122,8 +123,8 @@ const getInitialNotification = async (): Promise<Notification | null> => {
}

// This is actually typed as ?Object (and so effectively `any`); but if
// present, it must be a JSONable dictionary. (See PushNotificationIOS.js and
// RCTPushNotificationManager.m in Libraries/PushNotificationIOS.)
// present, it must be a JSONable dictionary. (See js/index.js and
// ios/RNCPushNotificationIOS.m in @rnc/push-notification-ios.)
const data: ?JSONableDict = notification.getData();
if (!data) {
return null;
Expand Down Expand Up @@ -172,7 +173,7 @@ export class NotificationListener {
// 'register' -> 'remoteNotificationsRegistered'
// 'registrationError' -> 'remoteNotificationRegistrationError'
PushNotificationIOS.addEventListener(name, handler);
this.unsubs.push(() => PushNotificationIOS.removeEventListener(name, handler));
this.unsubs.push(() => PushNotificationIOS.removeEventListener(name));
}

/** Private. */
Expand Down Expand Up @@ -267,53 +268,46 @@ export class NotificationListener {
export const getNotificationToken = () => {
if (Platform.OS === 'ios') {
// This leads to a call in RNCPushNotificationIOS to this, to
// maybe prompt the user to grant permissions:
// https://developer.apple.com/documentation/uikit/uiapplication/1622932-registerusernotificationsettings?language=objc
// (deprecated after iOS 10, yikes!).
// maybe prompt the user to grant authorization, with options for
// what things to authorize (badge, sound, alert, etc.):
// https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/1649527-requestauthorizationwithoptions
//
// (Then, in case we're interested, the library ensures that the
// Promise we get will resolve with the user's notification
// settings for the app: whether they've enabled alerts, the badge
// count, and sound.)
// If authorization is granted, the library calls this, to have the
// application register for remote notifications:
// https://developer.apple.com/documentation/appkit/nsapplication/2967172-registerforremotenotifications?language=occ
//
// The above-mentioned `registerUserNotificationSettings` function
// reports the permissions-request result to the app;
// `AppDelegate`'s `didRegisterUserNotificationSettings` method
// gets called (it's also deprecated):
// https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1623022-application?language=objc
// Following the library's setup instructions, we've asked that
// method to hand control back to the library.
// (Then, in case we're interested, the library calls
// https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/1649524-getnotificationsettingswithcompl
// and sets the eventual result to be the resolution of the
// Promise we get, so we can know if the user has enabled
// alerts, the badge count, and sound.)
//
// If authorization is granted, the library calls this (not
// deprecated), to have the application register for remote
// notifications:
// https://developer.apple.com/documentation/appkit/nsapplication/2967172-registerforremotenotifications?language=occ
// That function ends up sending the app a device token; the app
// receives it in our own code: `AppDelegate`'s
// The above-mentioned `registerForRemoteNotifications` function
// ends up sending the app a device token; the app receives it in
// our own code: `AppDelegate`'s
// `didRegisterForRemoteNotificationsWithDeviceToken` method:
// https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622958-application?language=objc
// (not deprecated). Following the library's setup instructions,
// we've asked that method to hand control back to the library.
// https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622958-application?language=objc.
// Following the library's setup instructions, we've asked that
// method to hand control back to the library.
//
// It looks like the library then creates a notification, with the
// magic-string name "RemoteNotificationsRegistered", using
// https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415812-postnotificationname?language=objc
// (not deprecated).
// https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415812-postnotificationname?language=objc.
// It listens for this notification with
// https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415360-addobserver
// (not deprecated) and, upon receipt, sends a React Native event
// to the JavaScript part of the library. We can listen to this
// event, with `PushNotificationIOS.addEventListener`, under the
// alias 'register'. (We can also listen for registration failure
// under the alias 'registrationError'.)
// and, upon receipt, sends a React Native event to the JavaScript
// part of the library. We can listen to this event, with
// `PushNotificationIOS.addEventListener`, under the alias
// 'register'. (We can also listen for registration failure under
// the alias 'registrationError'.)
//
// In short, this kicks off a sequence:
// permissions / register settings ->
// authorization, with options ->
// register for remote notifications ->
// send event we already have a global listener for
//
// This satisfies the stern warnings in Apple's docs (at the above
// links) to request permissions before registering with the push
// links) to request authorization before registering with the push
// notification service.
PushNotificationIOS.requestPermissions();
} else {
Expand Down
7 changes: 7 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2200,6 +2200,13 @@
resolved "https://registry.yarnpkg.com/@react-native-community/netinfo/-/netinfo-5.9.5.tgz#3bad0d855d2e813be085ec305139d4175c512ccc"
integrity sha512-PbSsRmhRwYIMdeVJTf9gJtvW0TVq/hmgz1xyjsrTIsQ7QS7wbMEiv1Eb/M/y6AEEsdUped5Axm5xykq9TGISHg==

"@react-native-community/push-notification-ios@^1.5.0":
version "1.7.3"
resolved "https://registry.yarnpkg.com/@react-native-community/push-notification-ios/-/push-notification-ios-1.7.3.tgz#7b41add329996df59382820d8a729fe609c2fb75"
integrity sha512-SLGQMxSB4WTvATjCXELxansnseLcmqJ6jIC8U4AyjxL30k3m1YmbrO4wGMw/ZF/VSC1toK+a39aD+ozDjon3mw==
dependencies:
invariant "^2.2.4"

"@react-navigation/core@^3.7.6":
version "3.7.6"
resolved "https://registry.yarnpkg.com/@react-navigation/core/-/core-3.7.6.tgz#e0244fcdc22937825b252197f70308bbe5709c58"
Expand Down

0 comments on commit 2c077cb

Please sign in to comment.