dappsnation / akita-ng-fire

Akita ❤️ Angular 🔥 Firebase
MIT License
131 stars 27 forks source link

Upgrade for Firebase 8 (CJS to EMS) #163

Closed alexis-mrc closed 3 years ago

alexis-mrc commented 3 years ago

Hello,

Firebase 8 can now be used with ESM instead of CJS. This would be great to upgrade akita-ng-fire to use it with Firebase 8 with the correct import.

https://firebase.google.com/support/release-notes/js

I can try to do a pull requests if needed

alexis-mrc commented 3 years ago

I did a PR #165 to solve this issue

fritzschoff commented 3 years ago

Thank you very much!

alexis-mrc commented 3 years ago

Do you know when i will can use it ?

I want to check if tree shaking with firebase can significantly reduce my angular bundle size !

fritzschoff commented 3 years ago

When you merge I can make a beta branch for you on NPM

alexis-mrc commented 3 years ago

I don't see how to merge it

fritzschoff commented 3 years ago

okay published a beta version. try to run npm i akita-ng-fire@beta

alexis-mrc commented 3 years ago

I have tested it

Tree shaking doesn't seems to be that efficient (from 88.56KB to 81.13 KB for the Gzip size of the chunk using firestore + akita-ng-fire)

By the way, I still got an error : xxxx.module.ts depends on '@angular/fire/firestore'. CommonJS or AMD dependencies can cause optimization bailouts.

I am not the only one getting it, i think a fix need to be done in @angular/fire to have a better tree shaking

I did another pull request #166 to fix firebase imports + update breaking changes file

fritzschoff commented 3 years ago

Thanks for all the PRs. I guess I will release version 4 if we now have peer dependency for firebase 8. I merged your PR and will update the v4 branch

fritzschoff commented 3 years ago

version 4 is now the latest. And your PR is merged. Thanks again for your work!

fritzschoff commented 3 years ago

Will close this issue now but feel free to create a new one