DEV Community

Cover image for NgRx Tips I Needed in the Beginning

NgRx Tips I Needed in the Beginning

Marko Stanimirović on November 10, 2021

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 libr...
Collapse
 
jamesnm profile image
James Moore

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.

Collapse
 
tomastrajan profile image
Tomas Trajan 🇨🇭

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!

Collapse
 
mikeleiza profile image
Mikel Eizagirre • 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 })))
        )
      })
    );
  });
Enter fullscreen mode Exit fullscreen mode

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 }
  );
Enter fullscreen mode Exit fullscreen mode

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 }))
    )
  });
Enter fullscreen mode Exit fullscreen mode

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 }
  );
Enter fullscreen mode Exit fullscreen mode

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?

Collapse
 
zokizuan profile image
Hamza Ahmed

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 }>()
);
Enter fullscreen mode Exit fullscreen mode

Then, your showLoadProductsError$ effect would be:

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

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 }
);
Enter fullscreen mode Exit fullscreen mode
Collapse
 
wojtrawi profile image
Wojciech Trawiński

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 :)

Collapse
 
mikeleiza profile image
Mikel Eizagirre

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...

Collapse
 
wojtrawi profile image
Wojciech Trawiński

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).

Collapse
 
wojtrawi profile image
Wojciech Trawiński

@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.

Collapse
 
markostanimirovic profile image
Marko Stanimirović

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.

Thread Thread
 
wojtrawi profile image
Wojciech Trawiński

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?

Thread Thread
 
markostanimirovic profile image
Marko Stanimirović

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.

Thread Thread
 
wojtrawi profile image
Wojciech Trawiński

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.

Thread Thread
 
markostanimirovic profile image
Marko Stanimirović

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! :)

Collapse
 
thardy profile image
Tim Hardy

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.

Collapse
 
thardy profile image
Tim Hardy • 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.

Collapse
 
markostanimirovic profile image
Marko Stanimirović

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 }))
        )
      );
    })
  );
});
Enter fullscreen mode Exit fullscreen mode

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.

Thread Thread
 
thardy profile image
Tim Hardy

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.

Collapse
 
thardy profile image
Tim Hardy

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.

Collapse
 
markostanimirovic profile image
Marko Stanimirović
Collapse
 
albanx0 profile image
Alban X • 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 }))
        )
      );
    })
  );
});
Enter fullscreen mode Exit fullscreen mode

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 }))),
  )
Enter fullscreen mode Exit fullscreen mode
Collapse
 
zoechi profile image
Günter Zöchbauer

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.

Collapse
 
technbuzz profile image
Samiullah Khan

Ł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)

Collapse
 
zoechi profile image
Günter Zöchbauer

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

Collapse
 
chandlerbaskins profile image
Chandler Baskins

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

Collapse
 
oidacra profile image
Arcadio Quintero

Excelent article.

Collapse
 
davidshortman profile image
David

Nailed it