flummox
flummox copied to clipboard
Improve HoC connectToStores Docs
I think that the connectToStores HoC pattern is a huge plus for Flummox, and that the actual docs don't showcase how awesome it is (Easy, intuitive Relay-like data fetching from stores).
I'd be happy to send a PR with an updated Quick start, examples and a "Why HoC > FluxComponent > fluxMixin". Also I could introduce
contextTypes: {
flux: React.PropTypes.object
},
because without it HoC becomes very clunky.
@acdlite basically I just want to check if this is the direction that you want on the guide/docs?
Yes to all, that would be a huge help
just checking: right now the HoC needs a higher FlummoxComponent to pass the flux object to the context right?. or is there a way to pass the flux object to the HoC?
Correct, currently no way to pass Flux directly to the HoC
Slightly unrelated: are there plans to add this behavior to the HoC?, should I open an issue?
Yeah, eventually. Also want to add injectActions
, but I'm not sure what the best API for that is yet.
been working on the docs!. Quickstart and React integration are ready. i'll be doing "Why HoC > FluxComponent > fluxMixin" next week!
https://github.com/acdlite/flummox/compare/master...burabure:master
:+1: Documentation on what FluxComponent.connectToStores()
solves, and how (notably what do its parameters represent) would be most welcome.
Once i hit the point (a very lucky shot) where "it works but i do not know why" i figured out what did what by intentionally breaking stuff, but its not exactly evident. And i was not even aware that HoC != FluxComponent.
:+1:
Thanks a lot for these docs @burabure ! It helped me a loooot. If I could make a comment on this part : (https://github.com/burabure/flummox/blob/master/docs/docs/guides/react-integration.md)
// Pass an object of store keys mapped to getter functions
MyComponent = connectToStores(MyComponent, {
posts: store => ({
postBody: store.getPostBody(this.props.post.id),
}),
comments: store => ({
comments: store.getCommentsForPost(this.props.post.id),
})
});
Depends on your module loading system, if you're using io.js, this
will be ok but in certain cases like with Babel, this
could be undefined.
=> https://github.com/babel/babel/issues/562
I think it could be helpful for those like me to include this point in the documentation. Thoughts ?
thanks @akofman, happy to help =)
about the issue: I think you're right. with ES6 arrow functions this
would point to the outside scope instead of connectToStores context. thanks!
i'll fix it right away!
Store state getters are not bound to the component instance, as of v3.0, so even without an arrow function, this.props
will always be undefined. The way to do this now is to use the props
parameter:
// Pass an object of store keys mapped to getter functions
MyComponent = connectToStores(MyComponent, {
posts: (store, props) => ({
postBody: store.getPostBody(props.post.id),
}),
comments: (store, props) => ({
comments: store.getCommentsForPost(props.post.id),
})
});
Oops, I hate that button :D
thank you @acdlite. is that behaviour documented?, I think there's something about them in the upgrade guide, but it seems everything else is outdated.
I'll add it to the component, HoC and mixin just in case =)
I don't think it's explicitly documented anywhere, just reflected in the examples.
Thanks :)
just checking: this.props
on the fluxcomponent would also be undefined since 3.0 right?
also, i think connectToStores={{
double wrapping with {} is wrong right?
class BlogPostPage extends React.Component {
render() {
<div>
<SiteNavigation />
<MainContentArea>
<FluxComponent connectToStores={{
posts: store => ({
post: store.getPost(this.props.postId),
})
}}>
<BlogPost />
</FluxComponent>
</MainContentArea>
<SiteSidebar />
<SiteFooter />
</div>
}
}
In that case it refers to the props of the owner (BlogPostPage), which is probably what you want.
Double wrapping is correct because you're passing an object. The first set of braces is for embedding a JavaScript value in JSX; the second set is the object itself. It's the same concept as when you pass an inline object to the style
props.
stateGetter's second param (props) in the docs now.
https://github.com/burabure/flummox/commit/145f977adf8f6bd2028cd6231321cdbf882a33bd
@acdlite if you have a couple minutes for a quick check, in case I messed up some detail, it would be awesome =)
typo fixed and proof read. ready to pull in =)
Thanks! I'll try to get this merged in tonight. Sorry I've been a bit less responsive lately — I accepted a new job in a new city, so I've been really busy :)
hope the new job and the moving stuff are getting less crazy ;D
Let me know if I can help with anything to make this PR easier to pull
@acdlite we should get this merged I think :)
+1 this would be helpful to have merged. I cloned vesparny's repo and it uses the HoC, and with no documentation it was pretty confusing.
+1
While the function itself is anonymous - now that connectToStores
accepts actions I'd recommend renaming its usage in the docs to connectToFlux
or something similar.