Search code examples
rxjsngrx

ngrx/rxjs: can this code to first check the store and then do the API request be improved?


Before fetching new data from the backend, I'm first checking if it's in the store already, and I do take care that the store is always synced every time there's an update.

To do so, my effect does the following:

@Effect()
LoadImage: Observable<LoadImageSuccess> = this.actions$.pipe(
  ofType(AppActionTypes.LOAD_IMAGE),
  withLatestFrom(action => this.store$.select(selectImage, action.payload).pipe(map(result => ({ action, result })))),
  switchMap(observable => observable),
  mergeMap(({ action, result }) =>
    result !== null
      ? of(result).pipe(map(image => new LoadImageSuccess(image)))
      : this.imageApiService.getImage(action.payload).pipe(
          map(image => new LoadImageSuccess(image)),
          catchError(error => EMPTY)
        )
  )
);

This seems to work fine, but I wonder if it can be made nicer:

  1. The switchMap(observable => observable), doesn't look very nice.
  2. Would the condition in the mergeMap be better handled via a iif?
  3. When two components dispatch a LOAD_IMAGE action, my API call is still happening twice, because the first request hasn't completed yet (and hasn't put the image in the store) when the second request starts. This is not a common occurrence with images on my website, but might be with other components in the future and I wonder if there's a way to improve this.

Thanks!

Please note, if I remove the switchMap(observable => observable),, I get the following error:

Error

The object that gets passed to the mergeMap if I comment out the switchMap, is of type Store:

Object after withLatestFrom

EDIT:

Based on the accepted answer, this is what I ended up with:

@Effect()
LoadImage: Observable<LoadImageSuccess> = this.actions$.pipe(
  ofType(AppActionTypes.LOAD_IMAGE),
  mergeMap(action =>
    this.store$.select(selectImage, action.payload).pipe(
      mergeMap(imageFromStore =>
        imageFromStore !== null
          ? of(imageFromStore).pipe(map(image => new LoadImageSuccess(image)))
          : this.imageApiService.getImage(action.payload).pipe(
              map(image => new LoadImageSuccess(image)),
              catchError(error => EMPTY)
            )
      )
    )
  )
);

Solution

  • The problem is that the arg passed to withLatestFrom is the projection function, so that's why you have to resort to the switchMap hack.

    If you need to check based on the current action, I think you'd be better off doing it with something like this:

    ofType(...),
    
    // do the check
    switchMap(
      action => this.store.select(...)
        .pipe(
          ...,
        // act based on the check's result
        // decided to nest this as a response to question 3
        // since if multiple calls are made at the same time, only the last one will be good to go
        switchMap(({ action, result }) => ...)
      )
    ),