Refactor Firebase JS externals using Web v9 modular SDK #319

shepeliev · 2022-06-10T15:31:15Z

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

nbransby · 2022-06-12T14:38:37Z

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?

MinmoTech · 2022-06-29T15:03:54Z

I pulled this PR and it does indeed solve my issue #315 .
So while I haven't thoroughly looked through the code this looks good to me @nbransby

MinmoTech · 2022-07-02T18:42:14Z

@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)

shepeliev · 2022-07-03T12:41:01Z

@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.

MinmoTech · 2022-07-03T14:12:33Z

Sorry for the ping and thanks for the help!

shubhamsinghshubham777 · 2023-01-02T18:32:51Z

Is this thread dead? It would really help if someone could review and approve this PR.

Reedyuk · 2023-04-05T18:54:59Z

can you rebase and then i can have another look at it

shubhamsinghshubham777 · 2023-04-06T15:56:59Z

@shepeliev Have you tried using signInWithPopup with GoogleAuthProvider with this implementation? For me, it throws the following error: Error (auth/argument-error)

shepeliev · 2023-04-06T16:20:46Z

@shepeliev Have you tried using signInWithPopup with GoogleAuthProvider with 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.

Reedyuk · 2023-04-09T10:01:03Z

can you resolve the conflicts please

# Conflicts: # .github/workflows/pull_request.yml # firebase-common/src/jsMain/kotlin/dev/gitlive/firebase/externals.kt # firebase-database/src/commonTest/kotlin/dev/gitlive/firebase/database/database.kt # firebase-database/src/jsMain/kotlin/dev/gitlive/firebase/database/database.kt # firebase-firestore/src/jsMain/kotlin/dev/gitlive/firebase/firestore/firestore.kt

shepeliev · 2023-04-10T07:49:37Z

firebase-auth/src/commonTest/kotlin/dev/gitlive/firebase/auth/auth.kt

 import kotlin.random.Random
 import kotlin.test.*

 expect val emulatorHost: String
 expect val context: Any
-expect fun runTest(test: suspend () -> Unit)
+expect fun runTest(test: suspend () -> Unit): TestResult


Since we are using kotlinx-coroutines-test the result of runTest must be immediately returned from each test in JS.

…lator

shepeliev · 2023-04-10T18:56:11Z

can you resolve the conflicts please

@Reedyuk done

Reedyuk

Looks good to me, but due to the sheer amount of changes, i would expect @nbransby to give it the approval.

shubhamsinghshubham777 · 2023-04-21T04:44:20Z

@nbransby Could you please have a look at this? It's been a while.

nbransby · 2023-04-21T11:44:06Z

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?

shepeliev · 2023-04-21T17:05:41Z

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.

# Conflicts: # firebase-auth/src/jsMain/kotlin/dev/gitlive/firebase/auth/user.kt # firebase-common/src/jsMain/kotlin/dev/gitlive/firebase/externals.kt # firebase-common/src/jsMain/kotlin/dev/gitlive/firebase/externals2.kt # firebase-database/src/jsMain/kotlin/dev/gitlive/firebase/database/database.kt # firebase-firestore/src/jsMain/kotlin/dev/gitlive/firebase/firestore/firestore.kt # firebase-functions/src/commonMain/kotlin/dev/gitlive/firebase/functions/functions.kt # firebase-functions/src/jsMain/kotlin/dev/gitlive/firebase/functions/functions.kt

shepeliev changed the title ~~Refactor Firebase external using Web v9 modular SDK~~ Refactor Firebase JS externals using Web v9 modular SDK Jun 10, 2022

shepeliev added 5 commits June 12, 2022 19:58

Refactor app and database modules

21d2264

Refactor auth

ea7fa10

Refactor firestore

3846b15

Revert formatting

9e4422e

Revert formatting

bd371ed

shepeliev force-pushed the refactor-web-version-9 branch from 546c719 to bd371ed Compare June 12, 2022 16:58

shepeliev added 9 commits June 13, 2022 10:24

Refactor remote config

6d3b904

Refactor functions

99bceab

Refactor installations

322d629

Fix firestore tests

17e4ef3

Fix DataSnapshot size

fb4bd1c

Fix database tests

ab53f6f

Revert non-js changes

b6eb04b

Fix infinite test task

4ff80b2

Split externals

79e4635

shepeliev marked this pull request as ready for review June 14, 2022 06:39

shepeliev marked this pull request as draft June 14, 2022 07:10

shepeliev added 2 commits June 14, 2022 10:16

Fix Android test results path

94ef863

Increase waiting connection to database timeout

e6fccc3

shepeliev marked this pull request as ready for review June 14, 2022 07:52

Merge branch 'master' into refactor-web-version-9

1d36094

shubhamsinghshubham777 mentioned this pull request Jan 3, 2023

Firestore dependency causes Uncaught ReferenceError: $module$firebase_compat_app is not defined #346

Closed

Move externals to related modules

541ea99

shepeliev marked this pull request as draft April 6, 2023 16:21

Add externals signInWithPopup, signInWithRedirect, getRedirectResult

dd2e9d7

shepeliev marked this pull request as ready for review April 6, 2023 21:34

shepeliev added 5 commits April 9, 2023 19:49

Resolve conflicts

5bb89b8

Fix tests

d66734c

Switch firebase-database tests to karma

de2d1e2

Increase JS tests timeout

6e65b8a

shepeliev commented Apr 10, 2023

View reviewed changes

shepeliev and others added 3 commits April 10, 2023 12:10

Rename externals.kt to Unsubscribe.kt

f6d0156

Increase JS tests timeout for modules that interact with Firebase emu…

fddc5b8

…lator

Merge branch 'master' into refactor-web-version-9

cfbc082

Reedyuk requested review from Reedyuk, nbransby and michaelprichardson April 10, 2023 18:57

Reedyuk reviewed Apr 10, 2023

View reviewed changes

nbransby added 4 commits August 28, 2023 14:55

upgrade storage to js v9

be096bd

fix js tests

a7e335f

Merge branch 'master' into refactor-web-version-9

7b78af4

nbransby merged commit b29db6f into GitLiveApp:master Sep 2, 2023
1 check passed

shepeliev deleted the refactor-web-version-9 branch September 3, 2023 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Firebase JS externals using Web v9 modular SDK #319

Refactor Firebase JS externals using Web v9 modular SDK #319

shepeliev commented Jun 10, 2022 •

edited

Loading

nbransby commented Jun 12, 2022

MinmoTech commented Jun 29, 2022

MinmoTech commented Jul 2, 2022

shepeliev commented Jul 3, 2022

MinmoTech commented Jul 3, 2022

shubhamsinghshubham777 commented Jan 2, 2023

Reedyuk commented Apr 5, 2023

shubhamsinghshubham777 commented Apr 6, 2023

shepeliev commented Apr 6, 2023

Reedyuk commented Apr 9, 2023

shepeliev Apr 10, 2023 •

edited

Loading

shepeliev commented Apr 10, 2023

Reedyuk left a comment

shubhamsinghshubham777 commented Apr 21, 2023

nbransby commented Apr 21, 2023

shepeliev commented Apr 21, 2023

Refactor Firebase JS externals using Web v9 modular SDK #319

Refactor Firebase JS externals using Web v9 modular SDK #319

Conversation

shepeliev commented Jun 10, 2022 • edited Loading

nbransby commented Jun 12, 2022

MinmoTech commented Jun 29, 2022

MinmoTech commented Jul 2, 2022

shepeliev commented Jul 3, 2022

MinmoTech commented Jul 3, 2022

shubhamsinghshubham777 commented Jan 2, 2023

Reedyuk commented Apr 5, 2023

shubhamsinghshubham777 commented Apr 6, 2023

shepeliev commented Apr 6, 2023

Reedyuk commented Apr 9, 2023

shepeliev Apr 10, 2023 • edited Loading

Choose a reason for hiding this comment

shepeliev commented Apr 10, 2023

Reedyuk left a comment

Choose a reason for hiding this comment

shubhamsinghshubham777 commented Apr 21, 2023

nbransby commented Apr 21, 2023

shepeliev commented Apr 21, 2023

shepeliev commented Jun 10, 2022 •

edited

Loading

shepeliev Apr 10, 2023 •

edited

Loading