Skip to content

Discussion: 5.2 release #2086

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

Closed
jamesdaniels opened this issue May 24, 2019 · 43 comments
Closed

Discussion: 5.2 release #2086

jamesdaniels opened this issue May 24, 2019 · 43 comments

Comments

@jamesdaniels
Copy link
Member

jamesdaniels commented May 24, 2019

Hey all, figured I'd open an issue which could serve as a catch all for the 5.2 release and any issues people are having with the beta.

Release Candidate 3 is the most current on @angular/fire@next. We're aiming to release the week of June 27th, roughly aligning with the Angular 8 release.

Feel free to chime in.

Changelog

AngularFire 5.2 introduces support for Angular 8 and version 6 of the Firebase SDK.

Bug Fixes

  • firestore: Fix for builds targeting Node (#2079) (8a33826)
  • storage: Typo in updateMetadata method (#2029) (6133296)
  • messaging: Allow AngularFireMessaging to be included in a server build (#1938) (9b870a9)

Features

  • performance: AngularFire Performance Monitoring (#2064) (2469e77)
  • auth-guard: AngularFire Auth Guards (#2016) (e32164d)
  • firestore: Added option to include document IDs on valueChanges() (#1976) (7108875)
  • firestore: Support Firestore Collection Group Queries (#2066) (c34c0f3)
  • functions: Allow configuration of Functions Emulator Origin (#2017) (d12b4c5)
  • schematics: ng deploy schematic (#2046) (be0a1fb)
  • firestore: path on AngularFirestoreCollection's .doc is optional (#1974) (c2354f8)
@Splaktar
Copy link
Contributor

I'll give it a shot here soon. I'm just waiting on the next Material RC to be compatible with Angular rc.5 so that @angular/flex-layout will compile. I've already got the Perf module imported and will start doing some testing on that once everything is building happily, hopefully by tomorrow.

I'll try to take a look at ng deploy as well.

@jamesdaniels
Copy link
Member Author

Cut 5.2.0-beta.5 and I think that's it from me for the week.

@jamesdaniels
Copy link
Member Author

I lied, couldn't stay away from the computer 😛 After a couple more fixes I'm pretty happy with this, calling it 5.2.0-rc.1 cutting now.

@maximelefort
Copy link

Hi,
My ionic/angular project works like a charm with the 5.2.0-rc.1.
But when I want to import AngularFirePerformanceModule, I've got an issue.

Version info
Angular: ^7.2.15
Firebase: ^6.0.2
AngularFire: ^5.2.0-rc.1

Other (e.g. Ionic/Cordova, Node, browser, operating system):
Node: 10.15.3
OS: darwin x64

How to reproduce these conditions

  • Add a runtime config (http.get('/app-config.json'))
  • Init AngularFire with FirebaseOptionsToken
  • Import AngularFirePerformanceModule

Steps to set up and reproduce

NgModule looks like:

@NgModule({
declarations: [
AppComponent
],
entryComponents: [
],
imports: [
BrowserModule,
BrowserAnimationsModule,
HttpClientModule,
AngularFireModule,
AngularFirePerformanceModule,
AngularFireAuthModule,
AngularFirestoreModule,
IonicModule.forRoot(),
IonicStorageModule.forRoot(),
AppRoutingModule,
CoreModule,
],
providers: [
{ provide: APP_INITIALIZER, useFactory: appInitialization, deps: [AppConfigService], multi: true },
{ provide: FirebaseOptionsToken, useFactory: initializeFirebase, deps: [AppConfigService] },
{ provide: RouteReuseStrategy, useClass: IonicRouteStrategy }
],
bootstrap: [
AppComponent
]
})
export class AppModule {}

Factories look like this :

export function appInitialization(appConfig: AppConfigService) {
return (): Promise => {
return appConfig.loadAppConfig();
};
}

export function initializeFirebase(appConfig: AppConfigService) {
return appConfig.getConfig();
}

Sample data and security rules
allow read, write on all document (dev mode)

Debug output
** Errors in the JavaScript console **

core.js:15724 ERROR Error: Uncaught (in promise): FirebaseError: [code=invalid-argument]: projectId must be a string in FirebaseApp.options
FirebaseError: projectId must be a string in FirebaseApp.options
at new FirestoreError (index.cjs.js:352)
at Function.push../node_modules/@firebase/firestore/dist/index.cjs.js.Firestore.databaseIdFromApp (index.cjs.js:20456)
at new Firestore (index.cjs.js:20321)
at Object.firestore (index.cjs.js:21732)
at FirebaseAppImpl.push../node_modules/@firebase/app/dist/index.cjs.js.FirebaseAppImpl._getService (index.cjs.js:185)
at FirebaseAppImpl.firebaseAppImpl. [as firestore] (index.cjs.js:389)
at firestore.js:39
at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:391)
at Zone.push../node_modules/zone.js/dist/zone.js.Zone.run (zone.js:150)
at NgZone.push../node_modules/@angular/core/fesm5/core.js.NgZone.runOutsideAngular (core.js:17258)
at resolvePromise (zone.js:831)
at resolvePromise (zone.js:788)
at zone.js:892
at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:423)
at Object.onInvokeTask (core.js:17290)
at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:422)
at Zone.push../node_modules/zone.js/dist/zone.js.Zone.runTask (zone.js:195)
at drainMicroTaskQueue (zone.js:601)

** Output from firebase.database().enableLogging(true); **

** Screenshots **

Actual behavior
when I import AngularFirePerformanceModule it seems to initialize firebase earlier in the bootstrapping.
Configuration load and firebase initialization seem to be done in parallele.
That would explain why projectId is empty.

Expected behavior
Initialization should be done after the framework bootstrapping.

@jamesdaniels
Copy link
Member Author

@maximelefort I've made initialization of AngularFirePerformance eager in AngularFirePerformanceModule so that collection of automatic traces still happen even if AngularFirePerformance isn't injected into any modules... I could also provide a non-eager NgModule.

Another way to do this would be for you to provide FirebaseApp or AngularFirePerformance yourself, dependent on your AppConfigService. See the implementations here and here. I've been meaning to make use of custom providers a bit cleaner / better documented.

Thoughts?

@jamesdaniels
Copy link
Member Author

Actually, if I mark AngularFirePerformanceModule dependent on FirebaseApp it might work correctly out of box for you...

@maximelefort
Copy link

Thanks for your reply @jamesdaniels .

I think automatic traces without injected AngularPerformance is a good thing.
It's really easy to obtain traces without having to spend time to make custom traces.

I'm sorry, but I didn't understand anything. I'm not an expert of Angular and I'm a bit lost.

I don't understand how to provide FirebaseAppor AngularFirePerformance.
My AppConfigService is a simple file loader who return the firebase configuration object.
I use this object to initialize AngularFire with the FirebaseOptionsToken provider.

How can I get AngularFirePerformanceModule dependent on FirebaseApp?

@johanchouquet
Copy link
Contributor

Hi everyone,

About the version, wouldn't it be more consistent if the @angular/fire library would be of the same number of version than @angular ? In our case, it'd be version 8.

Because for now we would have:

None of these versions match.

As the library is @angular/fire it would make more sense to give it v8 (following angular versions) than following firebase versions.

Is it not too late for this ? What do you think ?

@jamesdaniels
Copy link
Member Author

@johanchouquet the main concern for that would be our major/minor cycles would be offset from Angular; as Cloud Next, Google I/O, and Firebase Summit are when we introduce new things for Firebase. If we stuck with semver then we'd either have to hold back a release for a number of months or increment much faster than either. TBH the latter is what I expected to happen but we've been fortunate that our dependant API surface hasn't changed. So we've been able to add support for both major versions of Firebase and Angular in minors. I don't expect that to be the case for much longer.

@inorganik
Copy link

inorganik commented May 28, 2019

I am hoping I messed something up on my end, but after installing 5.2.0-rc1, I am getting errors just from piping off of valueChanges().

In my service:

  constructor(
    private afs: AngularFirestore,
    private router: Router
  ) { }

  lookupPage(slug: string): Observable<PodIndex> {
    return this.afs.doc<PodIndex>(`podIndex/${slug}`).valueChanges().pipe(
      tap((podIndex) => console.log({podIndex}))
    );
  }

Typescript yells at me right away with a squiggly under the tap line, saying

Argument of type 'MonoTypeOperatorFunction' is not assignable to parameter of type 'OperatorFunction<PodIndex, PodIndex>'.

Both ng serve and ng build --prod fail with the same error.

I just created this project today with the latest everything -

"@angular/cli": "~7.3.3",
"@angular/core": "~7.2.0",
"firebase": "^6.0.4",
"@angular/fire": "^5.2.0-rc.1",
"rxjs": "~6.3.3",

Happy to provide any other details - before I upgraded to 5.2 RC1, I was running angularfire @ 5.1.3, which caused this issue when I tried to implement Universal.

@jamesdaniels
Copy link
Member Author

@inorganik what version of Typescript? We added options and some more advanced typings to valuechanges, which might explain it. Does snapshotchanges throw a similar error?

@inorganik
Copy link

Typescript 3.4.5, which ships with the latest VScode.

Yep, snapshotChanges() throws the same thing.

@jamesdaniels
Copy link
Member Author

Ah that’s probably it. We’re still targeting Typescript < 3.2 since we’re compatible with NG 6. Let me poke around and see what I can do... at the very least we should be forward compatible with a minor.

@maximelefort
Copy link

@inorganik I've got the same issue when I wanted to update rxjs to v6.5.2.
keeping the same version of rxjs from angularfire solve the problem for me : "rxjs": "6.4.0"

@Splaktar
Copy link
Contributor

Splaktar commented May 28, 2019

Everything is looking good with the RC for me (https://twitter.com/Splaktar/status/1133449910165155840) along with Angular version 8 rc.5. I'm still waiting for some of the Angular-specific stats to show up in the Firebase Performance console, but the log entries are getting sent over to the API in my production deployment.

Other than Firebase Performance and Hosting, this app isn't using much yet in production, but I did try out Firebase Auth with it in a dev branch and that looked good. I didn't try one of the new Auth Guards yet though.

Trying ng run <project>:deploy now...

@Splaktar
Copy link
Contributor

Splaktar commented May 28, 2019

I tried adding this to a project that already had AngularFire installed. I was hoping that it would add the deploy target to my project:

$ ng add @angular/fire
Skipping installation: Package already installed
? Please select a project: angular-latino (angular-hispano)
firebaseJson.hosting.find is not a function

My existing firebase.json file:

{
  "hosting": {
    "public": "dist/web",
    "ignore": [
      "firebase.json",
      "**/.*",
      "**/node_modules/**"
    ],
    "appAssociation": "AUTO",
    "rewrites": [
      {
        "source": "/enlaces/**",
        "dynamicLinks": true
      },
      {
        "source": "**",
        "destination": "/index.html"
      }
    ]
  }
}

@victorfrl
Copy link

Hi @Splaktar try with @angular/fire@next.

@Splaktar
Copy link
Contributor

Splaktar commented May 29, 2019

@victorfrl I tried ng add @angular/fire@next and ng add @angular/fire --next and got the same error.

@jamesdaniels
Copy link
Member Author

@Splaktar I've seen that too for projects that already have a firebase.json, I'll look into this.

@jamesdaniels
Copy link
Member Author

Interesting that it seems to be the RXJS version, thanks for hunting that down @maximelefort. I'll look into this in a couple hours.

@johanchouquet
Copy link
Contributor

@johanchouquet the main concern for that would be our major/minor cycles would be offset from Angular; as Cloud Next, Google I/O, and Firebase Summit are when we introduce new things for Firebase. If we stuck with semver then we'd either have to hold back a release for a number of months or increment much faster than either. TBH the latter is what I expected to happen but we've been fortunate that our dependant API surface hasn't changed. So we've been able to add support for both major versions of Firebase and Angular in minors. I don't expect that to be the case for much longer.

Fine by me, it was just an idea. I understand that versions should reflect changes of APIs if necessary.

@inorganik
Copy link

So we've been able to add support for both major versions of Firebase and Angular in minors.

Is 5.2 the first version to support Firebase 6? I don't know how feasible it is to follow the Firebase release cadence, but it's nice to know what version supports what. Angular seems to have fewer and fewer breaking changes in their major releases, but Firebase versions change a lot of things. I had to keep trying different combinations of versions of Firebase and AngularFire to find the ones that matched. No biggy though, just something to consider.

@maximelefort as I mentioned in my post I'm using 6.3.3 - which what ships with Angular 7.2. Turns out the issue was caused by a newer version of Typescript (3.4.5) - VScode was not using the version in my package.json.

@JonesM87
Copy link

Updating to this version, from 5.1.3, now using:
"@angular/fire": "^5.2.0-rc.1",
"firebase": "^5.11.1",
"rxjs": "^6.4.0",
"rxjs-compat": "^6.5.2",
"@angular/core": "^7.2.12",
"typescript": "^3.2.4",

I'm getting quite a few compilation errors all along the lines of:
.../node_modules/@angular/fire/node_modules/rxjs/internal/Observable").Observable<import(.../node_modules/@angular/fire/firestore/interfaces").DocumentChangeAction<{}>[]>' is not assignable to type 'import(.../node_modules/rxjs/internal/Observable").Observable<import(".../node_modules/@angular/fire/firestore/interfaces").DocumentChangeAction<{}>[]>'.
Types of parameters 'source' and 'source' are incompatible.
Type 'Observable<Action<DocumentSnapshot<{}>>>' is missing the following properties from type 'Observable<Action<DocumentSnapshot<{}>>>': filter, map, do, _do, and 4 more

I did try reverting to typescript 3.1.6 just in case, but as expected made no difference.

@jamesdaniels
Copy link
Member Author

Hey all, just cut 5.2.0-rc.2 which should address many of the issues you are seeing.

I've reworked the dependencies that were brought in for the schematics into peers. That way there shouldn't be any conflicts. When you ng add @angular/fire there's now an additional step before setting up ng deploy which will add the peers to you package.json.

Further I simplified the sub-module's package.json and indicated side effects, which should better inform webpack.

Let me know how this is looking!

@markgoho
Copy link
Contributor

Confirming that the rxjs errors have been eliminated with this release. Way to go James. Thanks for your hard work on this!!

@jamesdaniels
Copy link
Member Author

@markgoho awesome to hear! Thanks for the quick validation 😄

@jamesdaniels
Copy link
Member Author

@inorganik this is the first version to support Firebase v6, correct.

@jamesdaniels
Copy link
Member Author

jamesdaniels commented May 30, 2019

FYI @Splaktar I've made the ng add more accepting of a project being already setup with a firebase.json, etc. though currently it will add a new target, ignoring your existing config. Make sure to move over all your custom settings to the new target after ng add is run, so you don't lose your rewrites after you run ng deploy.

I'll probably add a prompt in a later release, maybe something like this:

It appears the project is already setup with Firebase Hosting. What would you like to do?

1) override the existing hosting config
2) add `ng deploy` as a new target and keep the existing hosting config
3) use the existing hosting config for `ng deploy`

@Splaktar
Copy link
Contributor

@jamesdaniels OK, that went a lot smoother, thank you!

Screen Shot 2019-05-30 at 17 14 27

angular-hispano/angular-hispano#42

@jamesdaniels
Copy link
Member Author

FYI I just cut 5.2.0-rc.3 which utilizes dynamic import for performance monitoring & fixes both the performance and messaging modules in server builds.

@jamesdaniels
Copy link
Member Author

I'm feeling good, will release shortly unless anyone runs into any regressions.

@swftvsn
Copy link

swftvsn commented May 31, 2019

5.2.0-rc.3 runs without problems here on two apps at the moment.

@chrste90
Copy link

@jamesdaniels
I think i found a bug with the new ng deploy command under windows. I added firebase to a new cli project with ng add. If i now try to deploy my application, i get the following error:
ENOENT: no such file or directory, open '\C\Users\name\project\.firebaserc', but the file exists and i can deploy my application manually using firebase deploy. Maybe there is some work needed to normalize the path for windows?

@jamesdaniels
Copy link
Member Author

@chrste90 I def think there is, however I am currently on the other side of the world from my Windows laptop ;) won't be back in San Francisco for another week and a half. Happy to accept a PR.

@chrste90
Copy link

@jamesdaniels Not sure how the best fix could look like, but the problems seems to be that the path in getFirebaseProjectName(projectRoot, target) for projectRoot is starting with /c/... instead of c:/....
Any idea how to read the file with the given path? Or should the path be changed to the correct format? If so, what would be the best solution? A simple replacement?

@jamesdaniels
Copy link
Member Author

Not sure. I believe it's pulled from the angular tree, so maybe this is a deeper problem. Thoughts @kirjs @mgechev?

@jamesdaniels
Copy link
Member Author

5.2.0 has been released. Closing this but we can continue to discuss.

@maximelefort
Copy link

@jamesdaniels, I'm still stuck with my runtime config when I import AngularFirePerformanceModule.

Configuration load and firebase initialization are done in parallels so the config isn't loaded when AngularfireModule try to init : stackblitz here

Another way to do this would be for you to provide FirebaseApp or AngularFirePerformance yourself, dependent on your AppConfigService.

I tried to provide FirebaseApp without success : stackblitz here

@mgechev
Copy link
Member

mgechev commented Jun 2, 2019

@chrste90, would you open an issue for #2086 (comment)? @kirjs and I will look at it.

@chrste90
Copy link

chrste90 commented Jun 3, 2019

@mgechev Done in #2101

@inorganik
Copy link

@chrste90 I think he meant either the angular repo or cli repo since it appears you are having an issue with ng deploy

@chrste90
Copy link

chrste90 commented Jun 3, 2019

Oh yes, i was not sure about it, so i created it in this repo, but i can create a new one in the cli repo if necessary. ng deploy lives in this repo but the projectRoot is coming from the cli, so it wasn't quite clear where to open.

@mgechev
Copy link
Member

mgechev commented Jun 3, 2019

This repo is fine, thanks!

@jamesdaniels jamesdaniels unpinned this issue Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests