addyosmani/todomvc — pull request #190 by richardharrington (2012-06-19 01:19:50+00:00)
Title: fixed a loss of context for 'this' keyword

My commit:

"fixed loss of context for 'this' keyword when AppView instance method addAll calls AppView instance method addOne (in the backbone app)."

Here's the way it was:

var AppView = Backbone.View.extend({

// ** snip **

addOne: function(todo) {
var view = new TodoView({model: todo});
this.$("#todo-list").append(view.render().el);
},

addAll: function() {
Todos.each(this.addOne);
},

// ** snip **

});

The line

Todos.each(this.addOne);

is actually passing 'addOne' as a parameter to the anonymous callback of the 'each' method without a context for its 'this' keyword, thus causing 'this' inside 'addOne' to refer to the window object. So in the line inside 'addOne',

this.$("#todo-list").append(view.render().el);

'this.$' evaluates at runtime to 'window.jQuery', which happens to work in this app, but it's probably not what you are intending, and it would quickly cause an error or unwanted behavior if you were trying to isolate a DOM element by class name instead of id, and there were many elements on the page with the same class name, all inside different top-level view objects. It would also break if you were using jQuery.noConflict() and running all this code inside a wrapper function, where the local variable $ meant something completely different from window.$.

Changing the call to 'addOne' by binding it to the context of the AppView instance,

Todos.each(this.addOne, this);

will solve this problem.

Thanks for making this example, by the way. It's incredibly helpful.