refactor(crud): simplify template logic, adjust actions, reducers, ...

[tomastrajan]

Hi @timdeschryver & @Jeykairis !

I had a look at the new CRUD feature and simplified / adjusted couple of things. Please have a look and let me know if you see some more opportunities to improve :)

Biggest changes:

1. move logic from crud.component template into the component itself
2. simplify component logic
3. adjust action creators to always come with readonly payload
4. move persistence logic into @Effect based on Tims article

Cheers!

[Jeykairis]

It looks cleaner as is even if we subscribe to the selectedBook. What do you think @timdeschryver ?

Thank you for the refactoring and make the effect works as I wanted! And following the article we could write it the short way persistBooks = this.actions$.pipe(.

I would just add some space between buttons and the footer with :

.col-md-6 {
  margin-bottom: 20px;
}

[timdeschryver]

What's your opinion on using an observable selectedBook vs a "normal" selectedBook object @tomastrajan ?

this.store
      .pipe(select(selectSelectedBook), takeUntil(this.unsubscribe$))
      .subscribe(book => (this.selectedBook = book));

[tomastrajan]

What's your opinion on using an observable selectedBook vs a "normal" selectedBook object @tomastrajan ?

this.store
      .pipe(select(selectSelectedBook), takeUntil(this.unsubscribe$))
      .subscribe(book => (this.selectedBook = book));

Hi @timdeschryver!

This one in particular is because of the following reasons:

1. I like to keep template logic at minimum
2. Multiple subscription to the (something$ | async)?.prop somehow feel wrong
3. Using that object / property more leads to wrappers like *ngIf="(something$ | async) as value
4. I use the "normal" selectedBook in the component methods which relates to the point number 1 when I prefer to not implement and pass this from the template like (click)="componentMethod(unwrapedObservable)" but rather do simple (click)="componentMethod()"

With all these things combined it looked simpler to just subscribe and store reference once and then use it in both template and component methods.

This is actually a recurring question I keep getting and keep thinking about so would appreciate all feedback!

[timdeschryver]

For me I prefer working with the observable, but will usually create a dumb component, accepting the object instead of an observable.

<book-form [book]="selectedBook$ | async "></book-form>

[tomastrajan]

@timdeschryver that's a very good point. I have been thinking about approach to big templates vs more smaller components for some time now too. I guess more components are more "Angular" way and also promote better reuse.

What is your opinion about the use of these unwrapped objects in the component methods? Eg in our example the use of unwrapped selectedBookId in the multiple component methods? The only thing I can think of is using of | async pipe in template and passing of the unwrapped object to the method form the template as parameter, but that kinda splits logic into two places?

[timdeschryver]

The dumb component will have an eventemitter:

@Input() selectedId: number;
@Output() selectBook = new EventEmitter<number>();
...
this.selectBook (this.selectedId);

[timdeschryver]

I'm not sure what you with "but that kinda splits logic into two places". I usually don't put any (or much) logic in dumb components, they usually only are for user interactions.