-
Notifications
You must be signed in to change notification settings - Fork 5k
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][Android] Fix FCMv1 icons and NPE #29204
[expo-notifications][Android] Fix FCMv1 icons and NPE #29204
Conversation
The Pull Request introduced fingerprint changes against the base commit: 2838b79 Fingerprint diff[
{
"type": "file",
"filePath": "../../packages/expo-notifications/plugin/build/withNotificationsAndroid.js",
"reasons": [
"expoConfigPlugins"
],
"hash": "7631970262a7128665478596c766d12b49b1da5a"
},
{
"type": "dir",
"filePath": "../../packages/expo-notifications/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "ace590ae951e514059650b7d06b90f1fd27f54fe"
}
] Generated by PR labeler 🤖 |
- Fixes #24844 - Corrects an NPE that can happen if a deep link in the app is opened - Change constants in the `expo-notifications` plugin to the correct ones expected for FCMv1 - Add the recommended code to guard against NPE when logging Android lifecycle events - CI should pass - Test app should show the correct icon and color in notification banners <!-- Please check the appropriate items below if they apply to your diff. This is required for changes to Expo modules. --> - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin).
Published in expo-notifications 0.28.5 |
@douglowder, @christopherwalter, Although I'm not a Java developer, I am curious about how using |
@mlukasik-dev thanks for the report. I will get the NPE fixed in the next two days. Sorry for my confusion about that API. |
# Why After the fixes in #29204 , there are still reports of NPEs happening in the notification lifecycle listener methods. These NPEs are coming from code that causes Android to log events. # How Remove the logging code that was causing the issue. # Test Plan - CI should pass - Test app should work, show correct icons in notifications, and responses should show up even if app is killed # Checklist - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin).
After the fixes in #29204 , there are still reports of NPEs happening in the notification lifecycle listener methods. These NPEs are coming from code that causes Android to log events. Remove the logging code that was causing the issue. - CI should pass - Test app should work, show correct icons in notifications, and responses should show up even if app is killed - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin).
@mlukasik-dev NPE is fixed in #29370 and published in |
That was quick. Thank you very much! I can confirm that the issue has been resolved and deep links are working correctly on Android. |
- Fixes #24844 - Corrects an NPE that can happen if a deep link in the app is opened - Change constants in the `expo-notifications` plugin to the correct ones expected for FCMv1 - Add the recommended code to guard against NPE when logging Android lifecycle events - CI should pass - Test app should show the correct icon and color in notification banners <!-- Please check the appropriate items below if they apply to your diff. This is required for changes to Expo modules. --> - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin).
After the fixes in #29204 , there are still reports of NPEs happening in the notification lifecycle listener methods. These NPEs are coming from code that causes Android to log events. Remove the logging code that was causing the issue. - CI should pass - Test app should work, show correct icons in notifications, and responses should show up even if app is killed - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin).
Why
How
expo-notifications
plugin to the correct ones expected for FCMv1Test Plan
Checklist
npx expo prebuild
& EAS Build (eg: updated a module plugin).