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

Extend event binding with optional context #70

Closed
dirkbonhomme opened this issue Mar 15, 2014 · 10 comments
Closed

Extend event binding with optional context #70

dirkbonhomme opened this issue Mar 15, 2014 · 10 comments

Comments

@dirkbonhomme
Copy link
Contributor

It's common to add a third "context" argument when binding event handlers in Backbone and many other frameworks. This way you can avoid .bind() on all your callbacks and it eases unbinding:

http://backbonejs.org/#Events-on

// Removes just the `onChange` callback.
object.off("change", onChange);

// Removes all "change" callbacks.
object.off("change");

// Removes the `onChange` callback for all events.
object.off(null, onChange);

// Removes all callbacks for `context` for all events.
object.off(null, null, context);

// Removes all callbacks on `object`.
object.off();

Or when applied to Pusher:

{
    initialize: function(){
        var channel = pusher.subscribe('my-channel');
        channel.bind('lorem-ipsum', this.handleLipsum, this);
        channel.bind('foo-bar', this.handleBar, this);
    },

    teardown: function(){
        channel.unbind(null, null, this);
    }
}

This will remain compatible with the current implementation. Would you be interested in a PR for this feature? I'm prepared to add it to the codebase but don't want to maintain a separate fork..

@zimbatm
Copy link
Contributor

zimbatm commented Mar 18, 2014

Go for it. We were talking about that the other day and I think it's a good idea. Let me know if you're hitting any blocker.

@pl
Copy link
Contributor

pl commented Mar 19, 2014

Hi Dirk,

Sounds good, I think it makes the code cleaner and as you said, the API is backwards compatible. I'll be happy to merge it into the next JS release.

@dirkbonhomme
Copy link
Contributor Author

I can't seem to figure out how to install all Ruby gems on my machine (OS X Mavericks). Any tips? What kind of configuration are you running it on?

Fetching additional metadata from http://rubygems.org/..
Using rake (0.9.2.2)
Using builder (3.2.2)
Using mime-types (2.0)
Using xml-simple (1.1.2)
Using aws-s3 (0.6.3)
Using timers (1.1.0)
Using celluloid (0.15.2)

Gem::Ext::BuildError: ERROR: Failed to build gem native extension.

    /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/bin/ruby extconf.rb 
/System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/bin/ruby: invalid option -m  (-h will show valid options) (RuntimeError)

extconf failed, exit code 1

Not familiar with Ruby/gems so there might be an easy fix..

@pl
Copy link
Contributor

pl commented Mar 19, 2014

Hm, seems like a problem with building ffi, which is a native gem. I am getting a different error if I switch to the system version of Ruby.

Ruby bundled with OS X is a bit crap - I would recommend using rbenv (or rvm) to install it properly. Also, you should install gcc, because Apple decided to swap it for clang and it might cause some issues with native extensions. Here are some instructions on how to do that:

http://blog.zerosharp.com/installing-ruby-with-homebrew-and-rbenv-on-mac-os-x-mountain-lion/

@pl
Copy link
Contributor

pl commented Mar 19, 2014

I should document this and pusher-js should use one platform for building and testing - requiring Ruby and Node.js is a bit of a pain.

@zimbatm
Copy link
Contributor

zimbatm commented Mar 20, 2014

Another way is to install Homebrew and then brew install ruby. That would give you a most recent and probably fixed version of ruby.

@dirkbonhomme
Copy link
Contributor Author

Finally managed to install all Ruby related dependencies on a Vagrant box but now I can't install Karma. Guess it has to do with a Python mismatch that's required to compile one of its dependencies (Socket.io > ws).

You guys don't happen to have a pre-configured Vagrant box for running these tests? 😆

@pl
Copy link
Contributor

pl commented Mar 24, 2014

Hi Dirk,

I'm sorry this process is so painful - I never had these problems and it seems like I was the only one running tests since 2.0 ;)

Don't waste your time on Karma, just test it on Chrome or whatever browser you're using, run the code through jshint (will catch missing semicolons, extra commas, etc.) and that's enough for me :) I will do proper cross-platform testing before releasing the next stable version or release candidate.

@dirkbonhomme
Copy link
Contributor Author

Guess you have to be either experienced with Ruby / Python or have a lot of luck installing compatible versions :D

Anyway, I got bin/jasmine running and pushed the changes. Thanks for the tip.

@pl
Copy link
Contributor

pl commented May 15, 2014

Released in 2.2.0, closing. Thanks for the PR once again :)

@pl pl closed this as completed May 15, 2014
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

3 participants