customerio / customerio-expo-plugin

MIT License
11 stars 9 forks source link

Incompatibility with expo notifications and get native token is hanging. #68

Closed deantes closed 1 year ago

deantes commented 1 year ago

Our application has the need to run both expo-notifications and Customerio rich push notifications.

We have been running into an issue over the last few days where when either requesting the device token with getDevicePushTokenAsync or customerio sending an add device token will hang. The promise neither rejects nor resolves.

After some investigation it seems that add this function to app delegate seems to be causing the issue.

//// Explicitly define remote notification delegates to ensure compatibility with some third-party libraries
- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken
{

  return [pnHandlerObj application:application deviceToken:deviceToken];

}

Do you have any suggestions?

dgsunesen commented 1 year ago

+1 on that. Experiencing the exact same issue and it’s currently blocking us from using this

euc-callum commented 1 year ago

+1 also experiencing this - we use expo-notifications to allow us to request push notification permissions as well as retrieve the device token we send to Customer.IO. We also use expo-notifications for non-marketing push notifications.

Looks like others are struggling to get Customer.io working with and without expo-notifications as well:

https://github.com/customerio/customerio-expo-plugin/issues/69

And also to access the device token without expo-notifications:

https://github.com/customerio/customerio-reactnative/issues/127

joeyhotz commented 1 year ago

+1 - it'll be blocking us from using customer.io until there's a fix

xtreem88 commented 1 year ago

Hello everyone I apologize for the issue you are facing. Can you all share what workflow you are using? Managed or Bare?

dgsunesen commented 1 year ago

Hello everyone I apologize for the issue you are facing. Can you all share what workflow you are using? Managed or Bare?

Managed here

euc-callum commented 1 year ago

Also managed.

deantes commented 1 year ago

Managed

joeyhotz commented 1 year ago

Managed here too

jbtheard commented 1 year ago

Managed workflow

deantes commented 1 year ago

Thanks for looking into this. Are you able to give an update? We need to decide if we wait for a fix or look for alternative solutions.

zdnk commented 1 year ago

Same here. The issue is that this plugin is rewriting the app delegates methods for push notifications instead of using Expos app delegate subscription that is supported for Expo modules.

zdnk commented 1 year ago

Btw I am also on managed workflow.

The issue is that CIO is rewriting the app delegate methods, but does not call super which is required so all other modules keep working OK.

This what the resulting code looks without CIO:

- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken
{
  return [super application:application didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
}

This is what it looks like with CIO:

- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken
{

  return [pnHandlerObj application:application deviceToken:deviceToken];

}

There is the super call missing. I believe that would fix everything.

xtreem88 commented 1 year ago

I apologize for the delay in response, it is currently being worked on and would be released as soon as possible

euc-callum commented 1 year ago

One option we're considering is to not utilise Customer.IO's SDKs at all and instead integrate with their API from the app to register device tokens.

Initial testing suggests that we can see all the same functionality (via utilising expo-notifications) except the use of images in iOS push notifications.

I'm hoping that this fix will release before we pull the plug on that approach - but figured it'd be good to share this information for anyone else on this thread who is also somewhat time-pressured on getting a working solution here.

zdnk commented 1 year ago

@xtreem88 any update on the progress? :)

ami-aman commented 1 year ago

The issue has been resolved and the fix is available in the latest release 1.0.0-beta.8. Please let us know if you encounter any further issues.

zdnk commented 1 year ago

I just would like to point out, that only thing you have actually fixed is the push notification registration, but the other delegate methods remain unchanged. For example if user receives or taps on push notification which is not CustomerIO notification, theres no way for handle those scenarios right now.

deantes commented 1 year ago

The fix seems to working locally, but without adding the capability manually in xcode it will return this error.

Error: Notification registration failed: "Push Notifications" capability hasn't been added to the app in current environment: no valid “aps-environment” entitlement string found for application

Even though expo-notifications add the entitlements file it does seem to register the capability.

In a managed workflow this will still be unusable at this stage.

ami-aman commented 1 year ago

@zdnk

Thank you for pointing this out. The current release only addresses the issue with receiving the device token, as per the requirements of the ticket. The other delegate methods have not been updated yet. We apologize for any confusion that may have caused. . Please note that the other delegate methods have not been updated yet, and the specific scenario you mentioned has not been tested yet. If you are experiencing any other issues, we suggest opening a new ticket to avoid any confusion. Our team will be happy to investigate further.

ami-aman commented 1 year ago

@deantes You'll need to manually add Push Notifications as a capability in Xcode, as mentioned in our documentation. Here's the link to our documentation for reference.

We are working on adding the capability to our plugin to automatically enable push notifications, so that you won't have to perform any manual steps. While we do not have a specific timeline to share at this moment, we will keep you updated on any progress or developments.

euc-callum commented 1 year ago

I've raised a ticket for the above @deantes. I believe it has wider implications on potentially interacting negatively with other plugins so I'd be wary of utilising it even with manual change suggested above.

https://github.com/customerio/customerio-expo-plugin/issues/72