scttnlsn / backbone.io

Backbone.js sync via Socket.IO
http://scttnlsn.github.io/backbone.io
541 stars 66 forks source link

Allow io to be injected #22

Closed aikomastboom closed 9 years ago

aikomastboom commented 11 years ago

To allow our server architecture to be more flexible, we have added this change. It allows us to create the server elsewhere and hook multiple websocket libraries to the same connection.

scttnlsn commented 11 years ago

This is great. When io is passed in the server argument is uneeded though. I wonder if io could be passed as the first argument and we could distinguish if it was a port number, http server or io object?

scttnlsn commented 11 years ago

Perhaps the io object should always be injected and we should just bump the minor version. Minimal effort required to migrate.

/cc @Incubatio

Incubatio commented 11 years ago

This is great. When io is passed in the server argument is uneeded though. I wonder if io could be passed as the first argument and we could distinguish if it was a port number, http server or io object?

Yes, you're right server is not needed. Basically, encapsulating the initialization of socket.io in backbone.io does not respect Inversion Of Control, blocking readability and re usability.

As long Aiko Mastboom's patch is bad, i made https://github.com/scttnlsn/backbone.io/pull/28 .

Perhaps the io object should always be injected and we should just bump the minor version. Minimal effort required to migrate. I did inject the io object.

If bump mean skip, i don't think we should skip this minor patch as long current version raise several problems:

scttnlsn commented 9 years ago

I'm deprecating backbone.io and continuing development on a similar project: data.io Please see the README for more information.

If you'd still like to address this in the context of data.io please open an issue/pull-request for that project.

Thanks, Scott