1
votes

I'm using ngrx in order to store my user state. With user state I mean an IUser class object with some user properties.

I've set several action ('USER_LOGIN', 'USER_LOGIN_SUCCESS' and 'USER_LOGJN_FAILED').

Currently, when an USER_LOGIN_SUCCESS is dispatched, the reducer assigns the new user information to my IStore.user ngrx store field. When it's changed:

this.user$ = this.store$.select(state => state.user);
this.userSub = this.user$.subscribe(
    (user: IUser) => {
        if (user.id != null)
           this.router.navigate(['/app']);
    }
);

As you can see, I only check if the state of my changed user is user.id != null and then I navigate to my /app route.

Is there to do it using a mor elegant way. For example,

  1. without using a explicit subscription to my IStore.user field?
  2. Could I navigate straightforwardly to a route when an action ('USER_LOGIN_SUCCESS') is dispatched?
  3. Could I focus all this behavior in a single injectable?

Any ideas?

2

2 Answers

5
votes

Yes, you can do this in a more elegant way.
Let's review it step by step :)

1) Use filter from rxjs instead of if statement

this.userSub = this
  .user$
  .filter((user: IUser) => user.id !== null)
  .subscribe((user: IUser) => this.router.navigate(['/app']));

2) Avoid to handle subscriptions one by one
(there's a great article from Ben Lesh on Medium about that)

private componentDestroyed$: new Subject<void>();

ngOnInit() {
  this
    .user$
    .filter((user: IUser) => user.id !== null)
    .takeUntil(this.componentDestroyed$)
    .subscribe(_ => this.router.navigate(['/app']));
}

ngOnDestroy() {
  this.componentDestroyed$.next();
  this.componentDestroyed$.complete();
}

3) Do not apply your logic inside the subscribe

Use a do or a map to handle your logic, before the subscribe.
- do if you simply need to do something
- map if you want to return the result

this
  .user$
  .filter((user: IUser) => user.id !== null)
  .takeUntil(this.componentDestroyed$)
  .do(_ => this.router.navigate(['/app']))
  .subscribe();

(CF next part for the "why")

4) Use "selectors" if you want to re-use or export some logic

If I had to make some guesses on your code, I imagine that your user$ is the current user. The one connected right ? And you may have to copy all that code in other part of your app to find this user again.

That a good use case for a "selector".
(I exported 2 functions here so this code can be AOT friendly).

export function _getCurrentUser(store: Store<any>) {
  return store$
    .select(state => state.user)
    .filter((user: IUser) => user.id !== null);
}

And then in your component :

this
  .store$
  .let(getCurrentUser)
  .takeUntil(this.componentDestroyed$)
  .do(_ => this.router.navigate(['/app']))
  .subscribe();

In the end we have

a user.selector.ts :

export function getCurrentUser(store: Store<any>) {
  return store$
    .select(state => state.user)
    .filter((user: IUser) => user.id !== null);
}

a component :

private componentDestroyed$: new Subject<void>();

ngOnInit() {
  this
    .store$
    .let(getCurrentUser)
    .takeUntil(this.componentDestroyed$)
    .do(_ => this.router.navigate(['/app']))
    .subscribe();
}

ngOnDestroyed() {
  this.componentDestroyed$.next();
  this.componentDestroyed$.complete();
}
0
votes

One thing you could do to tidy is use the .skipUntil operator on your subscription as follows;

this.user$ = this.store$.select(state => state.user);
this.user$.skipUntil( user => user.id );
this.userSub = this.user$.subscribe(
    (user: IUser) => {
           this.router.navigate(['/app']);
    }
);

then you don't need the conditional inside the subscription.