2012.11.09

Avoiding the this keyword on jQuery related code

jQuery implements some nice abstractions for DOM manipulation and browser events, removing a lot of crossbrowser issues and making it easier to accomplish non-trivial tasks. - it also has drawbacks and some poor design decisions, but I will leave that for another post. Maybe one day I will write my jQuery 2.0 wishlist… - Today I will explain why I usually avoid using the magic this keyword inside event handlers and inside most methods that manipulates jQuery collections.

The this keyword

The fact that you can call a method and pass a different context, by using Function#apply and Function#call, is one of the most powerful things for code reuse and also one of the most misunderstood features in JavaScript. jQuery (ab)uses this feature a lot, which in my (not so humble) opinion is not a very good design decision.

Newbies and code reuse

It is confusing for newbies and it favors code that is tight coupled with a specific scenario, the chance of reusing the same method latter is reduced.

Ambiguity and hidden dependency

The this keyword is too ambiguous, which makes the code harder to understand, you need to check how the method is invoked to understand what value this holds.

function injectMoreInfoLink(i){
    $(this).append('<a href="#/foo/'+ i +'">More Info</a>');
}
$('.foo').each(injectMoreInfoLink);

If you use an IDE/editor which gives some sort of code completion based on the parameters that the method accepts you wouldn’t know that you need to invoke the injectMoreInfoLink function as injectMoreInfoLink.call(element, i). Very counterintuitive. List all dependencies and avoid magic, it is going to be way easier to understand and reuse it in the future:

function injectMoreInfoLink(i, target){
    $(target).append('<a href="#/foo/'+ i +'">More Info</a>');
}
// works exactly the same way as previous example
$('.foo').each(injectMoreInfoLink);
// you can call it easily if needed
injectMoreInfoLink(12, someElement);

PS: jQuery got the arguments order wrong on the map and each callbacks. On the ES5 Array methods the arguments order is reversed (value comes before index). This always drives me crazy and the main reason why I usually don’t use the static versions of map and each to iterate over arrays.

Event Listeners, testability and bad practices

I strongly believe that the abuse of the this keyword influences bad practices, since it’s easier to do it the wrong way.

Just to point out how the this keyword works inside jQuery event listeners:

// the same behavior applies if using event delegation
$('.btn').on('click', function(evt){
    // target can be a child of ".btn" or ".btn" itself
    console.log(this === evt.target); // true or false
    // currentTarget is the ".btn" itself
    console.log(this === evt.currentTarget); // true
});

A common pattern:

function Foo(name){
    this.name = name;
    // anti-pattern, constructors should not cause side-effects!
    $('.bar').click(this.doAwesomeStuffOnClick);
}
Foo.prototype = {
    doAwesomeStuffOnClick : function(evt){
        evt.preventDefault();
        // misleading, unsure if `this` is the instance or an element
        $(this).toggleClass('awesome');
        // ops, won't work as expected
        // how do you access the instance name ??
        console.log(this.name); // undefined
    }
};
var myObj = new Foo('lorem ipsum');

Another option is to create a local var self and inline the doAwesomeStuffOnClick method - on this naive example that would work - but I would rather avoid this kind of pattern, it leads to code that is harder to maintain/reuse/test. Sooner than you expect you will have 5 nested functions and a huge pile of spaghetti code - been there, done that, no thanks.

function Foo(name){
    this.name = name;
    // this is an anti-pattern, avoid it as much as possible !!!
    var self = this;
    $('.bar').click(function(evt){
        evt.preventDefault();
        $(this).toggleClass('awesome');
        console.log(self.name);
    });
}
var myObj = new Foo('lorem ipsum');

I would probably implement it like this:

function Foo(name){
    this.name = name;
    // override method and bind to proper context
    this.doAwesomeStuffOnEvent = $.proxy(this.doAwesomeStuffOnEvent, this);
}
// all methods are grouped on the prototype
// can be reused/tested/overwritten individually
Foo.prototype = {
    init : function(){
        $('.bar').click(this.doAwesomeStuffOnEvent);
    },
    doAwesomeStuffOnEvent : function(evt){
        evt.preventDefault();
        this.doAwesomeStuff(evt.currentTarget);
    },
    doAwesomeStuff : function(target){
        $(target).toggleClass('awesome');
        console.log(this.name);
    }
};
var myObj = new Foo('lorem ipsum');
myObj.init();

For most cases that might be overkill, but I’d rather code something that is easy to change than have to deal with a huge pile of mess, requirements change way more often than we expect.

More lines of code is not always a bad thing. If I ever need to execute the doAwesomeStuff on a different event type, let’s say a keypress, just need to add the new listener. If I decide to subclass this object I can override each individual method. That way you can also trigger doAwesomeStuff without passing an Event object, which depending on the scenario is a huge win. This also improves the testability of the code since every single concern is split into small chunks that can be called individually – unit testing FTW.

Too much this

I’m not a huge fan of plugins, in fact I think most people should not be writing plugins at all. If the this keyword is already confusing inside a simple code snippet, now think about a jQuery plugin.

// WARNING!! this code is useless and a very bad example of a plugin.
// we don't really need that many nested functions, it's just to point how
// confusing it can be to track what the `this` keyword means inside each fn
// PS: I've seen code like this many times
jQuery.fn.doFoo = function(className){
    // `this` points to the jQuery collection that was matched before calling
    // the `doFoo` method
    this.each(function(){
        // `this` is an individual element of the jQuery collection
        $(this).find(className).each(function(){
            // `this` is an individual element child of the collection
            $(this).append('bar');
        });
    });
    // we return the original collection so the user can "chain" multiple calls
    return this;
};
// FYI: it could be rewritten as `this.find(className).apend('bar');`

It can get even more confusing, now imagine a junior developer trying to debug or tweak this code… #chaos

Conclusion

Be explicit whenever possible, more lines of code and longer variable names aren’t always a bad thing, code completion/snippets are here to help, it usually pays-off in the long run. Code should be easy to read by humans, typing in most cases isn’t the bottleneck.

Further reading


Comments

For the love of all that is good and holy, don't write code like: Foo.prototype = ... It breaks inheritance. I know it's easier, but it's also wrong.

Great post!

@jon on this specific case I'm not using any inheritance, in fact I rarely use it in JavaScript. If you wanted to extend another object could just use a method like amd-utils/lang/createObject and set the prototype the same way, no need to change the literal object structure.

Foo.prototype = createObject(Parent.prototype, literalObjectWithFooProperties);

But I agree that it is better to not use the literal object in most cases since it is less flexible, specially if you want/need real private methods. If you list all properties/methods separately you can group code by proximity (you can place the private methods close to the method that calls it) and also avoid overriding the existing prototype (if it was set before).

On this case I used dumb code samples everywhere, the reader should focus on the use of the this keyword. Cheers.

Excellent post, and well exampled. I never understood why jQuery decided that the each iterator callback should be called in the context of the iteration value. It's far more convenient to just pass the value in the function callback (value first of course!) and they could have even taken the effort to pre-wrap the element in $. Why $.each behaves that way and $('selector').each doesn't is even more confusing.

//life would be nice in jQuery2.0
$('selector').each(function(elem) {
    elem.hide();
});

Granted there would be a performance cost for the corner case where you just wanted the raw elements, but it is definitely a corner case in practice. In any situation where you needed the raw iteration performance you probably need raw selection performance as well.

And although I'm less inclined to avoid some feature because it's magic, or difficult to learn for beginners, in this case it's just not necessary which is a bad use of this.

The 'this' keyword in Javascript is a total abomination I avoid as much as possible; It's broken and having 87 occurences of 'this' in one file is just an eyesore.

However, JQuery got the order of the map/each args right :) It's FAR more common having to iterate over the items of an Array without needing the index than the opposite.

[...] I rarely need the index but I need a reference to the element 99% of the times. And no, don’t tell me to use this to reference the element (I avoid it). [...]

Leave a Comment

Please post only comments that will add value to the content of this page. Please read the about page to understand the objective of this blog and the way I think about it. Thanks.

Comments are parsed as Markdown and you can use basic HTML tags (a, blockquote, cite, code, del, em, strong) but some code may be striped if not escaped (specially PHP and HTML tags that aren't on the list). Line and paragraph breaks automatic. Code blocks should be indented with 4 spaces.