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

Firebase 9 support #205

Open
alexis-mrc opened this issue Jun 20, 2021 · 42 comments · Fixed by #221
Open

Firebase 9 support #205

alexis-mrc opened this issue Jun 20, 2021 · 42 comments · Fixed by #221
Assignees
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request

Comments

@alexis-mrc
Copy link
Contributor

Firebase 9 is currently and beta and support tree shaking
Angularfire 7 (in beta) support Firebase 9

it'd be lovely to be able to us akita-ng-fire and Firebase with tree shaking (even in beta, and most of the job would be done for the official release)

Thanks

@fritzschoff
Copy link
Collaborator

Hey, thanks for the report. Could make a beta release on version 7. Firebase 9 is pretty different from version 8 right?

@alexis-mrc
Copy link
Contributor Author

the import are differents to make the tree shaking aviable

You can take a look here : https://firebase.google.com/docs/web/modular-upgrade

For exemple :
v8

import firebase from "firebase/app";
import "firebase/auth";

const auth = firebase.auth();
auth.onAuthStateChanged(user => { 
  // Check for user status
});

v9

import { getAuth, onAuthStateChanged } from "firebase/auth";

const auth = getAuth(firebaseApp);
onAuthStateChanged(auth, user => {
  // Check for user status
});

@fritzschoff fritzschoff added dependencies Pull requests that update a dependency file enhancement New feature or request labels Jun 21, 2021
@fritzschoff fritzschoff self-assigned this Jun 21, 2021
@GrandSchtroumpf
Copy link
Collaborator

@avelow akita-ng-fire is based on @angular/fire. We would update the version 9 when @angular/fire is updated too.

@alexis-mrc
Copy link
Contributor Author

Hi,

Firebase 9 has been released and @angular/fire too.
I am eager to used tree shaking :)

Do you know when you think akita-ng-fire with the new version may be aviable ?

Thanks a lot !

@GrandSchtroumpf
Copy link
Collaborator

We'll take a look at it in the coming weeks. But it's not in high priority for me right now.
You're welcome to open a PR if you want.

@alexis-mrc
Copy link
Contributor Author

I just take a look and I think and I don't use enough of akita-ng-fire to be able to update it. It's a really big update and I will just wait.
Thank you for your job on this library !

@Disane87
Copy link

Too bad this isn't working actually. Just wanted to dive into firebase with akita and now I have to downgrade manually :-(

@randallmeeker
Copy link
Collaborator

FWIW I'm happy to jump in and see if I can do this over the next couple of weeks. Just want to make sure I'm not replicating work someone else has started. If there is a fork out there that has the work started let me know. Otherwise, I will this week.

@alexis-mrc
Copy link
Contributor Author

alexis-mrc commented Sep 14, 2021

I did nothing, I wasn't confident on the big changes needed to be done.

I am really happy to read that someone will do it !

@GrandSchtroumpf
Copy link
Collaborator

@randallmeeker thanks for offering to help on this. You can fork branch v6. You can open a draft PR to open a discussion on the implementation, and I can provide add comments on the code along the way if you need help.
Please tag me on the PR so I can get the notification :).

@randallmeeker
Copy link
Collaborator

Thanks, I'm going to get started on this today (a lil late). Does the test suite work out of the box?

@alexis-mrc
Copy link
Contributor Author

Thanks for the update

I saw that the update was done using the "compat" library

On https://firebase.google.com/docs/web/modular-upgrade, Firebase Team say :
About the compat libraries
There are two types of libraries available for Firebase Web SDK version 9:

Modular - a new API surface designed to facilitate tree-shaking (removal of unused code) to make your web app as small and fast as possible.
Compat - a familiar API surface which is fully compatible with the version 8 SDK, allowing you to upgrade to version 9 without changing all of your Firebase code at once. Compat libraries have little to no size or performance advantages over their version 8 counterparts.
This guide assumes that you will take advantage of the version 9 compat libraries to facilitate your upgrade. These libraries allow you to continue using version 8 code alongside code refactored for version 9. This means you can compile and debug your app more easily as you work through the upgrade process.

Keep in mind: the compat libraries are a temporary solution that will be removed completely in a future major SDK version (such as version 10 or version 11). Your ultimate goal is to remove compat code and keep only version 9 modular-style code in your app.

How could we move from angular/fire/compat to the new angular/fire ?
It seems some angular/fire object completly changed or were removed.

@GrandSchtroumpf
Copy link
Collaborator

@randallmeeker I couldn't publish with Ivy, I've got this error message :

ERROR: Trying to publish a package that has been compiled by Ivy in full compilation mode. This is not allowed.
Please delete and rebuild the package with Ivy partial compilation mode, before attempting to publish.

I removed your changed, so I could publish. You can install it with npm install akita-ng-fire@next to try it out.

@avelow moving to angular/fire will change most of the code. And it'll introduce breaking changes from existing version. As I wrote in the readme, I'm not actively maintaining this lib anymore, but I'm ok to review PRs from the community.

@hakimio
Copy link
Contributor

hakimio commented Sep 30, 2021

@GrandSchtroumpf since you are releasing a new major version, breaking changes are expected. I would vote for doing proper upgrade instead of this "compat mode" approach.

Also, full or partial "Ivy" compilation in libraries should not be enabled since it will break backwards compatibility with older Angular versions.

@kjetilhp
Copy link

If this is not gone be maintained, a compat release now is better than waiting for a might never happen release, then we can upgrade firebase and angular/fire and migrate away from this if it is not going forward.

@hakimio
Copy link
Contributor

hakimio commented Sep 30, 2021

I'll try to do a proper upgrade to Firebase v9 sometime next week if nobody else is taking the task.

@randallmeeker
Copy link
Collaborator

randallmeeker commented Sep 30, 2021

@hakimio I did the compat upgrade now because the full update for Firebase V9 is not feature complete with angular/fire. I assumed it was best to wait for angular/fire to finish full support before we attempted here. This allows everyone to update to Firebase v9 in the short term.

@hakimio
Copy link
Contributor

hakimio commented Sep 30, 2021

@randallmeeker what features are missing?

@randallmeeker
Copy link
Collaborator

I don't know for sure, I'm just reading from the upgrade guide here
https://github.com/angular/angularfire/blob/master/docs/version-7-upgrade.md#upgrading-to-angularfire-70

@randallmeeker
Copy link
Collaborator

@GrandSchtroumpf I get the IVY issue. The difference between building the app and publishing the app. Make sense.

@hakimio
Copy link
Contributor

hakimio commented Sep 30, 2021

@randallmeeker Maybe it's support for some obscure features like Firebase App Check. I think having proper integration is crucial and it's still worth trying to do proper migration.

@randallmeeker
Copy link
Collaborator

@hakimio It looks like analytics features and a few other things are missing. I don't think anyone is saying to not continue with "proper" integration, and I have already done some of work for full v9, but that this is the best first step and unavoidable for people who want to upgrade to V9 sooner rather than later (like myself).

You speak as if we are saying that this is where we stop all development when I'm not saying that at all.

I think as soon as AngularFire 7.1 (here soon) is out we can see a more stable feature complete version, fixed documentation and a bunch of bugs ironed out. I don't think its a huge lift overall as I have already made what I think is about 80% of the necessary changes. I think the bigger / harder work is probably just going through and updating the documentation.

Like I said I have started some work on this, but can't touch it for another week or so. I'll share a branch of my changes over the weekend and we can collab on it if you want (or feel free to submit yourself, I take no offense if you don't want to wait).

@alexis-mrc
Copy link
Contributor Author

Giving the "compat" version could be a great minor version for akita-ng-fire without breaking change and the "propre" integration could be the next major release.

I would like to help but i don't really know how to.

@hakimio
Copy link
Contributor

hakimio commented Sep 30, 2021

@randallmeeker Ok, sure. Let me know once you have pushed your changes to Github and I'll try to help with the migration.

@avelow I agree. The compat mode should be v6.1 not v7.

@randallmeeker
Copy link
Collaborator

@hakimio @avelow due to the dependency requirements, compat was still a breaking change and requires v7. That was unavoidable.

@hakimio
Copy link
Contributor

hakimio commented Oct 6, 2021

@randallmeeker did you have time to share your changes?

@randallmeeker
Copy link
Collaborator

@hakimio Here is my branch. So far I got through the collection service. But now think this is going to need a ton more work than I thought, and I honestly don't think angularfire is there yet.

https://github.com/randallmeeker/akita-ng-fire/tree/non-compat

I'll keep at it, but not in a meaningful way till I see more updates on the angularfire side probably.

@hakimio
Copy link
Contributor

hakimio commented Oct 7, 2021

@randallmeeker thanks for sharing your changes.
I have updated "Real Time" and "Auth" services.
Why exactly do you think "angularfire is not there yet? What's missing?

@hakimio
Copy link
Contributor

hakimio commented Oct 8, 2021

Update on my progress
DONE:

  • Went through all the files and properly migrated everything to Firebase v9 modular SDK (it's quite a lot of work but doable)
  • Demo app seems to be working as it should
  • 29/31 collection service tests pass
  • 5/5 await-sync-query tests pass

TODO:

  • Fix 2 failing collection service tests
  • Figure out how to run real-time database service tests which don't seem to belong to the akita-ng-fire project
  • Update documentation

@fritzschoff any tips how to run real-time db tests?

@randallmeeker
Copy link
Collaborator

rock! sorry for not replying sooner. My main issue is the lack of documentation, but this is good news.

@hakimio
Copy link
Contributor

hakimio commented Oct 14, 2021

Just created a new PR (#223) to migrate to Firebase v9 modular SDK 🎉

@kjetilhp
Copy link

@hakimio - somewhat off topic, but asking anyway since you have spent this much time with the codebase lately, Mr Basal have released a new state management framework now called Elf - have you (or anyone else here) put any thought into creating an elf-fire project based on the knowledge from this project?

@hakimio
Copy link
Contributor

hakimio commented Oct 15, 2021

@kjetilhp yeah, I saw the new project, but, at least for now, I don't have any plans to migrate or look into creating elf-ng-fire. This is a pretty big library and creating something similar for elf will require quite a bit of effort.

@GrandSchtroumpf
Copy link
Collaborator

To be honest, I've stopped using the "akita" part of this lib since a while. It brought more complexity that it solved. At some point I think I'll publish a ng-fire base library with only the FireCollection and FireAuth, it might help built on top of it.

@hakimio
Copy link
Contributor

hakimio commented Oct 25, 2021

@kjetilhp @Disane87 @randallmeeker @avelow
7.0.0-beta.2 was just released with support for Firebase v9 modular SDK. Feel free to test it, ask questions and report any issues you find.

I have also updated documentation, README and the example app for v7. It now requires angularfire v7+, firebase v9.1 modular SDK and Angular v12. You can also check out angularfire modular example app to see how it's supposed to be used.

EDIT: BREAKING_CHANGES

@randallmeeker
Copy link
Collaborator

Great I will check it out this weekend

@hakimio
Copy link
Contributor

hakimio commented Nov 8, 2021

7.0.0-beta.3 has been released last week with the following changes:

@randallmeeker
Copy link
Collaborator

randallmeeker commented Dec 15, 2021

FWIW, been using this on a new project for a few weeks now and it appears fine so far props to @hakimio

@hakimio hakimio mentioned this issue Jan 26, 2022
@GrandSchtroumpf
Copy link
Collaborator

GrandSchtroumpf commented Jan 26, 2022

Hello, I've just release version 7.0.0 as latest after PR from @hakimio.

To all of you I'm sorry it took that long for this release process. As written in the readme, I'm not actively maintaining this project anymore. As previously mentionned, I personally don't work with akita anymore, and after working a lot with akita-ng-fire, I found out that managing global stores with firestore was introducing more issues that it was solving.

I still think that extending services with a FireCollection<T> make sense, so I started another lib without akita. It's not production ready yet, and documentation is poor, but if you ends up in the same situation than I, you might want to take a look.

Thank you all for the time and patience, and especially thanks to @hakimio & @randallmeeker for the amazing job and feedback 🎉. I feel very humbled by your contribution 😊.

@randallmeeker
Copy link
Collaborator

good news. I also thank @hakimio for doing the heavy lifting. and @GrandSchtroumpf this library has been great for almost all my firebase projects. I will say they are not particularly large wonder if you could give an example of where you started to hit a wall.

I know I have some collections where I use the ngFirebase lib directly and don't bother creating a store

I will admit that the stand-alone service is a good idea, my only suggestion that I have been wanting to implement is a firebase interceptor so that I can catch permission errors or connection issues and give context to the collections and the action being performed. When I have added rules after a large build, it has taken some creative logging to find where synced collection was breaking.

@oliveti
Copy link

oliveti commented Feb 13, 2022

Hello,

Thank you for your hard work on this migration ! I'm running a quite small app with this library so I thought I was going to benefit a lot from this migration to the modular library (I only use auth + firestore + fileStorage).

I was actually quite surprised to see that my prod bundle is actually slightly bigger after upgrade (I removed all imports of compat library).

Before upgrade :
Initial Chunk Files | Names | Raw Size | Estimated Transfer Size
main.663399c5f2234a4f.js | main | 897.48 kB | 216.51 kB
styles.128145065d320847.css | styles | 88.39 kB | 8.10 kB
polyfills.88a907aed18bb988.js | polyfills | 36.23 kB | 11.51 kB
runtime.bbaca53ce3a229be.js | runtime | 3.05 kB | 1.44 kB

                          | Initial Total          |   1.00 MB |               237.57 kB

Lazy Chunk Files | Names | Raw Size | Estimated Transfer Size
795.a9e04131eac95fdc.js | tangles-tangles-module | 336.71 kB | 74.80 kB
315.8af113ac13efda33.js | firebase-auth | 175.14 kB | 47.91 kB
362.1038467a03ba1deb.js | tangles-tangles-module | 36.82 kB | 9.67 kB
762.0019a366b825e3ec.js | tags-tags-module | 4.44 kB | 1.74 kB

Build at: 2022-02-13T12:04:44.210Z - Hash: 470a775eece6ad0b - Time: 44045ms

Warning: bundle initial exceeded maximum budget. Budget 500.00 kB was not met by 525.15 kB with a total of 1.00 MB.

After upgrade :
Initial Chunk Files | Names | Raw Size | Estimated Transfer Size
main.d5f6efead340e8b6.js | main | 917.87 kB | 231.61 kB
styles.848e83b8da6dcb5f.css | styles | 88.73 kB | 8.13 kB
polyfills.88a907aed18bb988.js | polyfills | 36.23 kB | 11.51 kB
runtime.ea9976455fcfb3fb.js | runtime | 2.80 kB | 1.31 kB

                          | Initial Total          |   1.02 MB |               252.57 kB

Lazy Chunk Files | Names | Raw Size | Estimated Transfer Size
789.a1fe70437298761f.js | tangles-tangles-module | 321.59 kB | 71.82 kB
308.1f72569f986188cd.js | tangles-tangles-module | 37.02 kB | 9.70 kB
762.f5a03e79f64fa3cf.js | tags-tags-module | 4.44 kB | 1.75 kB

Build at: 2022-02-13T13:12:25.176Z - Hash: 80541329b022570f - Time: 24276ms

Warning: bundle initial exceeded maximum budget. Budget 500.00 kB was not met by 545.63 kB with a total of 1.02 MB.

Does it make sense for you ? Is somehow tree shaking not working yet with the modular library and not in every cases ?

@hakimio
Copy link
Contributor

hakimio commented Feb 13, 2022

@oliveti the issue is that while Firebase v9 is tree-shakable, akita-ng-fire itself is not. It pulls pretty much everything in its classes and the bundle size increases because total size of Firebase increased as well.

EDIT: if you want a truly tree-shakable solution, you might want to try using vanilla @angular/fire with @ngneat/elf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants