-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
expo-notifications FCM V1 Migration Tracking Issue #28656
Comments
Something else I noticed that might be worth noting is that notification images (not icons) don't seem to be working when the app is in the foreground. Edit: Just to clarify, I tested this by sending the request to the FCM V1 api directly |
To add on to "the notification icon is not working", on Android I'm finding that the icon displays when the app is in the foreground. But I get a blank circle as an icon when the app is in the background This is happening with a development build |
No issue with the icon when we add the correct name/value pair on the Android manifest but it seems there is no "plugin" on the package expo-notification or this one is not working as expected. But this issue can be fixed "simply" by adding a manual plugin specified here #24844 (comment) in addition to the |
Hi, I am using Expo SDK 49. |
For me, the plugin here is unnecessary because the AndroidManifest that's created seems fine. I have 2 notification_icon additions. |
I have the same issue on IOS. |
PR #28883 has been submitted to fix the Android notification response issues when app is in background or not running. |
Awesome. thank you so much for tracking this down for all of us. Any idea when it will be available? |
Second that! Really need it :) |
0.28.2 didn't fix anything :( |
In my case I can confirm that with the new version, |
@fdelu see the fix that was just added in #28883 , specifically the new method in That is the method that takes the bundle passed into If you are missing part of the payload, you should have a look to see if there are parts of the bundle that are not being mapped by |
@fdelu in looking at this further, it may be that you rebuilt your Android app with the new expo-notifications package, but did not rebuild your JS bundle to pick up the required changes on the JS side: https://github.com/expo/expo/pull/28883/files#diff-37b24aeb2e0d5d7c4365c987c51f03db4b6d20a0927f3eefe0d9e0a35b2c4df7 The |
Yes, that seems like it. I noticed later that I had "dataString" but not "data" so I'm probably missing that. Not sure how that happened though. Thanks for your help! |
After updating to expo-notifications@0.28.2 and building, this works 🎉. Thanks, @douglowder. Great work! 👏 ... the only minor issue is that the notification icon is not displaying correctly with FCM V1. |
An update on this @douglowder: I verified and it is using the latest version. When my app is in the foreground, I get a notificationd and click on it, I see the "response received" log and everything works fine. However, when my app is closed (fully), I get a notification and click on it to open the app, then I don't see that "response received" log, the "data" field is missing and the request content contains the "dataString" field. Looking at the code for Another issue I've been having is that if I get a notification when the app is open, I fully close it and then I reopen the app by clicking the notification, const notification = useLastNotificationResponse();
console.log("LAST RESPONSE", notification);
console.log(
"LAST RESPONSE CONTENT",
notification?.notification?.request?.content
);
console.log(
"LAST RESPONSE TRIGGER",
notification?.notification?.request?.trigger
); The logs:
|
@fdelu yes indeed that is a bug! I think cutting and pasting the new code from NotificationEmitter should fix it... I really appreciate you taking the time to track that down. |
Hi folks, if you're in this thread due to icon-related issues (@deivijt @zandvakiliramin @haplionheart): Can you try using a config plugin like in this comment to ensure the correct fields are in your manifest? And let me know if that changes anything? @krazykriskomar re: your comment here, I wasn't sure if you have it working or not? Can you confirm if icon behavior is correct for you when sending notifs using FCM V1? |
Has anyone else experienced crashes on Android when opening app links (deep links) after migrating to Expo 51? The error message is: |
I see the issue was updated to mention cherry-picking changes into SDK 51. Is there a timeline when they'll be pushed to SDK50 as well? |
@jludwiggreenaction we are going to prioritize getting everything working on SDK 51, and after that we will start backporting these changes to SDK 50 and do testing there. |
new release today? 👀 |
I came across this issue while migrating our app's push notifications to FCM v1. only android
sdk 51 |
Not sure if it was supposed to fix the issues, but just reporting here:
|
I can confirm that getLastNotificationResponseAsync() still doesn't have any data property in the payload. This is after upgrading to 0.28.4. |
Upgrading to expo-notifications 0.28.4 resolved the push notifications issue, but the problem with opening app links persists. This isn't a configuration issue, as everything functioned correctly before upgrading to Expo 51. Here is the call stack that indicates the issue is related to the notifications module:
|
@mlukasik-dev that does indeed appear related to the recent changes to fix notification responses. Could you file a new issue (with a repro) and link it here? |
The fix for #28938 is now published in |
@douglowder with 0.28.4, it now requires I keep the withDrawableAssets plugin hack in or it errors out complaining that it can't find the asset for the icon. Is it expected that I leave the plugin in? |
@krazykriskomar I'll have a look. I didn't need that plugin for my test app, but it could be that my app isn't configured correctly to show that icon issue. @mlukasik-dev I believe I have a fix for the NPE you are seeing with deep links. Could you try the following patch? diff --git a/node_modules/expo-notifications/android/src/main/java/expo/modules/notifications/service/delegates/ExpoNotificationLifecycleListener.java b/node_modules/expo-notifications/android/src/main/java/expo/modules/notifications/service/delegates/ExpoNotificationLifecycleListener.java
index 3ae6481..a03e70d 100644
--- a/node_modules/expo-notifications/android/src/main/java/expo/modules/notifications/service/delegates/ExpoNotificationLifecycleListener.java
+++ b/node_modules/expo-notifications/android/src/main/java/expo/modules/notifications/service/delegates/ExpoNotificationLifecycleListener.java
@@ -11,6 +11,8 @@ import android.util.Log;
import androidx.core.app.NotificationCompat;
+import java.util.Objects;
+
import expo.modules.core.interfaces.ReactActivityLifecycleListener;
import expo.modules.notifications.notifications.NotificationManager;
import expo.modules.notifications.notifications.model.Notification;
@@ -70,7 +72,10 @@ public class ExpoNotificationLifecycleListener implements ReactActivityLifecycle
Log.d("ExpoNotificationLifecycleListener", method + " : keys count = " + extra.keySet().size());
for (String key : extra.keySet()) {
- Log.d("ExpoNotificationLifecycleListener", method + " : key = " + key + " = " + extra.get(key).toString());
+ Log.d(
+ "ExpoNotificationLifecycleListener",
+ method + " : key = " + key + " = " + Objects.requireNonNull(extra.get(key)).toString()
+ );
}
}
} |
@douglowder just so you have the error; if I don't have the withDrawableAssets.js plugin set up, I get this: |
@krazykriskomar I'm testing a fix that patches expo-notifications so that no additional plugin is needed. See #24844 (comment) |
Unfortunately, I can confirm that issue with no data in notification is still the case. Only for Android, only if app is killed. Works well if app is only on background. |
@kubmir did you also upgrade to expo-notifications 0.28.4, and rebuild your JS bundle? |
@douglowder I can confirm the same issue that @kubmir is having. I've done a full rebuild on 0.28.4. |
@kubmir @mikerogerz thanks I'll take a look, and post here once I figure out what the problem is. |
@kubmir @mikerogerz I did a rebuild of my test app with 0.28.4 plus the icon patch, and I do indeed see the expected data in the result of useLastNotificationResponse, even when the app is killed before opening the notification. I also see the icon in the notification banners. So I'm no longer seeing the issue that you reported above. I'm going to go ahead and create a PR with the fixes that I have, get that merged and published, and see what issues are remaining after that. |
PR created: #29204 |
I apologize, but I haven't had enough time to create a repro repo and test the patch yet. Do you still need me to create an issue, or is everything clear now? |
I would like to confirm that in the new version when the app is not killed the routing works now however when the app is killed it doesn't work. Also when notifications are sent from Firebase the data is still null in every case. Expo SDK: 51.0.9 |
I'm really sorry for bothering you, but I just want to clarify: will this PR be available in production on June 3rd? I need to decide whether to wait a few hours before publishing a new release of my app or to proceed with that release without the fix and release another update with the fix later. Thank you for your help! |
@douglowder useLastNotificationResponse has the same way of using the other Receiver ? i don't recall seeing it in the documentation |
My results with fresh build and JS bundle.
Push message sent with
|
App State | Sound | Banner | Icon | Pressed notification data |
---|---|---|---|---|
Foreground | ✅ | ✅ | ✅ | ✅ |
Background | ✅ | ✅ | ✅ | ✅ |
Killed | ✅ | ✅ | ✅ | Press launches app but notification request.content = { "body": null, "dataString": null, "title": null } |
Push message being sent with useFcmV1: true
:
App State | Sound | Banner | Icon | Pressed notification data |
---|---|---|---|---|
Foreground | ✅ | ✅ | ✅ | ✅ |
Background | ✅ | ❌ | ❌ | ✅ |
Killed | ✅ | ❌ | ❌ | ✅ |
So basically:
- Using
useFcmV1: false
- the results where unchanged with the new expo-notifications. - Using
useFcmV1: true
- the new expo-notifications resolved the missingrequest.content.data
, but there are still issues with the alert banner and icon when using the FcmV1 API.
I'm assuming the focus is just on getting things working properly with FcmV1: true
given the deadline.
Experiencing the same exact behaviour that @Codelica reported on Android 14
|
I'll also join in with out experiences. We are not using the
We are using the Interestingly enough is that in those cases, according to our logs, the whole {
"actionIdentifier": "expo.modules.notifications.actions.DEFAULT",
"notification": {
"date": 0,
"request": {
"identifier": null,
"content": {
"body": null,
"dataString": null,
"title": null
},
"trigger": {
"type": "push",
"channelId": null
}
}
}
} For the push notification banner, we are always able to see the app icon, notification title and body. If I can assist you with any logs, let me know. |
Icon is fixed for FCMv1 in #29204 . Fix is published in |
Similar to @pganster , when using Here is a push notification object I received:
I am running the following versions:
|
@douglowder, just letting you know our Android icon issue is totally fixed!!! Thank you for your dedication to this issue! |
This is a tracking issue for user-reported issues related to the Spring 2024 migration of Expo Push Notifications from FCM Legacy to FCM V1 for Android devices. There have been reported instances of brokenness in production when moving from FCM Legacy to FCM V1, and this issue will serve as a central place to track our investigation and resolution of these issues.
The known issues are as follows:
data
payload in client when receiving push notificationsexpo-notifications@0.28.4
@douglowder is working on a fix that incorporates the changes in this config plugin. We expect a PR by EOW (5/31) but need to confirm working behavior, so aim for fix in production by 6/3.Fixed in [expo-notifications][Android] Fix FCMv1 icons and NPE #29204, and fix is published inexpo-notifications@0.28.5
useLastNotificationResponse()
has null dataNew issues filed in expo/expo will be merged into this tracking issue for easier management. The deadline for migrating to FCM V1 is June 20, and we are committed to having a solution to these issues before end of May at the latest.
The text was updated successfully, but these errors were encountered: