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

23 seconds to get initial data ? #637

Closed
daslicht opened this issue Oct 23, 2016 · 29 comments
Closed

23 seconds to get initial data ? #637

daslicht opened this issue Oct 23, 2016 · 29 comments
Assignees
Milestone

Comments

@daslicht
Copy link

daslicht commented Oct 23, 2016

Hi,
i have followed the following documentation:

https://github.com/angular/angularfire2/blob/master/docs/2-retrieving-data-as-objects.md

Anything works fine, except that it takes 23 seconds to display the initial data.
If I update the data i see it immediately changed in the firebase console so the connection is good.

Thats my app:
https://daslicht.github.io/Tour-of-Heroes/items

I also tried it hosted on firebase with the same result

That's how I load the data :
https://github.com/daslicht/Tour-of-Heroes/blob/firebase/src/app/items/items.component.ts#L16

What am I missing please ?

@mukesh51
Copy link
Contributor

@daslicht works fine for me. created a quick app and the data got loaded in a second and i followed the documentation as well. did you try the sample example in documentation. how much time does it takes to load the initial data. also you're making a call to firebase in constructor. probably not a best practice. you want to change it something like this.

ngOnInit() {
    this.item = af.database.object('/item');
  }

 constructor(af: AngularFire) {    
 console.log('constructor: ',this.item);
  }

@daslicht
Copy link
Author

daslicht commented Oct 24, 2016

Hi,
thank you for the answer, I put the call in the constructor as it was shown in the Documentation.
Line 70: https://github.com/angular/angularfire2/edit/master/docs/2-retrieving-data-as-objects.md

Have you tried it with my firebase credentials ? Probably I have configured something wrong ion Firebase that it take so long ?

I also tried a similar way to you , something like this, but it was as slow as the other approach.

  af:AngularFire;
  constructor(af: AngularFire) { 
   // this.item = af.database.object('/item');
    console.log('constructor: ',this.item);
    this.af = af;
  }
  ngOnInit() {
    console.log('constructor: ',this.item);
    this.item = this.af.database.object('/item');
  }

just using af in the ngOnInit is not possible since af is not available there.

[ts] Cannot find name 'af'. Did you mean the instance member 'this.af'?
any

Or do you need to extend the Class somehow ?

@davideast
Copy link
Member

@daslicht @mukesh51 There seems to be a problem with Zones and auth right now. We are investigating. I'll post a workaround as soon as I find a proper one, and then we'll release a fix soon after.

@daslicht
Copy link
Author

Is there also a way to just extend or? implement the AngularFire Class so that you don't actually need to add it to the App Module but just to the Service. please?

@jeffbcross jeffbcross self-assigned this Oct 24, 2016
@jeffbcross jeffbcross added this to the beta.6 milestone Oct 24, 2016
@daslicht
Copy link
Author

I created a much simpler example without the Heroes overhead :

https://github.com/daslicht/angular2-firebase

@gianpaj
Copy link
Contributor

gianpaj commented Oct 25, 2016

Very strange. If you see the Network tab in Chrome for the frames in WebSocket connection, the data arrives very clicked, but then it's not displayed.

Have you tried subscribing to the FirebaseObjectObservable and printing in the console when the item is retrieved ?
I believe the ngOnInit() suggestion might be a good one to try as well.

@daslicht
Copy link
Author

daslicht commented Oct 25, 2016

Have you tried subscribing to the FirebaseObjectObservable and printing in the console when the item is retrieved ?
I tried logging it in the constructor but that just leads to the same result, what is the correct way to try this please? Pull request is welcome :)

@daslicht
Copy link
Author

FirebaseListObservable<any> seams to be NOT affected, they are displayed quite fast

@jwuliger
Copy link

Hello, I am having a serious issue right now with this as well. I am just doing a simple call as per the documentation. My exact code:

user: FirebaseObjectObservable<any>;

constructor(
    public router: Router,
    public auth: AuthService,
    public af: AngularFire
) {
    this.user = af.database.object( '/user-profile/' + auth.id );
}

Then in my template:

{{ (user | async)?.userName }}

I have tried to set this.user in the constructor and in ngOnInit both take almost 30 seconds to render to the page in the browser. When I do a console.log I get the data back instantly. I am using a staging database project with 3 records.

I also tried to use FirebaseListObservable as @daslicht stated, however this does not work for me. Is there anything I can do as a workaround?

Thanks

@daslicht
Copy link
Author

daslicht commented Oct 27, 2016

A possible workaround until we get a official fix is to wrap the return into a Zone:

https://github.com/daslicht/angular2-firebase/blob/oauth/src/app/app.component.ts#L53

@frankspin89
Copy link

I can confirm that FirebaseListObservable is not affected.

Just wrote the same function with FirebaseObjectObservable and FirebaseListObservable. The version with FirebaseObjectObservable takes more then 20 sec to load.

@jwuliger
Copy link

Yes I used the Zone work-around and that fixed the problem with FirebaseObjectObservable. FirebaseListObservable is working as expected. I just ran a test again. Thanks @daslicht and @frankspin89

@frankspin89
Copy link

@jwuliger glad to hear that the zone work-around is working for you. Sadly it's not working for me at the moment. Do you mind to share your solution?

@jwuliger
Copy link

@frankspin89 Sure! Sorry for the late reply. Here is my working component code:

import { Component, OnInit, NgZone } from '@angular/core';
import { Router } from '@angular/router';

import { AngularFire, FirebaseObjectObservable } from 'angularfire2';
import { AuthService } from '../../../core/services/auth.service';

@Component( {
    selector: 'dashboard-header',
    templateUrl: './header.component.html',
    styleUrls: [ './header.component.scss' ]
})
export class HeaderComponent implements OnInit {

    user: FirebaseObjectObservable<any>;

    constructor(
        public router: Router,
        public auth: AuthService,
        public af: AngularFire,
        public zone: NgZone
    ) { }

    ngOnInit() {
        this.af.database.object( '/user-profile/' + this.auth.id )
            .take( 1 )
            .subscribe( snapshot => {
                this.zone.run(() => {
                    if ( snapshot !== null && snapshot !== undefined ) {
                        this.user = snapshot;
                    }
                });
            });
    }

    signOut() {
        this.auth.signOut();
        this.router.navigate( [ '/dashboard' ] );
    }

}

Then is my template this is how I call the properties:

<img *ngIf="auth.authenticated" src="{{ user?.photoUri }}" alt="{{ user?.userName }}">

I hope this helps!!

@frankspin89
Copy link

frankspin89 commented Oct 28, 2016

Thanks @jwuliger for the provided example code. Sadly no improvements on the loading speed here ;-(

Here is a piece of my code:

Service

getUserProfile() {
    const getUser = new Subject();
    this.getUser().subscribe(
        authResp => {
          console.log('response received from getUser():', authResp);
          const data = this.af.database.object(`users/${authResp}`);
          getUser.next(data);
          getUser.complete();
        },
        err => {
          getUser.error(err);
          getUser.complete();
        }
      )
     return getUser.asObservable();
   }

Component

export class ProfileIndexComponent implements OnInit {
  user: FirebaseObjectObservable<UserProfile[]>;
  constructor(private authService: AuthService) {  }

  ngOnInit() {
    this.authService.getUserProfile().subscribe(data => {
      this.user = <FirebaseObjectObservable<UserProfile[]>>data;
      console.log('user',this.user)
    });
  }
}

Template

<h2>Profile index component</h2>
{{ (user | async)?.company }}

Every time I visit the corresponding page it takes around 20 sec to load the data.

I see the data instant in my console

user: FirebaseObjectObservable
_isScalar: false
_ref: U
_subscribe: (obs)
proto: Observable

@johnqiuwan
Copy link

I have a similar issue that my web app (using angular-cli workflow) takes around 10 - 15 seconds to render the data on the page. However, I see the data instant in my console.

I then clone the whole project with a different workflow (not using angular-cli workflow). The problem is gone!

I still not make sure if the problem is somewhere from angular-cli or from firebase side

@jwuliger
Copy link

@frankspin89 No problem for posting. Sorry you are still facing issues. I see you are not using the zone.run workaround. Have you tried that in your code that you posted? Either in your service or component code. Here would be an example using zone in your component code:

export class ProfileIndexComponent implements OnInit {
    user: FirebaseObjectObservable<UserProfile[]>;
    constructor( private authService: AuthService ) { }

    ngOnInit() {
        this.authService.getUserProfile().subscribe( data => {
            this.zone.run(() => {
                this.user = <FirebaseObjectObservable<UserProfile[]>>data;
                console.log( 'user', this.user )
            });
        });
    }
}

Also just out of curiosity, it looks like your data model for your user profile is being set as an array. Have you tried using it as an object? So instead of FirebaseObjectObservable<UserProfile[]> it would be FirebaseObjectObservable<UserProfile>

Also in your service, do you have authResp returning the UID/$Key of the user's profile you are getting?

On a separate note, I found that the code I posted above works in cases where I am lazy loading a module. Any other case I have to go about it differently. Here is an example of the code I use when I am not lazy loading:

The Service

@Injectable()
export class UserProfileService {
    public profile$: FirebaseObjectObservable<UserProfile>;

    constructor(
        public af: AngularFire,
        public auth: AuthService
    ) {
        const path: string = `/user-profile/${this.auth.id}`;
        this.profile$ = af.database.object( this.path );
    }
}

The Component

export class RandomComponent implements OnInit {

    user;

    constructor(
        public userProfileService: UserProfileService
    ) { }

    ngOnInit() {
        this.userProfileService.profile$
            .subscribe( user => this.user = user );
    }
}

The Template

<h1>{{ user?.userName }}</h1>

In this case I don't need to use zone.run Very strange behavior indeed.

@jeffbcross
Copy link
Contributor

Hey Folks, I'm working on a fix for this. It's actually a simple fix that's been fixed already for some of the auth APIs. I've already created a ZoneScheduler to schedule Observable values to be emitted in the correct zone, so I just need to cause the list and object factories to observe on this scheduler.

The crux of the problem is that Firebase's WebSocket listeners are being registered before Angular's zone exists, and thus are patched outside of Angular's zone, so change detection doesn't run when the WebSocket delivers new values. This isn't bad behavior from Firebase, more just a challenge with the way zone.js and Angular's zone are designed.

@jwuliger
Copy link

@jeffbcross Thanks. I can see the crux and why things have been happening the way they have.

@JT-Bruch
Copy link

@jeffbcross that is fantastic news. I just dropped in Auth today and noticed this issue. The timing worked out pretty good. :)

That said - do you know if this fix will be apart of the same release for the various typings (for installation) fixes as well?

@jeffbcross
Copy link
Contributor

@JT-Bruch Yes this is blocking the next release which will contain all fixes that are in master. Do you know of any typings issues that aren't fixed in master?

jeffbcross added a commit to jeffbcross/angularfire2 that referenced this issue Oct 28, 2016
Since Firebase is adding WebSocket listeners before the Angular Zone exists,
all callbacks when data is received are being executed outside of Angular's
zone and are not triggering change detection. This fix uses the existing
ZoneScheduler, which is already used in auth.onAuth, to cause the observables
created by af.list() and af.object() to be emitted inside the current Zone at
the time of calling .list() or .object(). 

Fixes angular#637
@JT-Bruch
Copy link

Not sure, I tried about a week ago to get master but was having more typings issues. I figured I'd wait and see as the fix is pretty easy as of right now, and looking at the commit history that was the same change made. Figured the issues I was having was due to the build process.

Appreciate all the work y'all put into this.

jeffbcross added a commit to jeffbcross/angularfire2 that referenced this issue Nov 1, 2016
Since Firebase is adding WebSocket listeners before the Angular Zone exists,
all callbacks when data is received are being executed outside of Angular's
zone and are not triggering change detection. This fix uses the existing
ZoneScheduler, which is already used in auth.onAuth, to cause the observables
created by af.list() and af.object() to be emitted inside the current Zone at
the time of calling .list() or .object(). 

Fixes angular#637
jeffbcross added a commit to jeffbcross/angularfire2 that referenced this issue Nov 1, 2016
Since Firebase is adding WebSocket listeners before the Angular Zone exists,
all callbacks when data is received are being executed outside of Angular's
zone and are not triggering change detection. This fix uses the existing
ZoneScheduler, which is already used in auth.onAuth, to cause the observables
created by af.list() and af.object() to be emitted inside the current Zone at
the time of calling .list() or .object(). 

See angular/angular#4603 for progress on making RxJS more zone-aware.

Fixes angular#637
@wayjake
Copy link

wayjake commented Nov 1, 2016

For anyone waiting on an official fix and would like to get the initial data rendered in less than 30 seconds, for the main app to re-render itself after a little under a second or roughly as long as it takes for your first round of data to arrive. This is a horrid hack and I'm slightly ashamed to share :) It just will allow you to continue using angularfire2 as normal without the zone.run() all over the place.

In my AppComponent called once to re-render the app:
constructor(private ref: ApplicationRef) { setTimeout(() => { this.ref.tick() }, 700); }

@daslicht
Copy link
Author

daslicht commented Nov 2, 2016

The issue also occurs when dealing with RxJS Subjects.
For example we have a list of slideshows and we want to store its reference to multiple slides in a separae slides collection:

...
    this.slideshows = this._af.database.list('/slideshows', {});
    this.selectedSlideKeySubject = new Subject();       
    this.slides = this._af.database.list('/slides', {
      query: {
        orderByChild: 'slideshowKey',
        equalTo: this.selectedSlideKeySubject
      }
    });
...
      this._af.database.list('/slides/').push({
        slideshowKey: this.selectedSlideshow.$key,
        name:name
      })
...
Finally apply filter:
this.selectedSlideKeySubject.next( this.selectedSlideshow.$key );

Where would I need to wrap with zone please?

@d3lm
Copy link

d3lm commented Nov 2, 2016

@daslicht You probably have to wrap the zone around your next call.

this.zone.run(() => {
  this.selectedSlideKeySubject.next( this.selectedSlideshow.$key );
});

@daslicht
Copy link
Author

daslicht commented Nov 2, 2016

hehe , already tried , no luck , thats why I am asking :)

@wayjake
Copy link

wayjake commented Nov 2, 2016

Working like a champ for me. Thanks @jeffbcross!

@daslicht
Copy link
Author

daslicht commented Nov 3, 2016

so it has been fixed now ?

@d3lm
Copy link

d3lm commented Nov 3, 2016

@daslicht Yes, @jeffbcross fixed the issue and released beta.6. Object and list factory now use the zone scheduler. I will upgrade my version later today to see if all my issues related to this topic are fixed. Thanks @jeffbcross for your work!

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