Not compatible with Angular

[xtazy]

Hello,

I don't think the library is compatible with Angular: in the src/auth.build.js file, there is a setInterval that is registered causing Angular to trigger a refresh of its view every second.

Is it wanted? Is there a way to avoid this?

Best regards.

[google-oss-bot]

Hey there! I couldn't figure out what this issue is about, so I've labeled it for a human to triage. Hang tight.

[google-oss-bot]

Hmmm this issue does not seem to follow the issue template. Make sure you provide all the required information.

[bojeil-google]

The library is widely used with Angular. There is an angular library angularfire dedicated for this. This is the first time I hear of this issue. Can you elaborate more on how to reproduce this.

[xtazy]

I'm using Angular 4.3.2 with angularfire2 4.0.0-rc.1 and firebase 4.2.0.

The problem appears as soon as I inject AngularFireAuth in one of my services. A setInterval is registered by the firebase library, causing Angular to refresh every second.

[xtazy]

There is the same issue opened in the angularfire2 project (angular/angularfire#1037) but I think the problem comes from the firebase library. I may be wrong.

[xtazy]

Don't know why, but after I restarted my laptop, the problem has gone away.

Issue can be close then, sorry for that.

[xtazy]

Hi again,

I've been able to reproduce the problem in a plunkr. It seems that the issue only appears with a mobile:

• open a new tab in Chrome
• open the dev tools
• switch to mobile view
• navigate to this plunkr: https://plnkr.co/edit/h6Ku3qtTicNO8N0VEFjU?p=preview
• look at the console: the view is refreshed every second

This behavior is indeed not comptabile with angular because it affects the performances in a really bad way, especially on a mobile.

[jshcrowthe]

@bojeil-google I was able to repro his issue. Can you take a look?

[bojeil-google]

For certain mobile environments, I need the setInterval (I can't clear it as it will break stuff). I don't understand how angularJS works internally and why this could cause this. I need to be able to query storage for changes in certain mobile browsers and I can't clear that otherwise stuff break.

[jshcrowthe]

@xtazy I was fiddled around with your plunkr and was able to reproduce the issue w/o Firebase leading me to believe this is actually an Angular issue.

Take a look at https://plnkr.co/edit/NLKhh0Q9XsTNSjuBKvbm?p=preview

I was able to remove the firebase inclusion and repro the issue with a setInterval.

This is an issue but you'll probably want to file this at https://github.com/angular/angular

[Ploppy3]

@bojeil-google It's Angular, not AngularJS
@jshcrowthe Please reopen this issue.

I'm pretty confident a setInterval will trigger ngDoCheck, the question is why is there one in firebase and why would it happen only in mobile mode ???

I really don't think this is needed or that there is an other way to do it. setInterval is often a bad practice and creates performance issues.

If you run your setInterval outside zonejs (angular wise) it should not trigger ngDoCheck anymore.
I'm not familiar with zonejs so take this with a grain of salt.

[bojeil-google]

Again, this is by design. Some mobile browsers have problems detecting a localStorage change when a window is in the background. This is needed to make sure it works if 'storage' event is not detected. The setInterval is not going away.

[Ploppy3]

Why not make it optional then?

[bojeil-google]

What do you mean, make it optional? So I should tell developers that signInWithPopup working in a mobile environment is optional?

[xtazy]

If the setInterval is gonna stay then this library is not compatible with Angular. Having a view check every second is not acceptable on a mobile.

[jshcrowthe]

@xtazy, @Ploppy3 thanks a bunch for your interest in making this work well! Just wanted to give a little more context of what is going on.

The calls to setInterval are triggering change detection due to the library being loaded into an Angular zone. This is a key part of how zones work and is intended behavior.

The Auth SDK currently is compensating for a deficiency found in Android browsers around the storage event firing when localStorage changes (i.e. it doesn't always happen). Rather than have users have issues, we are bridging the gap. This is also intended behavior.

There are some potential workarounds (cc @davideast), but the limitation you've pointed out is a limitation of any library that uses any of the async invocation methods (i.e. setInterval, setTimeout, setImmediate) not just Firebase.

Though we could remove the setInterval doing so would expose users to more instability on mobile platforms which is also something we don't want. Fixing this issue here, is probably not the right approach for the time being.

[Maistho]

Would it be possible to run the setInterval outside of the angular zone? That should allow the functionality to remain, but not trigger change detection in angular.

http://www.protractortest.org/#/timeouts#angular

[Ploppy3]

@Maistho
Issue opened here: angular/angularfire#1037