CODEWITHSUNDEEP
javascriptclosures
Jeff Dege
Jeff Dege

Reputation: 11740

Question about Javascript closures

I'm working with JQuery and jsTree, and I've run into some confusion about how closures work.

I have an object that has a .jsTree member, and a .populateTree method. The method is called with an array of strings, with which it is supposed to create the nodes of the jsTree.

The jsTree builds a tree control, in which every node has an anchor "", which contains the text of the node. I want to make clicking on the text toggle the node open or closed, just like clicking on the +/- button in the tree. So I'm trying to add a click() function to do that, and I'm getting unexpected behavior.

So, here is the code:

populateTree: function populateTree(nodeNames)
{
    if (!this.jsTree) // Note 1
        return;

    var me = this; // Note 2

    for (var i = 0; i < nodeNames.length; i++)
    {
        var nodeName = nodeNames[i];

        var node = this.jsTree.create_node(-1, "last", { state: 'open', data: nodeName }); //Note 3

        this.jsTree.create_node(node, "last", { data: "child one" }); // Note 4
        this.jsTree.create_node(node, "last", { data: "child two" });
        this.jsTree.create_node(node, "last", { data: "child three" });

        var anchor = node.find("a"); // Note 5
        anchor.click(function() { me.jsTree.toggle_node(node); }); // Note 6
    }
},

My problem? No matter which of the anchors I click on, it's always the last top-level node that opens and closes. It's like the closure for each of the "click" functions we've created has a closure that references only the last "node" variable. And that's not the way I thought closures worked.

Can someone help me understand where I went wrong in my understanding?

Thanks.

Upvotes: 2

Views: 638

Answers (3)

David Ly
David Ly

Reputation: 31606

The issue is that in Javascript, blocks do not define scope, functions do.

So even though nodeName and node are defined inside the for loop, they act identical to if they were defined outside, because the for loop block does not create a new scope.

That's why in the book "Javascript: The Good Parts" Crawford recommends that in Javascript you define local variables at the beginning of a function rather than closest to it's usage like you would in other languages. Defining them further down makes it appear as if it were scoped inside the containing block when in reality they are not.

Upvotes: 2

HBP
HBP

Reputation: 16063

Remember that closures are static. The value of the node used in your click handler will be that last value it was assigned, NOT the value it had when you the click handler was created.

The fix is to create a new closure for each click handler: 

var anchor = node.find("a"); // Note 5
(function (node) {
  anchor.click(function() { me.jsTree.toggle_node(node); }); // Note 6
}) (node);

The anonymous function is called with the current value of node, and that is the one referenced when the click handler is invoked. Each click handler is provided with its own value of node

Upvotes: 1

davin
davin

Reputation: 45565

The anonymous function which you attach as your click handler closes over a single node instance, once the loops finished executing, and many seconds later when the user clicks on the tree that anonymous function is executed, it will look at the scope it closed over when created, and see that node's value is that which it last held, like you noticed, which is that of the final iteration of the loop.

A quick fix could be:

anchor.click((function(node){ return function() { me.jsTree.toggle_node(node); }; })(node));

This way, the closed over value of node is the passed value, which will hold a different value for each iteration.

Upvotes: 3

Related Questions