[WIP] Call setState on React components once per dispatch #21
Conversation
I used this to simplify development by npm linking the local repo to my project and having babel watch for changes.
…onents Relates to optimizely/nuclear-js#195. For the connect method, this change adds only one observer to the reactor per call as opposed to adding a separate listener for each key returned by mapStateToProps.
I see let, var, const elsewhere in the project so I'm not if there is a recommended way to declare variables. But I use const in my own code so I went with that.
|
Also relates to optimizely/nuclear-js#193 |
|
Hmm, your approach seems reasonable, however we are trading evaluating all getters bound to a component vs only triggering getters that have changed and potentially calling re-render more than once. So the question comes down to whether its more expensive to evaluate extraneous getters or call render multiple times, and I'm not quite convinced one is always better than the other. It'd be nice to allow this to be configured somehow when using the decorator or have multiple decorators. Thoughts? |
|
Very good point. My initial thought is that there is probably a better means of doing aggregation than creating a new getter that will be recomputed when any of the bound getters change. I'm not yet sure what that would look like. For now i'm happy to simply use my own fork. In my codebase having aggregated state updates proved very helpful, especially with components that implement I agree that the perf tradeoff is likely a deal breaker for some. |
This PR is incomplete and open for discussion. I updated
connectto only call setState on the wrapped component once per dispatch. I haven't yet touchednuclearMixinas I wanted to get feedback first and see if this is a change you would be interested in, but I'd be happy to update that as well.This relates directly to optimizely/nuclear-js#195 and #13. Open to any feedback or suggestions.