fix(auth): add scheduler to schedule onAuth events through Angular zone #368
Conversation
| authCb = null; | ||
| backend = new FirebaseSdkAuthBackend(app); | ||
| })(); | ||
| var mockNgZone = Zone.current.fork({ |
There was a problem hiding this comment.
Could you add a comment on why you need to wrap the code in a zone. What exactly needs to be wrapped ?
There was a problem hiding this comment.
This code actually doesn't need to be run in a zone. I should fork the zone in the spec that checks the zone to make sure the subscription callback isn't in the <root> zone.
There was a problem hiding this comment.
I know; I'm saying this code is not actually necessary.
|
As discussed, please add a TODO comment to the code to fix the this properly. Is anyone working on a solution ? |
|
@vicb I'm working on addressing the bigger issue with Zone+Rx outside of this. But for now I changed the PR to wrap the subscriber functions at subscribe time. |
| @@ -112,3 +111,10 @@ export class FirebaseSdkAuthBackend extends AuthBackend { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| function wrapSubscriberMethods<T>(obs: Observer<T>): Observer<T> { | |||
|
Please add comments to the code for maintainabilty. LGTM once the comments are adressed |
jeffbcross
force-pushed
the
zoneschedule
branch
2 times, most recently
from
1778ed3 to
7fb596e
Compare
|
I'm going to make |
Closes #354