azproduction / rivets-backbone-adapter

Backbone.js adapter for Rivets.js data-bind with nested models and collections support
http://azproduction.github.io/rivets-backbone-adapter/example/index.html
MIT License
48 stars 15 forks source link

Don't require Rivets nor Backbone yourself #19

Open moll opened 9 years ago

moll commented 9 years ago

Hey,

Please don't require Rivets and Backbone in the code of RivetsBackboneAdapter. Let them be passed in when initializing the module to decouple their loading. In fact, don't require Backbone at all as we're in a dynamic language here.

Cheers

azproduction commented 9 years ago

Both of them are direct dependencies. I can't avoid require'ing them.

moll commented 9 years ago

Nah, that's not a good pattern. Export a simple Rivets handler for the person to set on their instance of Rivets. Globals are a big no-no.

Last I checked you could kill the Backbone dependency, but if you can't figure out how, use dependency injection. Saves depending on the loader and, again, from globals. ;-)

azproduction commented 9 years ago

I see your point. In one hand it could give flexibility, but in the other it forces developer to manually resolve dependences. And it does not make sense since in the most cases we have the only rivets and backbone in project.

require('rivets')
require('rivets-backbone-adapter')(rivets); 
// ^^^ actually the same as require('adapter'); in most cases

Also I can't drop backbone as every single method requires it. And in the same time I should not force developers to resolve deps manually: require('rivets-backbone-adapter')(rivets, backbone).

moll commented 9 years ago

Doing things right isn't always easy. One should also never change variables one doesn't own. I have to remind that that particular Rivets instance could be shared by code that doesn't expect it to have a Backbone adapter. ;-)

Yeah, you should force developers to resolve their dependencies. They have to require their own version of Rivets anyway (in the style of require("./lib/rivets") in code to ensure that it comes with your adapter. Otherwise there's a race condition where they get a Rivets object before the adapter is attached. There's no way around that.