clayallsopp / react.backbone

Plugin for React to make Backbone migration easier
MIT License
839 stars 60 forks source link

enhance modelOrCollection(props) to accept state as well #27

Closed kinergy closed 10 years ago

kinergy commented 10 years ago

I use filtered collections in my React components and I would to set them up in getInitialState(). The initial filtering is expensive and I'd like to use React.Backbone to setup all the listening to the collection events.

Do you see any issue with changing modelOrCollection from modelOrCollection(props) to: modelOrCollection(props, state)

I'd be happy to put together a PR.

clayallsopp commented 10 years ago

Hm yeah, it seems rather un-React to externalize state, I wouldn't be a huge fan of something like that in React.Backbone.

It sounds like such an enhancement would be doable with your own React mixin though?

markijbema commented 10 years ago

Actually, I sort of wanted to create something like this earlier. While I disagree on this usecase, in general, being able to have collections outside of props is useful. My suggestion would be to pass in an function instead of a prop name. That way you could also have global models (currentUser) or models in the explicit React context. I think I would be in favor of such a PR, but I think it's rather hard to get the binding/unbinding just right.

To get back to the problem at hand, I think there's probably an easier solution. A logical way would be to have a wrapping component which creates the filtered collection and passes it into the React.Backbone class. Don't forget that React components are not required to do anything view-related. You can have components which just logic, which only render one other component (or even null).

kinergy commented 10 years ago

Thanks for the comments - I did indeed implement both of the proposals above in a couple places.