DeborahK / Angular-NgRx-GettingStarted

Materials for our Pluralsight course: Angular NgRx: Getting Started: http://bit.ly/AngularNgRx.
MIT License
547 stars 722 forks source link

why ProductEditComponent does not have OnPush ? #46

Closed testcs12345 closed 3 years ago

testcs12345 commented 3 years ago

looks like ProductEditComponent is a presentational component as it only has the Input and Output Decorator to talk to parent component and it does not have any NgRx related code which is very pure component.

DeborahK commented 3 years ago

As the ProductEditComponent currently stands, it can't be OnPush because of the way it handles the validation messages. Those are done with direct assignments. Specifically displayMessage.

The validation would need to be modified before it could support OnPush change detection.

testcs12345 commented 3 years ago

@DeborahK ok, thx, if I add markForCheck( ) before https://github.com/DeborahK/Angular-NgRx-GettingStarted/blob/master/APM-Demo5/src/app/products/product-edit/product-edit.component.ts#L69 can it support OnPush ?

btw, as far as I remember, you mentioned something like reducer replaces the whole exiting state and generates a new state instead of mutating the state thus to keep it immutable .

but for the https://github.com/DeborahK/Angular-NgRx-GettingStarted/blob/master/APM-Demo5/src/app/products/state/product.reducer.ts ,

the state here is the ProductState which is part of the AppState. if AppState is the big object, the ProductState is a property of this big object, your reducer here is just replacing the ProductState property here and not replacing other direct property of the big object. Thus it is not replacing the whole state, instead, it just changes part of it and that is kind of mutating the object , which violates the immutability principle. please correct me if I am wrong.

duncanhunter commented 3 years ago

@bakkiung the NgRx library manages the aggregated state and state history for you so you do not need to say update my slice of state and then now update the whole state tree. The library will handle making a new copy of state and also the history of the previous state. This is a cool article describing reducers and scans in RxJS that describe how these work in relation to NgRx you might like https://gist.github.com/btroncone/a6e4347326749f938510#whats-a-reducer.

In many of the large NgRx apps you inevitable have places where you need to use ChangeDetectorRef.markForCheck() but best to use it pragmatically and look for a way to refactor and avoid it. In this case you can try adding ChangeDetectorRef.markForCheck() but sometimes if the component is working in a less reactive way or you are refactoring an existing app to use NgRx it might be better to leave off onPush and likely you will not be able to see a performance difference.