Redux section rewrite #864
Conversation
|
@bennett000 Yep, everything to do with ng2-redux (other than the mention of libraries) has been removed. I remember I asked during a standup if it made sense to transition it all to ngrx since ngrx will be the standard going forward seeing as this is what google is supporting, and you had told me yes :) So poof! It's gone, LOL |
|
@egervari I do recall that. I don't think I took the time to understand the ramifications of that choice. Sorry for my confusion. In retrospect It feels unfair to our ng2-redux efforts not to cover it. Although if we're going to pivot to a cookbook format that would be irrelevant anyway. Let's talk at/after standup. I know David had some ideas about the the book in general. |
Conflicts: handout/redux/README.md handout/redux/configuring_your_application_to_use_redux.md handout/redux/redux_actions.md handout/redux/using_redux_with_components.md
|
tl;dr: I agree with both :) I agree with @egervari in the sense that I do think that ngrx is the "standard" solution for redux in Angular and we should cover that in detail. But I also agree with @bennett000 in that we shouldn't remove ng2-redux from the book. ng2-redux is valid alternative with a different value proposition (to not reinvent the wheel). Plus, I think ng2-redux is our library of choice for client projects. If we remove that content altogether, how can we even talk to our clients about using ng2-redux if we don't even promote it? In my opinion, ngrx should be the focus on the section and ng2-redux an addendum showcasing the difference in terms of value proposition and API. |
|
I don't have a problem with adding back in the content for ng2-redux - happy to do it. I would like to hear ideas on how to add ng2-redux without interrupting the flow of the book or the content I wrote. Perhaps we can write a new section that goes through the same example - but at a faster pace without the concepts? We can talk about this during our meeting we have scheduled - I'm not sure the best way to go about it. |
|
@egervari yes, a quick example showing the difference of usage might be enough in my opinion for ng2-redux. For the most part they are the same. You can point people to the repo if they want more information about ng2-redux |
|
|
||
| getCurrentValue(): Observable<number> { | ||
| return this.store.select(appState => appState.counter.currentValue) | ||
| .filter(currentValue => Boolean(currentValue)); |
There was a problem hiding this comment.
What problem is this filter trying to solve? Something like this in the component's template?
{{ (myObservable$ | async)?.myProperty }}
There was a problem hiding this comment.
Yep, pretty much. I just want to say, "Hey, you can do stuff like this...". I think people forget that they are observables, so they create these complex expressions in their templates, but they are never interested in the null case and get all of this needless complexity and ugliness. Since I saw this anti-pattern a lot, I just thought I'd mention it. I can remove it if you want.
There was a problem hiding this comment.
I use this construct all the time but usually with arrays and buffers for filtering input. You can shortcut it to:
getCurrentValue(): Observable<number> {
return this.store.select(appState => appState.counter.currentValue)
.filter(Boolean);This eliminates an extra closure on each iteration/event. That said $20 says the minifier optimizes that anyway ;)
There was a problem hiding this comment.
Yay, I will add that short-cut.
I have a habit of writing things out explicitly to help me out when i read it down the road - so I don't have to think too much when I read it 3 months later :) And my vision sucks, so less staring/straining at something the better it is for me, especially when there's a lot of tersely written ()'s and function calls - I find that it can be hard to follow unless I start putting each call on one line and making the structure of the calls very obvious and visible.
On some other projects, I've seen really nasty expressions where they are doing an *ngIf on 3 different observables with ?: operators, and that was a strong motivation for me to share this.
|
|
||
| constructor(private store: Store<AppState>) { | ||
|
|
||
| } |
There was a problem hiding this comment.
We can save some space:
constructor(private store: Store<AppState>) {}
There was a problem hiding this comment.
Sure, I can do that. I sort of like putting in the spaces as a personal preference but I don't mind changing it.
Moved ngrx under State management, and added placeholders for rxjs and ng2-redux (to be done later by someone else). Changed extra space in constructor definition Used Boolean short-cut
|
@bennett000 @barretodavid Made modifications per review comments and added state management stuff we talked about |
…llow the other sections with multiple words just after I pushed
@egervari is there a link or something where I could read about this? :) |
Completely rewrote this section on Redux, reworking everything to use Ngrx as well as making it flow a lot better. Concepts are now introduced in the right order, a lot more information is given concerning best practices and gotchas, and I just generally cleaned it up. It also contains a lot of new material, including @effect's
connected to #717