Skip to content
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

Add Firebase Messaging API #467

Open
davcres opened this issue Jan 30, 2024 · 5 comments
Open

Add Firebase Messaging API #467

davcres opened this issue Jan 30, 2024 · 5 comments
Labels
API coverage Request for missing APIs

Comments

@davcres
Copy link

davcres commented Jan 30, 2024

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

@davcres davcres added the API coverage Request for missing APIs label Jan 30, 2024
@nbransby
Copy link
Member

No plans yet, but PRs welcome!

@mr-kew
Copy link

mr-kew commented May 14, 2024

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
Copy link
Member

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
Copy link

mr-kew commented May 24, 2024

@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
Copy link
Member

Makes total sense, will review the PR now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API coverage Request for missing APIs
Projects
None yet
Development

No branches or pull requests

3 participants