Search code examples
angularrxjsngrxngrx-storengrx-effects

Best Practice to initiate Effect and action


So I spent a lot of time to make those two approaches work, specially with all the ngrx operators. Yet, I'm not able to decide which would be the best approach for the use case im working on.

So I have the following case:

  1. On page load, I make sure the user is logged in, and load all the user data in an auth feature state.
  2. I want also to load all the settings on page load in a settings feature state, however loading the settings from the api requires having the current user id, that i get from the previous step.

My store is modeled as below:

enter image description here

So the two approaches i implemented to make this work was:

First Approach

  1. I load here the user data, and
  2. whenever i receive it the effect,
    1. i dispatch the AllSettingsRequested action that loads the settings.
@Effect({dispatch: false})
loadUser$ = this.actions$
.pipe(
    ofType<UserRequested>(AuthActionTypes.UserRequested),
    withLatestFrom(this.store.pipe(select(isUserLoaded))),
    filter(([action, _isUserLoaded]) => !_isUserLoaded),
    mergeMap(([action, _isUserLoaded]) => this.auth.getUserByToken()),
    tap(_user => {
        if (_user) {
            this.store.dispatch(new UserLoaded({ user: _user }));
            this.store.dispatch(new AllSettingsRequested()); /* Dispatch the Load Settings Action */
        } else {
            this.store.dispatch(new Logout());
        }
    }),
    catchError(err => {
        console.log(err);
        return of([]);
    })
);

Then in the Settings Effect I can simply add the following:

@Injectable()
export class SettingsEffects {
    loadSettings = createEffect(() => this.actions$.pipe(
        ofType(SettingsActionTypes.AllSettingsRequested),
        withLatestFrom(this.store.pipe(select(currentUser))),
        mergeMap( ([action, user]) => 
                this.settingsService.getSettings( user.id )
            ),
        map(s => {
            return new AllSettingsLoaded({settings: s.model});
        })
    ))
}

Approach works fine, however dispatching the SettingsRequest action from the UserLoaded effect does not feel right.

Second Approach

Second approach is just initiate the SettingsRequested action on page load, and make the effect wait till the user data is loaded and then call the settings api and load the settings.

@Injectable()
export class SettingsEffects implements OnInitEffects  {

    loadSettings = createEffect(() => this.actions$.pipe(
        ofType(SettingsActionTypes.AllSettingsRequested),
        mergeMap(action => {
            console.log("im here");
            return combineLatest(
              of(action),
              this.store.pipe(select(currentUser))
            )
             }
          ),
        skipWhile(([action, currentUser]) => {
            console.log('Checking Current User.. ' + currentUser);
            return (!currentUser); 
        }),
        mergeMap( ([action, user]) => 
                this.settingsService.getSettings( user.id )
            ),
        map( (s) => {
            return new AllSettingsLoaded({settings: s.model});
        })
    ));

    /* Dispatch the action on page load */
    ngrxOnInitEffects(): Action {
        return { type: SettingsActionTypes.AllSettingsRequested };
    }

As you can see the effect here got more complex, however, i don't need to dispatch the SettingsRequest action from the userload effect like the first approach.

I hope i was able to articulate the problem and the two approaches. My question is, which approach makes more sense, in terms of ngrx best practices?


Solution

  • I would opt for a third approach.

    Instead of dispatching UserLoaded and AllSettingsRequested from approach one, I would just dispatch UserLoaded. The settings effect can listen to this action and fetch the settings. By doing this you can also don't have to read the user id from the store, since it exists on the UserLoaded action.