Firebase 9 support - Githubissues

alexis-mrc commented 3 years ago

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 commented 3 years ago

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

alexis-mrc commented 3 years ago

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
});

GrandSchtroumpf commented 2 years ago

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

alexis-mrc commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

randallmeeker commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

alexis-mrc commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

randallmeeker commented 2 years ago

@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 commented 2 years ago

@randallmeeker what features are missing?

randallmeeker commented 2 years ago

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 commented 2 years ago

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

hakimio commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

hakimio commented 2 years ago

@randallmeeker did you have time to share your changes?

randallmeeker commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

hakimio commented 2 years ago

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

kjetilhp commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

Great I will check it out this weekend

hakimio commented 2 years ago

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

Fixed duplicate values emitted by CollectionService and FireAuthService because rxfire library used by angularfire has changed and hard-coded default includeMetadataChanges option to true. Starting with beta.3 we've changed includeMetadataChanges back to the default value of false and if you need to listen to metadata changes, you can override the config in your classes extending CollectionService and FireAuthService. PR #228
Fixed callFunction() utility function typings. PR #225

randallmeeker commented 2 years ago

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

GrandSchtroumpf commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

                          | Initial Total          |   1.00 MB |               237.57 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.

                          | Initial Total          |   1.02 MB |               252.57 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 commented 2 years ago

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

dappsnation / akita-ng-fire

Firebase 9 support #205