Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't require Rivets nor Backbone yourself #19

Open
moll opened this issue Dec 12, 2014 · 4 comments
Open

Don't require Rivets nor Backbone yourself #19

moll opened this issue Dec 12, 2014 · 4 comments

Comments

@moll
Copy link

moll commented Dec 12, 2014

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
Copy link
Owner

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

@moll
Copy link
Author

moll commented Dec 15, 2014

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
Copy link
Owner

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
Copy link
Author

moll commented Dec 15, 2014

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants