Fix for #1385 that was caused by firebase releasing 4.8.1 #1386
Conversation
…ce. Fixing import for main @firebase/app. This resolves failing tests.
| @@ -1,6 +1,4 @@ | |||
| import { NgModule, NgZone } from '@angular/core'; | |||
| import * as firebase from 'firebase/app'; | |||
| import 'firebase/auth'; | |||
There was a problem hiding this comment.
isn't it better using scoped one?
import '@firebase/auth';
// ...There was a problem hiding this comment.
I am confused. This is simply a removal as it was not being used here. I did not add back any imports, only removed.
There was a problem hiding this comment.
Oh my bad. Sorry, I think I need some coffee.
There was a problem hiding this comment.
No worries, I feel ya. Been up since 7am yesterday (now 1pm). I am seeing cross eyed so I really was not sure if I was missing something!
|
@myspivey thanks so much for your work here! I'm coming up on xmas vacation but will try to get a review in here ASAP + merge before the holiday if I can. |
|
Please merge, pretty please? |
|
Just did a review and it looks great. Huge kudos to @myspivey for saving the day. |
|
Would love to see this in an RC release ASAP :) Thank you @myspivey! I tried the |
|
For anyone who's looking to test their code without building angularfire2 manually, i've pushed the new build to a github repo. You can install with Make sure you change your |
Checklist
npm install,npm run build, andnpm testrun successfully? yesDescription
With #334 from firebase-js-sdk being released in 4.8.1, this breaks the imports for angularfire2. This commit resolves that by updating to the new typings that were released in 4.8.1. I did not bump the package version for firebase to above 4.8.1 but since this is not a backwards compatible change that might be important to do. There was also a name collision with FirebaseApp so I choose to cast the upstream to FBApp until the proper solution can be determined. Let me know if there are any issues with the solution and I will address them! Thanks for your consideration.