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

ApplicationRef.isStable never goes to "true" when an instance of AngularFireAuth is injected #1347

Closed
pglazkov opened this issue Nov 19, 2017 · 25 comments
Assignees

Comments

@pglazkov
Copy link

pglazkov commented Nov 19, 2017

When AngularFireAuth is injected anywhere on the page, the ApplicationRef.isStable never emits true. This causes problems with latest @angular/service-worker, which relies on ApplicationRef.isStable to install the worker. This probably also would cause server-side rendering to timeout, but I didn't test this.

Version info

Angular: 5.0.0

Firebase: 4.6.2

AngularFire: 5.0.0-rc4

How to reproduce these conditions

Inject AngularFireAuth into a component and subscribe to the ApplicationRef.isStable observable:

import { Component, ApplicationRef, ChangeDetectorRef } from '@angular/core';
import { AngularFireAuth } from 'angularfire2/auth';

@Component({
  selector: 'my-app',
  templateUrl: './app.component.html',
  styleUrls: [ './app.component.css' ]
})
export class AppComponent  {
  constructor(appRef: ApplicationRef, af: AngularFireAuth) {    
    appRef.isStable.subscribe(s => {
      console.log(s); 
    });
  }
}

Here is a project that reproduces the problem: https://stackblitz.com/edit/angularfire2-appref-isstable

Expected behavior

The ApplicationRef.isStable observable should emit true shortly after application is loaded.

Actual behavior

The ApplicationRef.isStable observable emits only initial false value.

@jamesdaniels
Copy link
Member

I'm actually tackling right now. Thanks for the report!

@jamesdaniels
Copy link
Member

Once patched I'll cut a new RC.

@playground
Copy link

I have a somewhat different issue, seems like return ApplicationRef; never got executed.

I'm new to angularfire2, can someone offer some good use cases for using angularfire2?

eggp added a commit to greg-nagy/jegybazar that referenced this issue Nov 29, 2017
@deftomat
Copy link

deftomat commented Dec 11, 2017

The same issue when I injects AngularFirestore.
We cannot use an @angular/service-worker as it is not registered due to this issue.

@LanderBeeuwsaert
Copy link

jep, same issue here.

@val-samonte
Copy link

I am having the same issue when injecting AngularFirestore

@patrickmichalina
Copy link

Any updates on this issue?

@Splaktar
Copy link
Member

Splaktar commented Jan 28, 2018

I am seeing this as well. It is being tracked in angular/angular#20970 where I added a full reproduction repo on GitHub. It's possible that angular/zone.js#999 may solve this, but it's still waiting on a review.

Update: I tested the PR (angular/zone.js#999) but it didn't help in my case.

@Splaktar
Copy link
Member

Splaktar commented Feb 10, 2018

From @JiaLiPassion "firebase auth make a very very long setTimeout to do proactive token refresh. (more than 200000ms)". Based on this comment, AngularFire2 needs to be doing something like this when running this long setTimeout.

I'm not sure how we could disable Firebase Auth from doing this so that AngularFire2 could do it in an Angular-friendly way, but this will be a problem for integration testing as well as Service Worker registration. @jamesdaniels is anyone on the Firebase team able to look into this?

For anyone blocked on the Service Worker registration, you can manually call navigator.serviceWorker.register('/ngsw-worker.js') yourself as mentioned here.

@iamart
Copy link

iamart commented Feb 11, 2018

angularfire2 cannot leave RC stage until this is fixed.

@jamesdaniels
Copy link
Member

Fixed in rc7

@PierreDuc
Copy link

PierreDuc commented May 28, 2018

The issue persists in the Auth module.

After you login some setTimeout is set at 3300000ms. The code is too obfuscated to pinpoint where it exactly comes from, but it is somewhere inside auth.js from firebase.

By using a so called TaskTrackingZoneSpec, I was able to determine that when this zone task is created, the NgZone.isInAngularZone() returns true, and the e2e tests will timeout.

When I put this inside the onScheduleTask of the trackingZone:

if (task.type === 'macroTask' && task.data.delay > 3200000 && task.data.delay < 3400000) {
  setTimeout(() => {
    Zone.current.cancelTask(task);
  });
}`

The tests will run fine, because that particular task is no longer blocking the stable state of the angular zone, but also that task no longer exists, and I don't know how important it is ;).

Apparently some calls inside the AngularfireAuthModule trigger this timeout to run inside the zone. I haven't figured out which one exactly yet. My best guess is that it has something to do with the IdToken, judging by the stack trace

@PierreDuc
Copy link

After a little more digging I found that the problem is originating from the fireauth.AuthUser.prototype.initializeProactiveRefreshUtility_.

This is at least triggered on login with using just email and password, or by using the getIdToken() and setting forceRefresh to true.

For now the workaround is whenever you call these functions is to wrap it inside a ngZone.runOutsideAngular -> ngZone.run

@PierreDuc
Copy link

PierreDuc commented May 31, 2018

Just talking with myself here, I managed to work around it by having to override the AngularFireAuth like this:

{ provide: AngularFireAuth, useClass: AngularFireOwnAuth },

And have this:

@Injectable()
class AngularFireOwnAuth extends AngularFireAuth {
  readonly idToken: Observable<string | null>;

  constructor(
     @Inject(FirebaseOptionsToken) options: any,
     @Optional() @Inject(FirebaseNameOrConfigToken) nameOrConfig: string|any|undefined,
     @Inject(PLATFORM_ID) platformId: Object,
     zone: NgZone
  ) {
    super(options, nameOrConfig, platformId, zone);

    this.idToken = this.user.pipe(
      switchMap(async user =>
        user ? zone.run(async() => await zone.runOutsideAngular(() => user.getIdToken())) : null
      )
    );
  }
}

This will make sure the getIdToken is called outside the angular zone. If this is called inside the zone, it will trigger the long setTimeout and prevent angular from ever getting stable. At least not for 3300000ms :)

@epelc
Copy link

epelc commented May 31, 2018

Thanks @PierreDuc! I can confirm the run outside zone method works as intended. I'm not using angular fire but it fixes it with interval from rxjs since it's the same idea.

@jamesdaniels
Copy link
Member

jamesdaniels commented May 31, 2018 via email

@PierreDuc
Copy link

@jamesdaniels The problem is that the observable this.user is inside the zone. So any pipe from it will be in the zone as well. Besides having to override the service, I also had to make any call to the AngularFireAuth, which in turn can possibly trigger the initializeProactiveRefreshUtility_ from firebase, to run outside the zone, and reenter after done.

I didn't have the time to figure out which of the calls trigger it, but I'm sure that at least user.getIdToken is a naughty one

@JiaLiPassion
Copy link
Contributor

@PierreDuc, maybe the problem is goog.Promise is not zone aware? Could you provide a reproduce repo?

@PierreDuc
Copy link

@JiaLiPassion well, the issue is a long lasting setTimeout, triggered by firebase itself. This timeout is running inside the angular zone, and preventing the zone to be without macroTasks. Don't think that goog.Promise is involved here.

To reproduce a repo, I have to set-up a mock firebase app, where you can register and login. Don't think I have time soon to make something like that

@JiaLiPassion
Copy link
Contributor

@PierreDuc , ok, I know about the long lasting setTimeout one.
About the code here

if (task.type === 'macroTask' && task.data.delay > 3200000 && task.data.delay < 3400000) {
  setTimeout(() => {
    Zone.current.cancelTask(task);
  });
}`

Maybe there is a better way to solve this one, if this long setTimeout is not relevant to test,
you can try this in onScheduleTask,

onScheduleTask: (delegate, curr, target, task) => {
  if ( task.source === 'setTimeout' && task.data.delay > 3200000 && task.data.delay < 3400000) {
      task.cancelScheduleRequest();
      task = Zone.root.scheduleTask(task);
      return task;
  }
  return delegate.scheduleTask(target, task);
}

@PierreDuc
Copy link

@JiaLiPassion Ah, but that's just in an additional test class called a TaskTrackingZoneSpec, which I modified a bit to determine what was causing the hanging angular zone.

But yes instead of overriding the AngularFireAuth, I could implement it like this, just in the e2e, although I don't really want something like this in my specs. It will make the e2e code different from the production code.

@JiaLiPassion
Copy link
Contributor

@PierreDuc , got it, I believe in Testability there is an additional API to let you decide when to done, but anyway, it also use TaskTrackingZoneSpec. When you have time, if you can share your e2e code, maybe I can help to find out how to make it look better, because it maybe a common use case for using zone for test. Thanks.

@JeongJun-Lee
Copy link

This issue should be reopened. Angular9 + AngularFire6 has same issue.

@Splaktar
Copy link
Member

@JeongJun-Lee it would be appropriate to open a new issue in that case.

You may want to look into https://angular.io/api/service-worker/SwRegistrationOptions as that allows you to register a SW without waiting for isStable to be true.

@yadimon
Copy link

yadimon commented Sep 1, 2020

i am developing a plugin to ignore such special async tasks.
https://www.npmjs.com/package/protractor-sync-options-plugin

angular/protractor#4584 (comment)

would be nice, if community supports me with pr and tests/issues for your use-cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests