search

clayallsopp / react.backbone

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

component.onModelChange ? #40

Open tsjoberg opened 9 years ago

tsjoberg commented 9 years ago

Believe @markijbema can shed some light on this but I couldn't find an answer:

https://github.com/clayallsopp/react.backbone/blob/master/react.backbone.js#L37

is component.onModelChange a React component property? or added by react.backbone mixin? I couldn't find any reference to this in React docs or in the mixin code as well

markijbema commented 9 years ago

Seems like an extension point. Default is force update, but if you override onModelChange, that's called instead. If this isn't in the docs, lets keep this issue open until it is ;)

tsjoberg commented 9 years ago

Just a proposal but would it make sense to just remove this altogether? Doesn't seem as useful and is not documented anywhere so people would not know this existed without checking the code?

markijbema commented 9 years ago

It has been here since the very first commit. If you want to remove it, I think we should at least add a deprecation warning: https://github.com/clayallsopp/react.backbone/commit/c15403e1bd66d7ad53931166dad472164ed0df17

@clayallsopp do you remember why you added this? Do you use this?

clayallsopp commented 9 years ago

I don't use this method anymore, fine with removing it or deprecating