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.