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

Suggestion: logout to return a promise #583

Closed
ghost opened this issue Oct 1, 2016 · 6 comments · Fixed by #772
Closed

Suggestion: logout to return a promise #583

ghost opened this issue Oct 1, 2016 · 6 comments · Fixed by #772

Comments

@ghost
Copy link

ghost commented Oct 1, 2016

Suggestion: logout to return a promise

Why not have FirebaseAuth.logout return a promise?

Version info

Angular: 2.0.1
Firebase: 3.4.0
AngularFire: 2.0.0-beta.5

Use case

Most of the time, if not always after logout you'll need to redirect.

Debug output

Here is what I mean: line 89
https://github.com/youpage/angular2-firebase/blob/master/src/auth/services/auth-service.ts

And the call from the Toolbar: line 26
https://github.com/youpage/angular2-firebase/blob/master/src/layout/components/toolbar.ts

Expected behavior

FirebaseAuth.logout should return a promise

Actual behavior

No returning behaviour

Thank's

P.S. Other than this, great job guys, I love it.

@mukesh51
Copy link
Contributor

mukesh51 commented Oct 1, 2016

@youpage It is worth noting that logout() is an asynchronous operation. There is an open bug against the Firebase SDK to make this return a promise.

@katowulf @davideast FYI

@katowulf
Copy link
Contributor

katowulf commented Oct 4, 2016

Yep, not a lot we can do here until the underlying SDK is fixed. There's currently no way for AngularFire to know when the action is completed either.

@DennisSmolek
Copy link

IMO it made sense not to rely on logout() for the redirect.

What we do is bind to the Auth.. Something like:

doLogin(whatever){
    this.af.auth.login().then(() => {
        this.af.auth.subscribe(auth => {
            if(!auth) doRedirect();
        }
    }
}
doLogout() {
    this.af.auth.logout();
}

That way if the user gets logged out for whatever reason(expires, remote, etc) it redirects. We use this in kiosks that are left on for a long time..

@adrianabreu
Copy link

Checking firebase docs I see that

signOut
signOut() returns firebase.Promise containing void

https://firebase.google.com/docs/reference/js/firebase.auth.Auth#signOut

@katowulf
Copy link
Contributor

katowulf commented Nov 4, 2016

The best practice here is still to monitor the auth subscription, for the reasons @DennisSmolek already noted. Looks like the SDK is working as expected and we could also return a promise here now.

@adrianabreu
Copy link

I tried to follow that best practise, but I can't get this to work properly:

            this.authService.login(formValue.email, formValue.password)
            .then( success => {
                this.authService.getState().subscribe(auth => {
                    if(!auth)
                        this.router.navigate(['login']);
                });
                this.router.navigate(['reviews']);
            })

When I log in for the first time everything works fine.
I log out, everything works fine.
But I can't log in again, the router does not navigate to reviews unless I reload the app.

Am I doing something wrong?
Thank you for your time

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

Successfully merging a pull request may close this issue.

5 participants