CODEWITHSUNDEEP
angulartypescriptrxjsngrxmergemap
xandermonkey
xandermonkey

Reputation: 4412

NgRx effect mergeMap approach

I have two implementations of the same effect, and both work. I'm having a hard time understanding the differences between the two and which is more "correct".

Please find them below:

Option 1. IDE cannot figure out the type for instance in the last map.

    pollingStarted$ = createEffect(() =>
        this.actions$.pipe(
            ofType(pollingStarted),
            mergeMap(action => action.instances),
            map(instance => performRequest({ instance }))
        )
    );

Option 2. All types work out and make sense. This is more correct to me but I want to figure out and understand the differences.

   pollingStarted$ = createEffect(() =>
        this.actions$.pipe(
            ofType(pollingStarted),
            mergeMap(({ instances }) =>
                instances.map(instance => performRequest({ instance }))
            )
        )
    );

Upvotes: 4

Views: 3413

Answers (2)

Andrei Mihalciuc
Andrei Mihalciuc

Reputation: 2258

It seems that the first approach should not work:

pollingStarted$ = createEffect(() =>
    this.actions$.pipe(
        ofType(pollingStarted),
        mergeMap(action => action.instances),
        map(instance => performRequest({ instance }))
    )
);

mergeMap in this case flats your array, and map returns an Observablve for each emitted value. At the end you get an Observable of Observables (Observable<Observable<your type>>). You need to use one of Higher Order Observables instead of map to make it working.

Second option is the right one:

pollingStarted$ = createEffect(() =>
    this.actions$.pipe(
        ofType(pollingStarted),
        mergeMap(({ instances }) =>
           instances.map(instance => performRequest({ instance }))
        )
     )
 );

mergeMap in this case merges an array of observables produced by instances.map into a single observable. The benefit of using this approach is that you have control over observables, you can apply catchError to each performRequest or apply it on a higher level after mergeMap to have a single error handling for all performRequest calls.

Upvotes: 1

Athanasios Kataras
Athanasios Kataras

Reputation: 26362

There is a very good guide here of good and bad practices.

Consider your second example. What if you wanted to add anothe map in your second map?

   pollingStarted$ = createEffect(() =>
        this.actions$.pipe(
            ofType(pollingStarted),
            mergeMap(({ instances }) =>
                instances.map(instance => performRequest({ 
                    instance.map(() => { // And another map})
                }))
            )
        )
    );

This would very soon make your code undreadable. What about error handling?

In your first example, you can add just one catchError, that would apply for all maps. In the second case, you woul need to do an error handlind for each map you are having there.

    // VERY BAD: nesting subscribes is ugly and takes away
    // the control over a stream

The same applies to maps, and any other operator that should be piped. The pipe operator is the equivalent of linux pipe |, and is considered a best practice. It provides cleaner code.

It might not make sense for just a couple of them, but when it gets nested in multiple levels, it gets very nasty and the code becomes unreadable.

I've recently did a refactoring to get state 2 to look like one in a big project, so that I can manage the code better.

Upvotes: 2

Related Questions