GitLiveApp / firebase-kotlin-sdk

A Kotlin-first SDK for Firebase
https://gitliveapp.github.io/firebase-kotlin-sdk/
Apache License 2.0
1.17k stars 155 forks source link

Add Firebase Messaging API #467

Closed davcres closed 4 months ago

davcres commented 9 months ago

First of all, thanks for your awesome work. Is it planned to develop the Firebase Messaging API?

nbransby commented 9 months ago

No plans yet, but PRs welcome!

mr-kew commented 6 months ago

How about doing just the frame? Something like:

commonMain/messaging.kt

expect val Firebase.messaging: FirebaseMessaging

expect class FirebaseMessaging

androidMain/messaging.kt

actual val Firebase.messaging
    get() = FirebaseMessaging(com.google.firebase.messaging.FirebaseMessaging.getInstance())

actual class FirebaseMessaging(val android: com.google.firebase.messaging.FirebaseMessaging) {
    // no members
}

iosMain/messaging.kt

actual val Firebase.messaging
    get() = FirebaseMessaging((FIRMessaging.messaging()))

actual class FirebaseMessaging(val ios: FIRMessaging) {
    // no members
}

It would allow for unified access to the native libraries using this library with less implementation needed. Nowadays to use messaging at all you have to import native libraries separately alongside this library and manage their versions properly. It would be much cleaner & easier to just import messaging and use the native library in ios/androidMain this way through ios/android properties.

nbransby commented 5 months ago

its a start would be happy to accept a PR for this but you can also just add this in your code or just call com.google.firebase.messaging.FirebaseMessaging.getInstance() / FIRMessaging.messaging() where you need it as all the client side code would need to be separated into androidMain and iosMain anyway

mr-kew commented 5 months ago

@nbransby Yes you are pretty much correct, but the idea of this PR is not to bring messaging into commonMain. That would be very difficult anyway since both native libraries differ so much imho. I thinks it is completely fine/expected for the messaging to be accessible only in androidMain and iosMain.

The idea of this PR is to bring easier access to the messaging inside androidMain and iosMain.

Because right now I can't just call com.google.firebase.messaging.FirebaseMessaging.getInstance(). I have to add a dependency for it. I can't use firebase-kotlin-sdk library to do it because dev.gitlive:firebase-messaging does not exist. So I have to import the android firebase library like so:

sourceSets.androidMain.implementation("com.google.firebase:firebase-messaging:$version")

But now I have to figure out the version. For example com.google.firebase:firebase-messaging:24.0.0 with dev.gitlive:firebase-firestore:1.12.0 gives linking errors. So I have to somehow know to use 21.0.0 instead and manage it when I update the firebase-kotlin-sdk.

I understand that the PR might not be entirely aligned with the vision for this library (as it does not provide commonMain functionality). But I think having just an empty frame that provides same functionality as native libraries, but comes with added benefit of version managment, would still provide better experience for the users of this library.

...and as I said, it really is't that much code/effort. I already submitted the PR.

nbransby commented 5 months ago

Makes total sense, will review the PR now

BasBuijsen commented 5 months ago

@mr-kew i saw your PR and added a few much used functions that are pretty much equal between platforms. I think this will already deliver 80% of the value for FCM. Its in this PR :)