7

I have a mobile app I'm building and right now I'm working on authentication. Before I hit my home page I need to hit a variety of endpoints on an API I've built before I can display data to the user.

All the endpoints are returning the correct data when tested in Postman, however I'm getting a null value in my second async call when I utilize it in my app.

I'm sure it has something to do with the order in which these calls are made, so I was just looking for some help as to how I can properly wait for one call to finish before starting another one.

public login() {

this.showLoading();

this.userService.getUserIdFromUserName(this.registerCredentials.username) // WORKS
  .subscribe(
    res => {
      console.log(res);
      localStorage.setItem("UserId", res.toString());
    },
    err => {
      console.log(err);
    });

this.userService.getEmployeeIdFromUserId(localStorage.getItem("UserId")) // THIS RETURNS NULL 
  .subscribe(
    res => {
      console.log(res);
      localStorage.setItem("EmployeeId", res.toString());
    },
    err => {
      console.log(err);
    });

this.authService.login(this.registerCredentials)
  .subscribe(

    data => {
      this.loading.dismissAll();
      console.log('User logged in successfully! ', data);
      this.nav.push(TabsPage);
      localStorage.setItem("Username", this.registerCredentials.username);
      localStorage.setItem("isLoggedIn", "true");
    },

    error => {
      this.loading.dismissAll();
      this.showAlert("Uh oh!", "Something went wrong. Please re-enter your login credentials or check your connection.");
      console.log(error);
    });
  }
Johannes Rudolph
  • 35,298
  • 14
  • 114
  • 172

2 Answers2

14

Your original code has a bug that leads to this error. You have three calls in your code which I will call A), B), and C):

A) this.userService.getUserIdFromUserName(this.registerCredentials.username) // WORKS

B) this.userService.getEmployeeIdFromUserId(localStorage.getItem("UserId")) // THIS RETURNS NULL 

C) this.authService.login(this.registerCredentials)

What you need to understand about RXJS is the difference between a cold Observable (which represents all information required to start an async operation) and a hot Observable (which is an Observable with the async operation already started).

The three calls A), B) and C) merely build cold observables which are started the moment you call .subscribe() on them. So by the time B) is built, A) is already started but has not completed yet. So the call to localStorage.getItem("UserId") will return null, because A) has not yet invoked its subscriber's next callback.

So what you want to do is for B) to wait on A). Also instead of stuffing something into global state (localStorage) it's probably better to flow the result from A) through to B). Enter the .mergeMap() operator:

this.userService.getUserIdFromUserName(this.registerCredentials.username) // WORKS
  .map(res => res.toString())
  .do(userId => localStorage.setItem("UserId", userId)) // cleanly separate side-effects into .do() calls
  .mergeMap(userId => this.userService.getEmployeeIdFromUserId(userId))
  .map(res => res.toString())
  .do(employeeId => localStorage.setItem("EmployeeId", employeeId))
  .subscribe(
    employeeId => {
      console.log(employeeId);      
    },
    err => {
      console.log(err);
    });

The nice thing about rxjs is that error handling just works all the way through your Observable chain. If you can execute C) in parallel, have a look at .forkJoin().

Finally, if you need a hands on explanation of .mergeMap() have a look at this answer: SwitchMap vs MergeMap in the #ngrx example

Community
  • 1
  • 1
Johannes Rudolph
  • 35,298
  • 14
  • 114
  • 172
  • +1: Great answer. Really easy to understand and overall very clean code here. Obviously this has nothing to do with you, but I wish the RxJS operators had better names. People wanted to get away from `selectMany` (which was a fine name) so they went with `flatMap`(which is also a fine name). Both and preserved the analogy with collections. `mergeMap` does nothing of the sort. It might be more descriptive in terms of the ordering, or the lack their of, but reads like it was named to deliberately provide intuition yet it fails miserably. They might as well rename it to `bind` and done with it. – Aluan Haddad Mar 21 '17 at 14:21
  • Question : wouldn't be switchmap a better solution ? (I know the difference , but still ...) – Royi Namir Apr 24 '17 at 06:49
0

This should work.Don't forget import 'rxjs/Rx'

this.userService.getUserIdFromUserName(this.registerCredentials.username)
  .map(res => res.toString())
  .do(userId => {
      console.log(res);
      localStorage.setItem("UserId", userId);
    })
  .flatMap(userId => {
       return this.userService.getEmployeeIdFromUserId(userId);
     })
  .do(res => {
      console.log(res);
      localStorage.setItem("EmployeeId", res.toString());
    })
Hikmat G.
  • 2,591
  • 1
  • 20
  • 40