-
Notifications
You must be signed in to change notification settings - Fork 789
mapStateToProps params #20
Comments
@johnthepink it should have ownProps. I thought I had a test in place for that? |
I had a few issues with various arguments as well. I don't think our tests totally cover the different lifecycle events with changes coming from props, redux, etc. |
@stubailo the tests are pretty minimal right now. @johnthepink would you be up to taking that for me today and tomorrow? Trying to be better about actually being on vacation ;) |
I managed to build an example app on top of it, so I'm happy! No rush on making these improvements, I think - vacation away! |
@stubailo @johnthepink is triumphantly moving our stavj to Apollo now so he may need some changes. I'm really happy it's all decent ly coming together though! |
Yep! I'll jump in on fixing some stuff here soon. Found a couple other things. |
Fixed with #51 |
It looks like
mapStateToProps
currently gets called with just the redux state. The docs show that it gets called with an object containingownProps
andstate
. Is it intended to containownProps
as well? Or should we update the docs? cc @jbaxleyiiiThe text was updated successfully, but these errors were encountered: