CODEWITHSUNDEEP
angularreduxrxjsfrontendngrx
Thomas
Thomas

Reputation: 150

Should we use NEVER in some NgRx Effects cases?

In some NgRx effects use cases, I wonder how to manage a "condition is not met" in the Observable stream. In the code below, actions should still be dispatched if the condition is met but, if the condition isn't, this isn't however an error.

  someEffect$ = createEffect(() =>this.actions$.pipe(
    ofType(SomeActions.fooFired),
    switchMap(action => this.sharedService.apiCall().pipe(
      concatMap((data: SomeType) => {
        if (data.condition === 'ok') {
          return [
            SomeActions.barFired({payload: data.payload}),
            SomeActions.successBar()
          ];
        } else {
          // EMPTY, NEVER... neither ?
        }
      }),
    )))
  );

Upvotes: 0

Views: 1041

Answers (2)

Andrei Gătej
Andrei Gătej

Reputation: 11934

I think a good way to answer this question is to first have a look at EMPTY and NEVER implementations, and then examine how concatMap()(a.k.a mergeMap(() => {}, 1)) works.

EMPTY:

export const EMPTY = new Observable<never>(subscriber => subscriber.complete());

and NEVER:

export const NEVER = new Observable<never>(noop);

You might already be familiar with the above snippets. Now it's important to highlight how concatMap, specifically mergeMap, works.

mergeMap takes in a callback function that defines how the inner observables will look like and a concurrent argument, which specifies how many inner observables should be active at the same time. An inner observable could be said that it's active if it has been subscribed to. It becomes inactive when it completes. If concurrent reached its max value and a new inner observable is about to be created, the outer value based on which

By looking at the source code

() => {
  // INNER SOURCE COMPLETE
  // Decrement the active count to ensure that the next time
  // we try to call `doInnerSub`, the number is accurate.
  active--;
  // If we have more values in the buffer, try to process those
  // Note that this call will increment `active` ahead of the
  // next conditional, if there were any more inner subscriptions
  // to start.
  while (buffer.length && active < concurrent) {
    const bufferedValue = buffer.shift()!;
    // Particularly for `expand`, we need to check to see if a scheduler was provided
    // for when we want to start our inner subscription. Otherwise, we just start
    // are next inner subscription.
    innerSubScheduler ? subscriber.add(innerSubScheduler.schedule(() => doInnerSub(bufferedValue))) : doInnerSub(bufferedValue);
  }
  // Check to see if we can complete, and complete if so.
  checkComplete();
}

/**
 * Checks to see if we can complete our result or not.
 */
const checkComplete = () => {
  // If the outer has completed, and nothing is left in the buffer,
  // and we don't have any active inner subscriptions, then we can
  // Emit the state and complete.
  if (isComplete && !buffer.length && !active) {
    subscriber.complete();
  }
};

we can see that if the inner observable completes, the outer one will send a complete notification only if it had completed before.

So, with that in mind, I'd say it's okay to return EMPTY if the condition is not met. It will just cause the inner observable to complete without making the outer observable emit any values.

Upvotes: 1

StPaulis
StPaulis

Reputation: 2916

This is a very opinionated question and answer but I will say my 2 words cause they may help.

First of all, I don't understand why concatMap is useful for. Usually nested observables can be avoided for cleaner result.

Second, as you create an Action for the Success you should handle Fail too. Something like this:

  someEffect$ = createEffect(() =>this.actions$.pipe(
  ofType(SomeActions.fooFired),
  switchMap(action => this.sharedService.apiCall()),
  switchMap((data: SomeType) => {
        if (data.condition === 'ok') {
          return SomeActions.successBar();
        } else {
          return SomeActions.failBar();
        }
      }),
    )))
  );

If you want to do another action on success, I would create another effect cause as you already said: (this is not very SRP compliant)

Should I return EMTPY (but that's complete the stream...) or NEVER ?

Try to Keep It Simple. Conventions may are useful sometimes, but they don't help others to understand what you want to say.

Try to keep your code clean and meaningful as much as you can and remember that @effects should return an Action. That's the default behavior.

If you are not sure If you are going to return a new Action use {dispatch: false} as parameter in the decorator but remember that you should trigger all the dispatches manually.

For more: How-to-create-an-ngrx-effect-that-doesnt-return-an-action

Upvotes: 1

Related Questions