react-native-firebase icon indicating copy to clipboard operation
react-native-firebase copied to clipboard

[🔥 ] [🐛] IOS sound - incorrect TS / incorrect data structure in messaging onMessage() fn

Open dusan-dragon opened this issue 2 years ago • 0 comments

Issue

Hello,

I am using react-native-firebase/messaging for listening to push notifications on IOS and when it is received on foreground I am showing push notification via notifee.

We are using custom sounds for push notification so I need to pass sound name from message received via onMessage() to nofifee... my code looks like:

const onReceiveNotificationIOS = async (message: FirebaseMessagingTypes.RemoteMessage) => {
  const notification = message.notification

  const body = notification?.body
  const title = notification?.title
  const sound = (notification as any)?.sound || 'default'

  void notifee.displayNotification({
    id: v4(),
    body: body,
    title: title,
    ios: {
      sound: sound,
    },
  })
}

The issue is with this line const sound = (notification as any)?.sound || 'default' where I need to type cast notification to any because notification type does not contain sound field however the data are there.

I believe this is bug. We are sending sound as string with file name from our BE so it is filled into notification object directly as sound field as a string.

If we would send object from our BE instead of string then it would be appended to notification.ios.sound However in this instance types are also wrong because it says it can be string | NotificationIOSCriticalSound which I believe is also wrong because it cannot be string.

It is pretty obvious from code that it is bug: https://github.com/invertase/react-native-firebase/blob/main/packages/messaging/ios/RNFBMessaging/RNFBMessagingSerializer.m#L182 As you can see, good intention is there as the comment above that line is pointing to right location but data are written to wrong location....

So fix is actually easy, I could create PR for that but it would be breaking change - if we fix location where we store the data... Other solution is just add sound field to type as optional string and remove string as possible type from ios subfield... And once the new major release will be ready we can introduce breaking change and fix the location?

What do you think?

Project Files

Javascript

Click To Expand

package.json:

"@react-native-firebase/messaging": "14.11.0",

firebase.json for react-native-firebase v6:

# N/A

iOS

Click To Expand

ios/Podfile:

  • [ ] I'm not using Pods
  • [x] I'm using Pods and my Podfile looks like:
# N/A

AppDelegate.m:

// N/A

Android

Click To Expand

Have you converted to AndroidX?

  • [ ] my application is an AndroidX application?
  • [ ] I am using android/gradle.settings jetifier=true for Android compatibility?
  • [ ] I am using the NPM package jetifier for react-native compatibility?

android/build.gradle:

// N/A

android/app/build.gradle:

// N/A

android/settings.gradle:

// N/A

MainApplication.java:

// N/A

AndroidManifest.xml:

<!-- N/A -->

Environment

Click To Expand

react-native info output:

 OUTPUT GOES HERE
  • Platform that you're experiencing the issue on:
    • [ ] iOS
    • [ ] Android
    • [x] iOS but have not tested behavior on Android
    • [ ] Android but have not tested behavior on iOS
    • [ ] Both
  • react-native-firebase version you're using that has this issue:
    • 14.11.0
  • Firebase module(s) you're using that has the issue:
    • messaging
  • Are you using TypeScript?
    • Y & 4.6.3

dusan-dragon avatar Jul 06 '22 21:07 dusan-dragon