Optimize things a little bit

[gaearon]

I updated the code to be more idiomatic Redux. I think it performs a little better in my testing.
I might be able to take another look tomorrow evening in case there’s something else I could optimize.

The basic idea here (apart from minor refactorings) is to separate independently changing data (the list of items and items themselves). This, of course, highly depends on the specific app, which is why we don’t have a proper guide for doing it, but the point is that it’s usually possible to optimize an app for specific use cases if there is a need for it.

Computing Derived Data is probably the best we have in terms of guides on how to do it, but it definitely could use some state structure refactoring advice content.

cc @ellbee for no specific reason—just in case he’s interested

[capaj]

This is freaking awesome. I can't wait to see the updated charts @mweststrate!

[Kerumen]

Just wondering why you add a shouldComponentUpdate even if the component is connected?
If I read correctly the react-redux docs, you can specifiy if a component implements a shouldComponentUpdate with the fourth argument of connect. The default is true. So by default, all components connected are pure, there is no need to write it.

No doubt you are aware of this, I didn't understand it correctly I guess.

[mweststrate]

@Kerumen you mean like in the original? For the first scenario (without selectors) TodoItem didn't use connect at all but just received it's todo item from the parent. So not using PureRenderMixin would have caused all TodoItem components to re-render if a todo changes, instead of only the affected ones.

[Kerumen]

No I was asking for the @gaearon rewrite. For instance here.

[michaseel]

@gaearon Could you update the unit tests too? I would like to see how you test stateless functional components!

[gaearon]

Well, it’s not that much of an improvement but it’s definitely better. Still can’t avoid going through 10K subscribers. There may be small things to optimize in React Redux as well so I’ll look there.

I removed shouldComponentUpdate later, please see the complete change list. I added it for testing in one of the commits but decided it is unnecessary.

No, alas, I won’t be adding tests here, you should check out Redux repo (and open PRs to it) to see how to test components.

[slorber]

Still can’t avoid going through 10K subscribers.

I guess 10k subscribers is better than 10k items list reconciliation.

But there are potential ways to avoid calling those 10k subscribers on every item change. Didn't have time to work on something concrete yet but it can be optimized further imho. See reduxjs/react-redux#269

[gaearon]

Yeah, there are definitely ways to solve this even with vanilla Redux. The goal here is to avoid complicating the code beyond recognition 🙂

[narqo]

This TodoActions import is useless here, isn't it?

[gaearon]

Yea, missed this one. Doesn't affect the perf though.

[mweststrate]

Thanks for the contributions! Will re-run benchmarks tomorrow.