Platform: Refactor Example App with New Best Practices and a Styleguide

[MikeRyanDev]

Enhancements:

• Developers have been asking for _years_ now that we update the example app to be production quality. We've adopted the official Angular styleguide but there's a lot more from our internal styleguide that we could make public.
• We should reevaluate the recommended approach to writing Actions. Classes work well but they incentivize the wrong behaviors. We see developers breaking serialization with classes and putting too much functionality in them. Internally, we've gone back to interfaces+factory functions. We might be able to get away with just type unions + factory functions since the mapped type capabilities in TypeScript 2.8 remove the need to independently reference an action interface.
• Remove all third party libraries from the example app. If we are baking in state persistence and development time checks into Store then this shouldn't be too painful.
• Remove @ngrx/db
• Make it build with Bazel. We should still support the Angular CLI build process but it would be a good sanity check to make sure it also builds with ABC.

[lanovoy]

Hey Mike! Could you suggest please any public examples of production quality application with those recommended practices implemented? We're just starting with ngrx/store and quite struggling with the boilerplate code and a lot of repetitions... I'm afraid that it may quickly become big and ugly and hard to maintain with the app grow...

[jotatoledo]

Another point that I would personally like to see would be an alternative approach to the use of guards to block navigation and prefetch single elements on router transitions. IMO would be nicer to trigger prefetch through an effect, withouth the necessity of blocking navigation until the prefetch is complete/failed.

An alternative is described in this post. Maybe you have your own in mind?

[michaeljota]

Hi Mike,
I write a proposal for ngrx using ducks as a base, but trying to follow the Angular Style Guide.

michaeljota/ngrx-ducks

I notice that most of the Style Code and code samples here are based on a folder-by-type organization. I think this is because most of the Redux code are made in this way. Angular Style Guide suggest the use of a folder-by-feature organization, so I tried to apply it with ngrx.

[wiredprogrammer]

Love the example-app. It is a great resource of ideas for project setup and ways to use ngrx. I know the team is busy working on the next version and ideas for ways to ease the implementation boilerplate but if you get time I'd love to see more than one example and possibly an example of something a little more complex ( around double the example apps amount of reducers). Or if you don't believe it necessary then maybe you can add a link in the documentation to other examples for other redux implementations that are sources of inspiration for the work you've done on the example app. The example app is a great starting point for those learning but a more complex example would be great for those already head deep into larger projects. I know I'm looking for a good way to set things up and keep my apps inline with this example app which is sort of ngrx style guide for the community right now. Thanks for all the hard work!!

[metaloha]

I'd love to see this as well. Just a bog-standard Angular[CLI]+NGRX project with at least 2 or 3 modules/components and using the recommended best-practices from both projects would be great.

[ahnpnl]

I vote for more intensive unit test examples and more about change detection strategy

[maxkuzmin]

@michaeljota using ducks is not recommended by Dan Abramov https://github.com/pitzcarraldo/reduxible/issues/8

[michaeljota]

@maxkuzmin Thanks. I didn't know about that. But most of the applications I work implements some kind of ducks.

However, what he says is that you may find yourself looking for a reducer, or an action that affect no only a single component, but multiples. This fall under the Angular concept or shared, and it should be placed at the level you are sharing it, eg: you are sharing a reducer in your feature module, then you probably would have a shared folder in that module. If you are sharing an action in your app, that should be handle differently according the stores, you should have it in your CoreModule.

This is not something about ducks or ngrx-ducks proposals, but sure Angular structure and Style Guide helps to grow up with less mistakes.

[dezoxel]

Please take a look at the Redux Actions as a good example for reducing actions boilerplate. From what I've seen so far it looks most solid and mature. I would happy to see something similar.

[lanovoy]

@dezoxel good one! We're using wonderful https://github.com/pelotom/unionize library.

[jcano]

As this refactoring is going to take a while and, not being new features or improvements, it could get postponed, are there any good examples or tutorials out there that show how it should be done?

I'm familiar with the old structure and practices, but I'm starting a new project and would be cool to "future-proof" it. I'm also happy to take on this ticket, if no one already has.

[maxkuzmin]

Please consider using NOUN_VERB (BookAdd, BookAddSuccess) structure of actions instead of currently being used VERB_NOUN (AddBook, AddBookSuccess). That is recommended by some unofficial
redux styleguides: https://medium.com/@kylpo/redux-best-practices-eef55a20cc72, https://gist.github.com/datchley/4e0d05c526d532d1b05bf9b48b174faf and as for me it makes sense.

[pablolazaro]

@MikeRyanDev When you talk about going back with type unions + factory functions you mean something like this?:

user.actions.ts

export interface Action {
  type: string;
}

export interface User {
  id: string;
}

export enum UserActionTypes {
  LOAD_USERS = '[User] Load Users',
  ADD_USER = '[User] Add User',
  UPSERT_USER = '[User] Upsert User'
}

export interface LoadUsersAction extends Action {
  type: UserActionTypes.LOAD_USERS;
  users: Array<User>;
}

export interface AddUserAction extends Action {
  type: UserActionTypes.ADD_USER;
  user: User;
}

export interface UpsertUserAction extends Action {
  type: UserActionTypes.UPSERT_USER;
  user: User;
}

export type UserActionsUnion =
  LoadUsersAction | 
  AddUserAction |
  UpsertUserAction;

export function loadUsers(users: Array<User>): LoadUsersAction {
  return {
    type: UserActionTypes.LOAD_USERS,
    users
  }
}

export function addUser(user: User): AddUserAction {
  return {
    type: UserActionTypes.ADD_USER,
    user
  }
}

export function upsertUser(user: User): UpsertUserAction {
  return {
    type: UserActionTypes.UPSERT_USER,
    user
  }
}

user.reducer.ts

export function reducer(state: any, action: UserActionsUnion): any {
  switch (action.type)  {
    case UserActionTypes.ADD_USER:
    case UserActionTypes.LOAD_USERS:
    case UserActionTypes.UPSERT_USER:
      return state; // Just an example
  }
}

Thank you in advance

[michael-lang]

I've rewritten the books sample app following the angular style guide more closely. The store code has reduced boilerplate as well, defining actions/reducers as classes without losing serializability of store actions.

https://github.com/michael-lang/book-sample-ngrx

Let me know if you see any deviations from the angular style guide, in that repo's issue list.

[jotatoledo]

@michael-lang what would you think about adding https://github.com/mweststrate/immer to your fork in order to reduce the use of the spread operator?

[michael-lang]

@jotatoledo, Immer looks interesting. I'm only proposing one change to ngrx with this repo. I don't want it to appear that this new one can only be adopted along with any other proposal. But feel free to fork my repo and add Immer to showcase how it works.

[alex-okrushko]

I second the folder-by-feature organization, as recommended by Angular Style Guide.
E.g. this is what I have for the workshop app.

[savaryt]

Can someone clarify what upsert means please ?

[Stephane-Peyroutet]

@savaryt Upsert is a shortcut for Update or Insert.
It means that your data is either updated (if it exists) or inserted (if it does not exist).

I really do like this issue and to gather up some the best ideas from @michael-lang example of even from the Angular ngrx-data project (even if their approach is somewhat different).

It's not the so infamous boil*plat (sorry for that one @brandonroberts :-) that itches me but much more how we could best use the store features using DRY principle.

Waiting for updates on this one.

[michaeljota]

I still like mine better. :P. Just saying.

[wiredprogrammer]

I was reading through the current example app and trying my best to understand how you were using containers versus components. I had assumed containers were pages initially but looking closer it appeared to be more smart versus presentation components. I did some googling and finally I think I got where you've gotten at least parts of your layout setup from.
https://redux.js.org/basics/usagewithreact#implementing-container-components

I hope when a style guide is written some of the reasoning behind layout will be documented to be clearer. I've been trying to help teach some of this to my own programming team and I've kept coming back to the example app for layout inspiration and struggling a little on some small parts about when to do what and how. I hope the link helps others.

[MattiJarvinen]

@jotatoledo https://github.com/mweststrate/immer#performance immer is nice and all if you have only a small dataset. For my case I have hundreds of objects, I'd prefer good examples of object spread and a mention about immer & etc and caveats of using those.

[mhamel06]

I think the actions could be simplified and boilerplate reduced if they are refactored like Martin Hochel did this Redux blog post: https://medium.com/@martin_hotell/improved-redux-type-safety-with-typescript-2-8-2c11a8062575

I don't know if action type enums should be used, but this solution works with or without them and also maintains type safety across reducers and effects.

Ex. From:

/**
 * Add Book to Collection Actions
 */
export class AddBookSuccess implements Action {
  readonly type = CollectionApiActionTypes.AddBookSuccess;

  constructor(public payload: Book) {}
}

export class AddBookFailure implements Action {
  readonly type = CollectionApiActionTypes.AddBookFailure;

  constructor(public payload: Book) {}
}

/**
 * Remove Book from Collection Actions
 */
export class RemoveBookSuccess implements Action {
  readonly type = CollectionApiActionTypes.RemoveBookSuccess;

  constructor(public payload: Book) {}
}

export class RemoveBookFailure implements Action {
  readonly type = CollectionApiActionTypes.RemoveBookFailure;

  constructor(public payload: Book) {}
}

/**
 * Load Collection Actions
 */
export class LoadBooksSuccess implements Action {
  readonly type = CollectionApiActionTypes.LoadBooksSuccess;

  constructor(public payload: Book[]) {}
}

export class LoadBooksFailure implements Action {
  readonly type = CollectionApiActionTypes.LoadBooksFailure;

  constructor(public payload: any) {}
}

export type CollectionApiActionsUnion =
  | AddBookSuccess
  | AddBookFailure
  | RemoveBookSuccess
  | RemoveBookFailure
  | LoadBooksSuccess
  | LoadBooksFailure;

To:

export const Actions = {
  /**
   * Add Book to Collection Actions
   */
  addBookSuccess: (payload: Book) =>
    createAction(CollectionApiActionTypes.AddBookSuccess, payload),
  addBookFailure: (payload: Book) =>
    createAction(CollectionApiActionTypes.AddBookFailure, payload),

  /**
   * Remove Book from Collection Actions
   */
  removeBookSuccess: (payload: Book) =>
    createAction(CollectionApiActionTypes.RemoveBookSuccess, payload),
  removeBookFailure: (payload: Book) =>
    createAction(CollectionApiActionTypes.RemoveBookFailure, payload),

  /**
   * Load Collection Actions
   */
  loadBooksSuccess: (payload: Book[]) =>
    createAction(CollectionApiActionTypes.LoadBooksSuccess, payload),
  loadBooksFailure: (payload: any) =>
    createAction(CollectionApiActionTypes.LoadBooksFailure, payload),
};

export type Actions = ActionsUnion<typeof Actions>;

Refactored with the example app: https://github.com/ngrx/platform/compare/master...mhamel06:action-refactor?expand=1

[mhamel06]

Looking at the latest createAction function and other helpers in NgRx 8, my comment above can be disregarded.

I do have a best practice question about selectors that I think could be addressed a little more clearly in the example app. For naming selectors, when should the get prefix be used instead of the select prefix?

They seem to be used interchangeably in the example app. For my own apps, I have gone with 'select' from current state, and get from other state.

Another option, which seems to be what is done in the current example app, is to select objects and get values. Anyway, clarification on the subject could be helpful for developers looking at the example app as a guide.

e.g. https://github.com/ngrx/platform/blob/f954e147e505393ce4f88a81c466b5a1a08313ef/projects/example-app/src/app/auth/reducers/index.ts#L27-L44
e.g.

[michael-lang]

@mhamel06 I use the 'get' prefix for pure functions that feed selectors, and 'select' prefix for selectors. A selector is the result of 'createFeatureSelector' or 'createSelector'. I thought at one point the ngrx sample followed this guideline. It may have deviated since it was not a documented standard.

// pure function => the 'get' prefix
export const getShowSidenav = (state: LayoutState) => state.showSidenav;

// selector - result of createFeatureSelector => 'select' prefix
export const selectLayoutState = createFeatureSelector<LayoutState>('layout');

// selector - result of createSelector => 'select' prefix
export const selectShowSidenav = createSelector(
  selectLayoutState,
  getShowSidenav
);

https://github.com/michael-lang/book-sample-ngrx/blob/master/src/app/core/store/layout.store.selectors.ts

[mhamel06]

@michael-lang Thanks for the reply. I think I do remember the older examples following this pattern and it feels like a good distinction.

[brandonroberts]

We are going to revisit this with some new context. Closing this one for now