NgRx effect mergeMap approach

Question

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 }))
            )
        )
    );

Great question. I wish I'd done it before, it would have saved me a lot of trouble! — Athanasios Kataras, Nov 22 '19 at 16:08

score 2 · Answer 1 · answered Nov 22 '19 at 16:04

2

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.

answered Nov 22 '19 at 16:04

Athanasios Kataras

25,191
4
32
61

Thanks for your response. I will point out that the inner maps you're referring to are not the rxjs pipeable map operator, but a simple map operation on an array. Also, I'm wondering if you might know why the typechecking for the first option doesn't work. – xandermonkey Nov 22 '19 at 16:26
What exactly is happening in the first vs second? The second has one less observable than the first, which in my opinion is better. – xandermonkey Nov 22 '19 at 16:26
Which one is the second observable? They both have exactly the same. – Athanasios Kataras Nov 22 '19 at 16:39
In the first example mergeMap will combine all the instances and return one observable with multiple emissions. The map right after it will then return one observable for each one of those emissions – xandermonkey Nov 22 '19 at 17:55
Whereas, the second example simply returns one observable that emits multiple times. – xandermonkey Nov 22 '19 at 17:55

score 1 · Accepted Answer · answered Nov 29 '19 at 11:36

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.

The first does work as far as I can tell. Your explanation of the second is clear. Thanks — xandermonkey, Nov 29 '19 at 17:35

NgRx effect mergeMap approach

2 Answers2