[expo-notifications][expo-permissions] Move notification permission requesters to notifications package

[sjchmiela]

Why

Fixes https://github.com/unimodules/react-native-unimodules/issues/136. This is what we wanted to do anyway—move code responsible for permissions to respective modules.

How

• removed EXUserNotificationPermissionRequester as it was left only as a fallback for when expo-notifications is not installed. Now that in Expo Client expo-notifications will be installed, this class would never be used.
• moved remote notification permission requester from expo-permissions to expo-notifications, effectively removing support for calling askAsync(Permissions.NOTIFICATIONS) from expo-permission if expo-notifications is not installed

  • while moving I write a way for the requester to receive callbacks about permission being accepted or declined. I used a UIApplicationDelegate singleton module to which requesters can register to receive callbacks.

  • since new notification permission requesters now may return null inside the permission response I had to either:

    • add support for NSNull in EXPermissionsManager (it is returned by the user notifications permissions requester)
    • exclude notifications and userFacingNotifications from being saved to kernel service since we don't scope them even so (thanks @lukmccall for the tip!)

    We decided to stick to the second idea.

Test Plan

• Ran Expo Client on an ExpoClient-less device, fetched and asked for both notification-related permissions.

[github-actions]

Native Component List for this branch is ready

[lukmccall]

LGTM 🎉

[robertying]

After this update, it seems expo-notifications is conflict with expo-permissions.

I got build errors:

duplicate symbol '_OBJC_IVAR_$_EXRemoteNotificationPermissionRequester._reject' in:
    /Users/rying/Library/Developer/Xcode/DerivedData/learnX-ecqsefafkiukdcapdpyhjfgvicvl/Build/Products/Debug-iphonesimulator/EXNotifications/libEXNotifications.a(EXRemoteNotificationPermissionRequester.o)
    /Users/rying/Library/Developer/Xcode/DerivedData/learnX-ecqsefafkiukdcapdpyhjfgvicvl/Build/Products/Debug-iphonesimulator/EXPermissions/libEXPermissions.a(EXRemoteNotificationPermissionRequester.o)
duplicate symbol '_OBJC_IVAR_$_EXRemoteNotificationPermissionRequester._userNotificationPermissionRequester' in:
    /Users/rying/Library/Developer/Xcode/DerivedData/learnX-ecqsefafkiukdcapdpyhjfgvicvl/Build/Products/Debug-iphonesimulator/EXNotifications/libEXNotifications.a(EXRemoteNotificationPermissionRequester.o)
    /Users/rying/Library/Developer/Xcode/DerivedData/learnX-ecqsefafkiukdcapdpyhjfgvicvl/Build/Products/Debug-iphonesimulator/EXPermissions/libEXPermissions.a(EXRemoteNotificationPermissionRequester.o)
duplicate symbol '_OBJC_CLASS_$_EXRemoteNotificationPermissionRequester' in:
    /Users/rying/Library/Developer/Xcode/DerivedData/learnX-ecqsefafkiukdcapdpyhjfgvicvl/Build/Products/Debug-iphonesimulator/EXNotifications/libEXNotifications.a(EXRemoteNotificationPermissionRequester.o)
    /Users/rying/Library/Developer/Xcode/DerivedData/learnX-ecqsefafkiukdcapdpyhjfgvicvl/Build/Products/Debug-iphonesimulator/EXPermissions/libEXPermissions.a(EXRemoteNotificationPermissionRequester.o)

Is there a way to resolve this?

It seems expo-permissions is not updated for a while and may not reflect this change?

[sjchmiela]

Good catch! It looks like the appropriate change has been published as part of expo-permissions@9.0.0—try using 9.0.0 or a higher version to fix this problem. Sorry for it!

[robertying]

Good catch! It looks like the appropriate change has been published as part of expo-permissions@9.0.0—try using 9.0.0 or a higher version to fix this problem. Sorry for it!

😅 @sjchmiela the npm version is still at 8.1.0: https://www.npmjs.com/package/expo-permissions

[sjchmiela]

Yeah, we recently revamped our release process, so now during release we first publish packages under new version, but not the latest tag, do the final QA of the whole SDK and only once we are sure everything is all right. If you click to Versions tab you will see

8.1.0 ......... latest
9.0.1 ......... next

which is expected. Sorry for the confusion1

[robertying]

@sjchmiela nice! thanks for the explanation!

In this case, I believe it's best if expo-notifications can also follow this rule? so that it won't get messed up with others.

[sjchmiela]

👍 I totally agree with you, when I was promoting expo-notifications to latest I wasn't aware that it will cause problems and I really wanted to distribute a recent fix (#8608) to the users. 😕 It's a tough world for such a complex network of dependencies. I'll make sure not to make this mistake again!

[robertying]

👍 I totally agree with you, when I was promoting expo-notifications to latest I wasn't aware that it will cause problems and I really wanted to distribute a recent fix (#8608) to the users. 😕 It's a tough world for such a complex network of dependencies. I'll make sure not to make this mistake again!

You are already doing an awesome job!