Marko Stanimirović for This is Angular

Posted on Nov 10, 2021 • Edited on Feb 6, 2022

NgRx Tips I Needed in the Beginning

#angular #ngrx #rxjs #redux

Cover photo by Léonard Cotte on Unsplash.

This article contains a list of tips and best practices for using the @ngrx/store and @ngrx/effects libraries. The list is based on the usual NgRx mistakes I've seen many times (some of which I've made myself) and on the great talks and articles that you can find in the resources section.

Store Tips
Effects Tips
Conclusion
Resources
Peer Reviewers

Store Tips

Put global state in a single place

Try to keep the global state of your application in a single place - NgRx store. Having the state spread across multiple stateful services makes an application harder to maintain. It also frequently leads to these services "re-storing" a derived state, which makes it harder to understand where the actual source of truth for a specific data lives.

However, if you are in the process of migrating your application to NgRx, then it's fine to keep legacy stateful services as a temporary solution.

Don't put the local state in the global store

The local state is tied to the lifecycle of a particular component. It is initialized and managed during the component lifetime and cleaned up when the component is destroyed.

It's completely fine to store the local state in the component and manage it imperatively. However, if you're already using a reactive global state management solution such as NgRx store, then consider using a reactive solution for the local state management such as @ngrx/component-store. It has many powerful features and fits perfectly with the global NgRx store.

Use selectors for the derived state

Don't put the derived state in the store, use selectors instead.

Let's first see the reducer that manages the state with the derived value:

export const musiciansReducer = createReducer(
  on(musiciansPageActions.search, (state, { query }) => {
    // `filteredMusicians` is derived from `musicians` and `query`
    const filteredMusicians = state.musicians.filter(({ name }) =>
      name.includes(query)
    );

    return {
      ...state,
      query,
      filteredMusicians,
    };
  }))
);

The value of filteredMusicians is derived from the query and musicians array. If you decide to keep the derived value in the store, then you should update it every time one of the values from which it is derived changes. The state will be larger, the reducer will contain additional logic, and you can easily forget to add filtering logic in another reducer that updates query or musicians.

The right way to handle the derived state is via selectors. The selector that returns filtered musicians will look like this:

export const selectFilteredMusicians = createSelector(
  selectAllMusicians,
  selectMusicianQuery,
  (musicians, query) =>
    musicians.filter(({ name }) => name.includes(query))
);

And musiciansReducer will now be much simpler:

export const musiciansReducer = createReducer(
  on(musiciansPageActions.search, (state, { query }) => ({
    ...state,
    query,
  }))
);

Use view model selectors

View model selector combines other selectors to return all state chunks required for a particular view. It's a great way to make a container component cleaner by having a single selector per container. Besides that, view model selectors provide additional benefits.

Let's first see what the container component will look like without the view model selector:

@Component({
  // the value of each Observable is unwrapped via `async` pipe
  template: `
    <musician-search [query]="query$ | async"></musician-search>

    <musician-list
      [musicians]="musicians$ | async"
      [activeMusician]="activeMusician$ | async"
    ></musician-list>

    <musician-details
      [musician]="activeMusician$ | async"
    ></musician-details>
  `,
})
export class MusiciansComponent {
  // select all state chunks required for the musicians container
  readonly musicians$ = this.store.select(selectFilteredMusicians);
  readonly query$ = this.store.select(selectMusiciansQuery);
  readonly activeMusician$ = this.store.select(selectActiveMusician);

  constructor(private readonly store: Store) {}
}

There are several drawbacks of this approach:

The size of the container component increases with the number of required state chunks.
Testing is harder - there can be many selectors to mock.
There are multiple subscriptions in the template.

Let's now create a view model selector for this container:

export const selectMusiciansPageViewModel = createSelector(
  selectFilteredMusicians,
  selectMusiciansQuery,
  selectActiveMusician,
  (musicians, query, activeMusician) => ({
    musicians,
    query,
    activeMusician,
  })
);

And the container now looks like this:

@Component({
  // single subscription in the template via `async` pipe
  // access to the view model properties via `vm` alias
  template: `
    <ng-container *ngIf="vm$ | async as vm">
      <musician-search [query]="vm.query"></musician-search>

      <musician-list
        [musicians]="vm.musicians"
        [activeMusician]="vm.activeMusician"
      ></musician-list>

      <musician-details
        [musician]="vm.activeMusician"
      ></musician-details>
    </ng-container>
  `,
})
export class MusiciansComponent {
  // select the view model
  readonly vm$ = this.store.select(selectMusiciansPageViewModel);

  constructor(private readonly store: Store) {}
}

The component is now smaller and easier for testing. Also, there is a single subscription in the template.

Treat actions as unique events

Treat NgRx actions as unique events, not as commands, and don't reuse them.

Commands can be fine for simple and isolated features. However, they can lead to dirty code and imply performance issues for complex functionalities that consume multiple feature states. Let's now walk through the example, to understand the importance of treating actions as unique events (a.k.a. good action hygiene).

There is a straightforward NgRx flow for pages that display a list of entities:

Dispatch the action to load the entity collection on component initialization.
Listen to this action in effect, load entities from the API, and return new action with loaded entities as a payload.
Create a case reducer that will listen to the action returned from the effect and add loaded entities to the state.
Finally, select entities from the store and display them in the template:

@Component(/* ... */)
export class SongsComponent implements OnInit {
  // select songs from the store
  readonly songs$ = this.store.select(selectSongs);

  constructor(private readonly store: Store) {}

  ngOnInit(): void {
    // dispatch the `loadSongs` action on component initialization
    this.store.dispatch({ type: '[Songs] Load Songs' });
  }
}

And this works fine. There is no need to change anything at first. However, what if we want to load another collection that is needed for a particular container component. In this example, imagine that we want to show the composer for each loaded song. If we treat actions as commands, then the ngOnInit method of SongsComponent will look like this:

ngOnInit(): void {
  this.store.dispatch({ type: '[Songs] Load Songs' });
  this.store.dispatch({ type: '[Composers] Load Composers' });
}

Here we come to another very important rule: Don't dispatch multiple actions sequentially. Sequentially dispatched actions can lead to unexpected intermediate states and cause unnecessary event loop cycles.

It would be much better to dispatch single action indicating that the user has opened the songs page, and listen to that action in both loadSongs$ and loadComposers$ effects:

ngOnInit(): void {
  this.store.dispatch({ type: '[Songs Page] Opened' });
}

"Songs Page" is the source of this action (it's dispatched from the songs page) and "Opened" is the name of the event (the songs page is opened).

This brings us to a new rule: Be consistent in naming actions, use "[Source] Event" pattern. Also, be descriptive in naming actions. It could help a lot in application maintenance, especially for catching bugs.

If we check the Redux DevTools for this example when actions are treated as unique events, we'll see something like this:

[Login Page] Login Form Submitted
[Auth API] User Logged in Successfully
[Songs Page] Opened
[Songs API] Songs Loaded Successfully
[Composers API] Composers Loaded Successfully

When we see a list of well-described actions, we can easily conclude what happened in our application:

The user submitted a login form.
Auth API responded that the login was successful.
The user opened the songs page.
Songs successfully loaded from the Song API.
Composers successfully loaded from the Composers API.

Unfortunately, this is not the case with commands:

[Auth] Login
[Auth] Login Success
[Songs] Load Songs
[Composers] Load Composers
[Songs] Load Songs Success
[Composers] Load Composers Success

Commands can be dispatched from multiple places, so we can't figure out what their source is.

Group actions by source

We saw in the previous example that one action can cause changes in multiple feature states. Therefore, do not group actions by feature state, but group them by source.

Create action file per source. Here are some examples of action files grouped by source:

// songs-page.actions.ts
export const opened = createAction('[Songs Page] Opened');
export const searchSongs = createAction(
  '[Songs Page] Search Songs Button Clicked',
  props<{ query: string }>()
);
export const addComposer = createAction(
  '[Songs Page] Add Composer Form Submitted',
  props<{ composer: Composer }>()
);

// songs-api.actions.ts
export const songsLoadedSuccess = createAction(
  '[Songs API] Songs Loaded Successfully',
  props<{ songs: Song[] }>()
);
export const songsLoadedFailure = createAction(
  '[Songs API] Failed to Load Songs',
  props<{ errorMsg: string }>()
);

// composers-api.actions.ts
export const composerAddedSuccess = createAction(
  '[Composers API] Composer Added Successfully',
  props<{ composer: Composer }>()
);
export const composerAddedFailure = createAction(
  '[Composers API] Failed to Add Composer',
  props<{ errorMsg: string }>()
);

// composer-exists-guard.actions.ts
export const canActivate = createAction(
  '[Composer Exists Guard] Can Activate Entered',
  props<{ composerId: string }>()
);

Don't dispatch actions conditionally

Don't dispatch actions conditionally based on the state value. Move the condition to the effect or reducer instead. This tip also relates to good action hygiene.

Let's first look at the case when an action is dispatched based on the state value:

@Component(/* ... */)
export class SongsComponent implements OnInit {
  constructor(private readonly store: Store) {}

  ngOnInit(): void {
    this.store.select(selectSongs).pipe(
      tap((songs) => {
        // if the songs are not loaded
        if (!songs) {
          // then dispatch the `loadSongs` action
          this.store.dispatch(songsActions.loadSongs());
        }
      }),
      take(1)
    ).subscribe();
  }
}

In the example above, the loadSongs action is dispatched if the songs have not already been loaded. However, there is a better way to achieve the same result but to keep the component clean. We can move this condition to the effect:

readonly loadSongsIfNotLoaded$ = createEffect(() => {
  return this.actions$.pipe(
    // when the songs page is opened
    ofType(songsPageActions.opened),
    // then select songs from the store
    concatLatestFrom(() => this.store.select(selectSongs)),
    // and check if the songs are loaded
    filter(([, songs]) => !songs),
    // if not, load songs from the API
    exhaustMap(() => {
      return this.songsService.getSongs().pipe(
        map((songs) => songsApiActions.songsLoadedSuccess({ songs })),
        catchError((error: { message: string }) =>
          of(songsApiActions.songsLoadedFailure({ error }))
        )
      );
    })
  );
});

Then, the component will look much cleaner:

@Component(/* ... */)
export class SongsComponent implements OnInit {
  constructor(private readonly store: Store) {}

  ngOnInit(): void {
    this.store.dispatch(songsPageActions.opened());
  }
}

Create reusable reducers

Use a single case reducer when multiple actions trigger the same state change:

export const composersReducer = createReducer(
  initialState,
  // case reducer can listen to multiple actions
  on(
    composerExistsGuardActions.canActivate,
    composersPageActions.opened,
    songsPageActions.opened,
    (state) => ({ ...state, isLoading: true })
  )
);

However, if any of these actions require a different state change, don't add additional logic to the existing case reducer as follows:

export const composersReducer = createReducer(
  initialState,
  on(
    composerExistsGuardActions.canActivate,
    composersPageActions.opened,
    songsPageActions.opened,
    (state, action) =>
      // `composerExistsGuardActions.canActivate` action requires
      // different state change
      action.type === composerExistsGuardActions.canActivate.type &&
      state.entities[action.composerId]
        ? state
        : { ...state, isLoading: true }
  )
);

Instead, create a new case reducer:

export const composersReducer = createReducer(
  initialState,
  on(
    composersPageActions.opened,
    songsPageActions.opened,
    (state) => ({ ...state, isLoading: true })
  ),
  // `composerExistsGuardActions.canActivate` action is moved
  // to a new case reducer
  on(
    composerExistsGuardActions.canActivate,
    (state, { composerId }) =>
      state.entities[composerId]
        ? state
        : { ...state, isLoading: true }
  )
);

Be careful with facades

I used facades as NgRx store wrappers before, but I stopped, and here are several reasons why:

If the Redux pattern is not your cup of tea and you have a need to wrap it in services, then you should take a look at service-based state management solutions such as Akita or NGXS (or use @ngrx/component-store for the global state as well).
Using facades doesn't make much sense when view model selectors are used and when good action hygiene is applied. You will have an extra layer for testing and maintenance, without any benefit.
Without strict rules in the coding guide, facades leave plenty of space for abuse (e.g. performing side effects).

However, if a container component has a local state but also uses a global state, then consider using the ComponentStore as a dedicated facade for that container. In that case, ComponentStore will manage the local state, but will also select global state slices and/or dispatch actions to the global store.

Effects Tips

Name effects like functions

Name the effects based on what they are doing, not based on the action they are listening to.

If we name the effect based on the action it listens to, it looks like this:

// the name of the effect is the same as the action it listens to
readonly composerAddedSuccess$ = createEffect(
  () => {
    return this.actions$.pipe(
      ofType(composersApiActions.composerAddedSuccess),
      tap(() => this.alert.success('Composer saved successfully!'))
    );
  },
  { dispatch: false }
);

There are at least two drawbacks of this approach. The first is that we cannot conclude what this effect does based on its name. The second is that it is not in accordance with open-closed principle - if we want to trigger the same effect for another action, we should change its name. However, if we name this effect as a function (showSaveComposerSuccessAlert), the previously mentioned drawbacks will be solved.

For example, if we want to display the same success alert when the composer is successfully updated, we only need to pass the composerUpdatedSuccess action to the ofType operator, without having to change the effect name:

// the effect name describes what the effect does
readonly showSaveComposerSuccessAlert$ = createEffect(
  () => {
    return this.actions$.pipe(
      ofType(
        composersApiActions.composerAddedSuccess,
        // new action is added here
        // the rest of the effect remains the same
        composersApiActions.composerUpdatedSuccess
      ),
      tap(() => this.alert.success('Composer saved successfully!'))
    );
  },
  { dispatch: false }
);

Keep effects simple

There are cases when we need to invoke multiple API calls to perform a side effect, or when the format of API response is not appropriate, so we need to restructure it. However, putting all that logic into the NgRx effect can lead to very unreadable code.

Here is an example of an effect that requires two API calls to get all the necessary data:

readonly loadMusician$ = createEffect(() => {
  return this.actions$.pipe(
    // when the musician details page is opened
    ofType(musicianDetailsPage.opened),
    // then select musician id from the route
    concatLatestFrom(() =>
      this.store.select(selectMusicianIdFromRoute)
    ),
    concatMap(([, musicianId]) => {
      // and load musician from the API
      return this.musiciansResource.getMusician(musicianId).pipe(
        // wait for musician to load
        mergeMap((musician) => {
          // then load band from the API
          return this.bandsResource.getBand(musician.bandId).pipe(
            // append band name to the musician
            map((band) => ({ ...musician, bandName: band.name }))
          );
        }),
        // if the musician is successfully loaded
        // then return success action and pass musician as a payload
        map((musician) =>
          musiciansApiActions.musicianLoadedSuccess({ musician })
        ),
        // if an error occurs, then return error action
        catchError((error: { message: string }) =>
          of(musiciansApiActions.musicianLoadedFailure({ error }))
        )
      );
    })
  );
});

This is large and unreadable effect, even with comments. However, we can move API calls to the service and make the effect more readable. The service method for getting the musician will look like this:

@Injectable()
export class MusiciansService {
  getMusician(musicianId: string): Observable<Musician> {
    return this.musiciansResource.getMusician(musicianId).pipe(
      mergeMap((musician) => {
        return this.bandsResource.getBand(musician.bandId).pipe(
          map((band) => ({ ...musician, bandName: band.name }))
        );
      })
    );
  }
}

It can be used from the loadMusician$ effect, but also from other parts of the application. The loadMusician$ effect now looks much more readable:

readonly loadMusician$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(musicianDetailsPage.opened),
    concatLatestFrom(() =>
      this.store.select(selectMusicianIdFromRoute)
    ),
    concatMap(([, musicianId]) => {
      // API calls are moved to the `getMusician` method
      return this.musiciansService.getMusician(musicianId).pipe(
        map((musician) =>
          musiciansApiActions.musicianLoadedSuccess({ musician })
        ),
        catchError((error: { message: string }) =>
          of(musiciansApiActions.musicianLoadedFailure({ error }))
        )
      );
    })
  );
});

If you're working with legacy APIs, you're probably having trouble with an API that doesn't return responses in the format your application needs, so you need to convert them. Apply the same principle described above: move the API call along with the mapping logic to the service method and use it from the effect.

Don't create "boiler" effects

Don't create effects that map multiple related actions into a single action:

// this effect returns the `loadMusicians` action
// when current page or page size is changed
readonly invokeLoadMusicians$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(
      musiciansPageActions.currentPageChanged,
      musiciansPageActions.pageSizeChanged
    ),
    map(() => musiciansActions.loadMusicians())
  );
});

// this effect loads musicians from the API
// when the `loadMusicians` action is dispatched
readonly loadMusicians$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(musiciansAction.loadMusicians),
    concatLatestFrom(() =>
      this.store.select(selectMusiciansPagination)
    ),
    switchMap(([, pagination]) => {
      return this.musiciansService.getMusicians(pagination).pipe(
        /* ... */
      );
    }) 
  );
});

Because the ofType operator can accept a sequence of actions:

readonly loadMusicians$ = createEffect(() => {
  return this.actions$.pipe(
    // `ofType` accepts a sequence of actions
    // and there is no need for "boiler" effects (and actions)
    ofType(
      musiciansPageActions.currentPageChanged,
      musiciansPageActions.pageSizeChanged
    ),
    concatLatestFrom(() =>
      this.store.select(selectMusiciansPagination)
    ),
    switchMap(([, pagination]) => {
      return this.musiciansService.getMusicians(pagination).pipe(
        /* ... */
      );
    }) 
  );
});

Apply single responsibility principle

In other words, don't perform multiple side effects within a single NgRx effect. Effects with single responsibility are more readable and easier to maintain.

Let's first see the NgRx effect that performs two side effects:

readonly deleteSong$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(songsPageActions.deleteSong),
    concatMap(({ songId }) => {
      // side effect 1: delete the song
      return this.songsService.deleteSong(songId).pipe(
        map(() => songsApiActions.songDeletedSuccess({ songId })),
        catchError(({ message }: { message: string }) => {
          // side effect 2: display an error alert in case of failure
          this.alert.error(message);
          return of(songsApiActions.songDeletedFailure({ message }));
        })
      );
    })
  );
});

If we apply the single responsibility principle, we'll have two NgRx effects:

// effect 1: delete the song
readonly deleteSong$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(songsPageActions.deleteSong),
    concatMap(({ songId }) => {
      return this.songsService.deleteSong(songId).pipe(
        map(() => songsApiActions.songDeletedSuccess({ songId })),
        catchError(({ message }: { message: string }) =>
          of(songsApiActions.songDeletedFailure({ message }))
        )
      );
    })
  );
});

// effect 2: show an error alert
readonly showErrorAlert$ = createEffect(
  () => {
    return this.actions$.pipe(
      ofType(songsApiActions.songDeletedFailure),
      tap(({ message }) => this.alert.error(message))
    );
  },
  { dispatch: false }
);

And here is another advantage: Effects with single responsibility are reusable. We can use the showErrorAlert$ effect for any action that requires an error alert to be shown.

Apply good action hygiene

The same principles described for actions that are dispatched via store should be applied to the effects:

Don't return an array of actions (commands) from the effect.
Return unique action that can be handled by multiple reducers and/or effects.

Let's first see an example where multiple actions are returned from the effect:

readonly loadAlbum$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(albumsActions.loadCurrentAlbum),
    concatLatestFrom(() => this.store.select(selectAlbumIdFromRoute)),
    concatMap(([, albumId]) => {
      return this.albumsService.getAlbum(albumId).pipe(
        // an array of actions is returned on successful load
        // then, `loadSongsSuccess` is handled by `songsReducer`
        // and `loadComposersSuccess` is handled by `composersReducer`
        mergeMap(({ songs, composers }) => [
          songsActions.loadSongsSuccess({ songs }),
          composersActions.loadComposersSuccess({ composers }),
        ]),
        catchError(/* ... */)
      );
    })
  );
});

I have seen similar effects many times. This happens when actions are treated as commands. You can see the drawbacks of this approach in the Treat actions as unique events section.

However, if we apply good action hygiene, the loadAlbum$ effect will look like this:

readonly loadAlbum$ = createEffect(() => {
  return this.actions$.pipe(
    // when the album details page is opened
    ofType(albumDetailsPageActions.opened),
    // then select album id from the route
    concatLatestFrom(() => this.store.select(selectAlbumIdFromRoute)),
    concatMap(([, albumId]) => {
      // and load current album from the API
      return this.albumsService.getAlbum(albumId).pipe(
        // return unique action when album is loaded successfully
        map(({ songs, composers }) =>
          albumsApiActions.albumLoadedSuccess({ songs, composers })
        ),
        catchError(/* ... */)
      );
    })
  );
});

Then, the albumLoadedSuccess action can be handled by the reducer(s) and/or other effects. In this example, it will be handled by songsReducer and composersReducer:

// songs.reducer.ts
export const songsReducer = createReducer(
  on(albumsApiActions.albumLoadedSuccess, (state, { songs }) => ({
    ...state,
    songs,
  }))
);

// composers.reducer.ts
export const composersReducer = createReducer(
  on(albumsApiActions.albumLoadedSuccess, (state, { composers }) => ({
    ...state,
    composers,
  }))
);

Conclusion

NgRx provides the ability to implement the same functionality in many different ways. However, some of the ways have emerged over time as best practices and you should consider applying them in your project to increase code quality, performance, and maintainability.

Resources

Good Action Hygiene with NgRx by Mike Ryan
Rethinking State in Angular Applications by Alex Okrushko
Building Sub States with NgRx Selectors by Brandon Roberts
Maximizing and Simplifying Component Views with NgRx Selectors by Brandon Roberts
Solving Angular Slow Renders with NgRx Selectors by Tim Deschryver
Start Using NgRx Effects for This by Tim Deschryver

Peer Reviewers

Big thanks to my teammates Brandon, Tim, and Alex for giving me helpful suggestions on this article!

Top comments (26)

James Moore • Feb 25

Wish I read this article years ago! Trying some of these techniques out gave me lots of aha moments this week. The biggest being naming actions like unique events wow... so this is what ngrx is supposed to look like. View model selectors also incredible. Anyone reading this go install the ngrx eslint plugin in your application wish I found it years ago as well.

Tomas Trajan 🇨🇭 • Apr 17 '22

Amazing article, it's always nice to see that many people reached the same conclusions! For me personally, I have came up with mostly same concepts as I used NgRx in multiple projects. Thank you for sharing!

Mikel Eizagirre • May 23 '22 • Edited

I love the article and I agree with most of the tips, but in my team we are discussing this case (simplified version):

We have a home page that loads some products.

When the Home Page is loaded it dispatches the action:

"[Home Page] Opened"

And we have this effect to handle it:

 loadProducts$ = createEffect(() => {
    return this.actions$.pipe(
      ofType(
        homePageActions.opened,
        ...
      ),
      exhaustMap(() => {
        return this.products.loadProducts().pipe(
          map(products => productsApiActions.productsLoadedSuccess({ products })),
          catchError((error) => of(productsApiActions.productsLoadedFailure({ error })))
        )
      })
    );
  });

The case is that we want to display a toastr message when the products can't be loaded.

We have a NotificationsModule to display messages and we want to use this effect:

  showErrorToastr$ = createEffect(() =>
    this.actions$.pipe(
      ofType(NotificationsActions.showError),
      tap(({ message }) => this.notifier.showError(message)),
    ),
    { dispatch: false }
  );

To map the failure with the show action we have:

  showLoadProductsError$ = createEffect(() => {
    return this.actions$.pipe(
      ofType(productsApiActions.productsLoadedFailure),
      map(({error}) => NotificationsActions.showError({ message: error.message }))
    )
  });

But with this approach we are not following thesebest practices:

Treat NgRx actions as unique events, not as commands.
Don't create "boiler" effects.

To follow both of these, we need to change the effect:

  showErrorToastr$ = createEffect(() =>
    this.actions$.pipe(
      ofType(
          ...
          productsApiActions.productsLoadedFailure),
      tap(({ message }) => this.notifier.showError(message)),
    ),
    { dispatch: false }
  );

But this way, NotificationsModule need to know every failure action from other modules and we don't want to do that. What would be the best way to do that?

Hamza Ahmed • Sep 7 '23

Create an intermediary action that represents "an error has occurred." Your NotificationsModule only listens for this action. Other parts of the application dispatch this action when an error occurs.

For example, you could create a generic ApplicationActions.errorOccurred action:

export const errorOccurred = createAction(
  '[Application] Error Occurred',
  props<{ message: string }>()
);

Then, your showLoadProductsError$ effect would be:

showLoadProductsError$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(productsApiActions.productsLoadedFailure),
    map(({error}) => ApplicationActions.errorOccurred({ message: error.message }))
  );
});

And your showErrorToastr$ effect would listen for ApplicationActions.errorOccurred:

showErrorToastr$ = createEffect(() =>
  this.actions$.pipe(
    ofType(ApplicationActions.errorOccurred),
    tap(({ message }) => this.notifier.showError(message)),
  ),
  { dispatch: false }
);

Wojciech Trawiński • Jul 6 '22

Wow, great that not only me have some concerns about it :)
Most of my colleagues who are very experienced developers also disagree with it.
You can also take a look at my discussion with the author of the following blog post tomastrajan.medium.com/level-up-yo...
IMO there's a place for both commands and events and keeping modules isolated is far more important than having beautiful logs in the NgRx devtools :)

Mikel Eizagirre • Jul 7 '22

Hi @wojtrawi ,

I raised the same question on github and liked the idea of using metadata in actions. It can be useful in many cases.

If you want to take a look here is the link: github.com/ngrx/platform/discussio...

Wojciech Trawiński • Feb 12 '22

Great article!
Seems like I've been following most practices even before reading it :D
However, naming effects like functions is new to me, but I really like it +1
When it comes to treating actions as events instead of commands, there is one thing which puzzles me. Imagine you have a global piece of state which e.g. keeps feature flags for a given user. Now, if I want to refresh (load once again) the feature flags data I need to update the effect within a feature flags module/library every time there is new trigger for it (e.g. SavePreferencesSuccess action) . Doesn't it break the FF modules/library encapsulation? IMO a more natural approach is to dispatch a generic LoadFeatureFlags action (which is a part of the public API of the FF module/lib) from the effect belonging to a given feature (e.g. preferences effect).

Wojciech Trawiński • May 6 '22

@markostanimirovic any thoughts on that? Isn't editing reducers and/or effects of a given encapsulated module against the open-closed rule? What about shared libraries via a private npm. I remember working on the project which used a shared library for handling authentication. The library added the auth chunk to the global store and e.g. provided a possibility to dispatch an action resulting in refreshing an access token. Following the advice on using events instead of command I would need to edit the source code of the library, since the source of action in an application using the library is not known beforehand.

Marko Stanimirović • May 6 '22

Like any solution/principle, good action hygiene also has pros and cons. I agree that it can violate the open-closed principle. However, it brings many benefits in terms of maintaining and upgrading the applications that are explained in this article.

In the case of auth library that is shared via private registry, I'd not expose any actions for dispatching. It can use NgRx flow internally and expose actions that can be listened to, but its side effects/state updates could be triggered from the outside world by using service(s). The similar situation is when you use @ngrx/router-store - it exposes actions that can be listened to, but side effects/state changes (navigation for example) are handled using the Angular Router (service). So you'll keep auth library encapsulated and still be able to apply good action hygiene in applications that use it.

Wojciech Trawiński • May 7 '22

Thanks for your reply.
In the case of the auth library, what you proposed is just not using NgRx at all for parts which would require generic actions. When talking about @ngrx/router-state, based on interaction via the Router instance it dispatches generic actions (so against good action hygiene). I do see some benefits of favoring actions as events over actions as commands, but I think that there's a place for both (see here for details blog.nrwl.io/ngrx-patterns-and-tec...).
Since, you seem to be a big fan of the actions as events approach I still have some questions in my mind, so let me ask them:
1) If you dispatch an action which is conditionally mapped to another action in an effect, what will be the source of an action in square brackets? The same as for the source action or sth else indicating it's from an effect?
2) Consider a single Angular app and a store chunk responsible for displaying notifications. It's an app wide module so it can be considered as a core one. Actions triggering notifications can be dispatched e.g. from Cars and Planes pages. Where would you define a file with CarsPage and PlanesPage actions containing actions responsible for triggering notifications? Under a feature module directory (then the Notifications module under core directory imports from the CarsPage/PlanesPage actions, since it needs to handle these actions in the notifications reducer) or under Notifications module directory (then a corresponding feature module imports the actions from there).
3) When using the Nx tool (even for a single app), the Notifications would be a shared library. Shared libs cannot access libs with a different scope by default, hence defining actions as a part of e.g. Cars data-access lib is not an option. Would you create an exception for the rule or define the actions as a part of the shared Notifications lib?

Marko Stanimirović • Jul 2 '22

1) If you dispatch an action which is mapped (conditionally or not) to another action you are not dispatching unique events, but commands (see "Don't create boiler effects" section).
2) With good action hygiene, you describe an event, not command (what particular action should trigger). Therefore, all actions (events) from "Cars Page" (or "Planes Page") should be in the same file (see "Group actions by source" section). Single action can trigger multiple effects/reducers.
2) 3) Take a look at this article: github.com/ngrx/platform/discussio.... With this approach, you can apply good action hygiene and not break encapsulation. The same can be applied for auth library.

Wojciech Trawiński • Jul 6 '22

So you say that Victor Savkin is wrong in his article and there should only exist event actions, right?
1) I do not understand why such mapping is considered by you as a bad practice. For example, triggering data loading only when it's not in the store IMO is far more clear when you have an effect which conditionally dispatches LoadData action (command, I know).
2) Ok, so at the level of a feature module I keep actions related to e.g. Cars Page and I import them in every other feature/core module which need to be aware of a given action related to Cars Page, e.g. Cars Page Opened should trigger action which results in showing notification, so core Notifications module imports actions from a feature module. That sounds crazy. I had a discussion with @tomastrajan and he suggested to move such actions under core scope (tomastrajan.medium.com/level-up-yo...). However, it looks odd to keep under core scope actions like cars-api.actions such because e.g. I want to show a notification after API response.
3) It gives implicit solution which is a hacky way just to stick at all costs with some practices that simply do not fit all the scenarios.

Marko Stanimirović • Jul 6 '22

I didn't say anybody was wrong. Don't do that, please.

My previous answers were about how I would handle scenarios that you described with good action hygiene applied because I thought you asked me exactly that:

Since, you seem to be a big fan of the actions as events approach I still have some questions in my mind, so let me ask them

Then you rephrased my answers the way you wanted them to sound, and said they were wrong or crazy, or even worse that I said someone else was wrong.

Keep using the commands if they work better in your projects. It's great that we have more options, so everyone can choose what suits them best!

Have a nice day, Wojciech! :)

Alban X • Jul 23 '23 • Edited

Really great guide, well done. With this tips the power of NGRX becomes even more powerful.

One Tip that I would add is: Avoid nesting in effects:

Instead of this:

readonly deleteSong$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(songsPageActions.deleteSong),
    concatMap(({ songId }) => {
      return this.songsService.deleteSong(songId).pipe(
        map(() => songsApiActions.songDeletedSuccess({ songId })),
        catchError(({ message }: { message: string }) =>
          of(songsApiActions.songDeletedFailure({ message }))
        )
      );
    })
  );
});

do this:

  readonly deleteSong$ = createEffect(() =>  this.actions$.pipe(
    ofType(songsPageActions.deleteSong),
    concatMap(({ songId }) => this.songsService.deleteSong(songId)),
    map(() => songsApiActions.songDeletedSuccess({ songId })),
    catchError((error) =>  of(songsApiActions.songDeletedFailure({ message }))),
  )

Tim Hardy • Jul 11 '22

Thank you for this article. I've come to almost all the same conclusions as you have, and I just now found your article. I have one question though, that I haven't found a clean/elegant answer to. While I agree that conditionally loading data should be done in an effect, and not in a component, this makes handling loading flags more difficult. I see two options for handling loading flags - either add the same logic in the effect to the reducer as well, or somehow dispatch a new action that says the data is definitely loading (I haven't found a clean way to do this).

I'm speaking specifically to the scenario where we dispatch something like "MemberActions.memberListComponentInitialized" where an effect conditionally loads Members based on whatever. You can't have a reducer listening to that same action which automatically sets a loading flag because the memberListComponentInitialized action may or may not actually trigger loading that data.

That is the last discomfort I've seen with this approach, and I'd love to hear your solution.

Tim Hardy • Oct 6 '22 • Edited

In case you visit these comments again, let me spell out the issue with the code you show above and loading flags. I'm still curious how you actually deal with those in your real code.

let's assume the songs are already loaded, and the user navigates to the songsPage
you dispatch songsPageActions.opened when the songs page is opened
the reducer for songsPageActions.opened sets isLoading = true
any spinny loading gifs subscribed to isLoading start spinning - telling the user that songs are being loaded
an effect handling songsPageActions.opened gets stopped at the filter... filter(([, songs]) => !songs), and neither songsApiActions.songsLoadedSuccess nor songsApiActions.songsLoadedFailure gets dispatched, therefore isLoading never gets set to false. I'm assuming the reducers for both of the above actions are where you set isLoading = false
the loading spinny gifs continue to spin... forever... and ever... because they never know when the data is loaded

You cannot use the songsPageActions.opened to set isLoading = true because the effect that actually calls an api to load the songs CONDITIONALLY calls that api to load the songs. Without the same condition in your reducer (yuck) you don't know if songs are actually being loaded.

Once again, I like everything you evangelize in your post here, but I have no answer (and I don't see you presenting a working answer) to handling loading flags.

Marko Stanimirović • Oct 12 '22

Hi Tim!

You can do something like this:

export type LoadState = 'INIT' | 'LOADING' | 'LOADED' | { errorMsg: string };

// songs.reducer.ts

interface State {
  songs: Song[];
  loadState: LoadState;
}

const initialState: State = {
  songs: [],
  loadState: 'INIT',
};

export const reducer = createReducer(
  initialState,
  on(SongsPageActions.opened, (state) => state.loadState === 'LOADED'
    ? state
    : { ...state, loadState: 'LOADING' }
  ),
  on(SongsApiActions.songsLoadedSuccess, (state, { songs }) => ({
    ...state,
    songs,
    loadState: 'LOADED',
  }),
  on(SongsApiActions.songsLoadedFailure, (state, { errorMsg }) => ({
    ...state,
    loadState: { errorMsg },
  })
);

// songs-api.effects.ts

readonly loadSongsIfNotLoaded$ = createEffect(() => {
  return this.actions$.pipe(
    ofType(SongsPageActions.opened),
    concatLatestFrom(() => this.store.select(songsSelectors.selectLoadState)),
    filter(([, loadState]) => loadState !== 'LOADED'),
    exhaustMap(() => {
      return this.songsService.loadSongs().pipe(
        map((songs) => SongsApiActions.songsLoadedSuccess({ songs })),
        catchError((error) =>
          of(SongsApiActions.songsLoadedFailure({ errorMsg: error.message }))
        )
      );
    })
  );
});

Additionaly, if you have a complex condition that is repeated in the effect as well as in the reducer, you can move it to a helper function and reuse it in both places.

Tim Hardy • Oct 13 '22

Thanks a lot! I'm going to try this. I like the idea of making the condition something reusable. I also have come to the conclusion that I don't actually need these conditions as often as I thought. In reality, I rarely stop the loading of something because it's already loaded. The actions that cause something to load almost always cause them to load. On the rare occasion I really need a conditional load, this sounds like a clean way to handle it.

Tim Hardy • Aug 10 '22

Still hoping for some experienced advice on this. Everything you've written here resonates with me, and seems very elegant for all the purposes of software maintenance. I'm just having trouble coming up with an elegant solution to loading flags. It seems like a bad idea to force logic into reducers, logic that should only be inside effects, in order to figure out if a particular action is actually triggering the loading of specific data. I also cannot figure out how to deterministically communicate via the store that data is definitely loading, but I think this is probably possible.

Marko Stanimirović • Oct 12 '22

dev.to/markostanimirovic/comment/2...

Günter Zöchbauer • Nov 11 '21

Enabling NgRx eslint rules pokes you in they eye when violating several of these rules, so strong recommend.

Chandler Baskins • Nov 11 '21

This is an article you keep coming back to. Well done 🎉

Günter Zöchbauer • Nov 11 '21

Great writeup!
I still prefer facades over viewmodel selectors. I want components as lean as possible and don't want them to know about the used state management approach.

Samiullah Khan • Feb 18 '22

Łukasz Ruebbelke mentioned it in one of his frontend master course.

Imagine introducing ngrx to medium to complex app, where there are developers of different of experience. A facade would make it easy to jell team A (that works on components side) and team B (that works on ngrx intricacies)

Arcadio Quintero • Nov 10 '21

Excelent article.

View full discussion (26 comments)

Some comments may only be visible to logged-in visitors. Sign in to view all comments.

DEV Community