firebase-kotlin-sdk
firebase-kotlin-sdk copied to clipboard
GitLiveApp
Refactor Firebase JS externals using Web v9 modular SDK
At the moment, despite the SDK is packaged with Web v9 Firebase libs we are using compat modules. The goal of this PR is to get rid legacy firebase externals in favor of modular SDK.
fixes #236 fixes #241 fixes #315 fixes #316
A lot of these changes look like just formatting? Makes it tricky to review - could you split the formatting / changes into separate commits so we can review this PR?
I pulled this PR and it does indeed solve my issue https://github.com/GitLiveApp/firebase-kotlin-sdk/issues/315 . So while I haven't thoroughly looked through the code this looks good to me @nbransby
@shepeliev I've found one potential issue but i'm not sure if it's caused by your PR or was there before:
As soon as I try to use Firebase.auth I get the following error:
> Task :compileDevelopmentExecutableKotlinJs FAILED
e: Module "dev.gitlive:firebase-auth" has a reference to symbol dev.gitlive.firebase.auth/FirebaseUser.displayName.<get-displayName>|-7122534302014937473[1]. Neither the module itself nor its dependencies contain such declaration.
This could happen if the required dependency is missing in the project. Or if there is a dependency of "dev.gitlive:firebase-auth" that has adifferent version in the project than the version that "dev.gitlive:firebase-auth" was initially compiled with. Please check that the projectconfiguration is correct and has consistent versions of all required dependencies.
The list of "dev.gitlive:firebase-auth" dependencies that may lead to conflicts:
1. "kotlin" (a library with unknown version)
2. "dev.gitlive:firebase-app" (a library with unknown version)
3. "dev.gitlive:firebase-common" (a library with unknown version)
4. "org.jetbrains.kotlinx:atomicfu" (a library with unknown version)
5. "org.jetbrains.kotlinx:kotlinx-coroutines-core" (a library with unknown version)
6. "org.jetbrains.kotlinx:kotlinx-serialization-core" (a library with unknown version)
@MinmoTech thanks for the comment. The PR is not a cause for this error. The reason is the bug in IR compiler: KT-48836. You can find a workaround here.
Is this thread dead? It would really help if someone could review and approve this PR.
can you rebase and then i can have another look at it
I'll be back to it soon
@shepeliev Have you tried using signInWithPopup with GoogleAuthProvider with this implementation? For me, it throws the following error: Error (auth/argument-error)
@shepeliev Have you tried using
signInWithPopupwithGoogleAuthProviderwith this implementation? For me, it throws the following error:Error (auth/argument-error)
Oh, looks like I didn't migrate that API to externals. Thanks for highlighting. I'll try to add it this weekend.
@nbransby Could you please have a look at this? It's been a while.
Although your effort is highly appreciated, the secret to getting timely approval on PRs is to produce small ones, this is huge and makes reviewers lose the will to live!
A quick scan it looks good but also seems there's a fair few changes in there which are unrelated to the subject of the PR, can you highlight these and explain the rational?
Although your effort is highly appreciated, the secret to getting timely approval on PRs is to produce small ones, this is huge and makes reviewers lose the will to live!
Yeap, you're might right there are some more changes than it would be wanted. However, I don't think it would make sence to migrate from Firbase JS v8 to v9 module by module in separate PRs just for reducing their size.
A quick scan it looks good but also seems there's a fair few changes in there which are unrelated to the subject of the PR, can you highlight these and explain the rational?
All of those changes are related to the PR subject exepting couple of lines that fix tests in JS. I've drop a comment earlier: https://github.com/GitLiveApp/firebase-kotlin-sdk/pull/319/files#r1161516897
It's required as at the moment JS tests are broken in the master branch and test nothing at all despite all of them are green.