1

I have a knockout viewModel with an array of items. I want to watch one property (selected) on all the items and take action when it's changed.

For that I have a SectionManager that will do that. I initialize the manager and set up subscribes for every item. The myid closure is not captured, it is always 3, the last value. Can somebody tell me where I've lost it?

Example: If you click on an item in the list, the asterisk will indicate if it is selected. That works. The subscribed function is called too, and the myid is written to the console, but that's always 3.

John

HTML:

  <!DOCTYPE html>
  <html>
  <head>
     <title></title>
     <script type="text/javascript" src="Scripts/jquery-1.7.2.js"></script>
     <script type="text/javascript" src="Scripts/knockout-2.1.0.debug.js"></script>
     <script type="text/javascript" src="test.js"></script>
  </head>
  <body>
     <ul data-bind="foreach: roles">
        <li data-bind="click: toggle">
           <span data-bind="text: id"></span>
           <span data-bind="visible: selected">*</span>
        </li>
     </ul>
  </body>
  </html>

and this script:

  var roles = [
     { id: 1 },
     { id: 2, selected: true },
     { id: 3 }
  ];

  var viewModel = (function (roles) {
     var obj = {};
     var arr = [];
     for (var i = 0; i < roles.length; i++) {
        arr.push({
           id: roles[i].id,
           selected: ko.observable(roles[i].selected || false),
           toggle: function () {
              this.selected(!this.selected());
           }
        });
     }
     obj.roles = ko.observable(arr);

     return obj;
  })(roles);

  var sectionManager=(function(){
     return {
        init: function (roles) {
           for (var i = 0; i < roles.length; i++) {
              var item = roles[i];
              var myid = item.id;

              item.selected.subscribe(function () {
                 console.log(myid);  // ALWAYS 3!!
              });
           }
        }
     };
  })();

  $(function () {
     sectionManager.init(viewModel.roles());
     ko.applyBindings(viewModel);
  });
John Landheer
  • 3,999
  • 4
  • 29
  • 53

1 Answers1

3

You are running into a classic issue caused by closures in JavaScript. In your loop there is really only one variable named myid. The functions that you are creating when subscribing to each item all have access to that same myid variable. At the end of the loop it's value is 3, so when the handlers run they all report the value of that variable.

There is a lot of reference out there for closures and scoping in JavaScript. For example, here is another SO question: JavaScript closure inside loops – simple practical example

One way for you to handle this, is to not use the intermediate variable and ensure that your handler's run with the current item.

item.selected.subscribe(function () {
    console.log(this.id);
}, item);

In this example, the second argument defines the value of this when the function runs. So, it will run with the correct item as the context and you can access its properties from this.

http://jsfiddle.net/rniemeyer/WV6g5/

You could even use item.id in the second argument.

As you pointed out below, you can certainly create a function that takes in your variable and have it return a function to create a closure around that specific value.

var subscriber = function(id) 
{ 
    return function() { 
        console.log(id); 
    }; 
}; 

...

item.selected.subscribe(subscriber(myid)); 

In the context of Knockout, I think that it is more practical to deal with your data item and ensure that it is this when your handler runs.

Community
  • 1
  • 1
RP Niemeyer
  • 114,592
  • 18
  • 291
  • 211
  • This is indeed a correct answer for this particular case. Thanks to your link a more general solution (not relying on 'this' but fixing the closure) would be to use var subscriber = function(id) { return function() { console.log(id); }; }; item.selected.subscribe(subscriber(myid)); If you agree maybe you can add it to your answer. – John Landheer Aug 17 '12 at 06:02
  • I updated the answer with your alternate solution. In the context of Knockout, I would still recommend binding it to your data item. It is simpler and gives you the flexibility of dealing with multiple properties on your item. Understanding and controlling the value of `this` is an important concept in Knockout. Thanks. – RP Niemeyer Aug 17 '12 at 13:01
  • I understand, and if it was just the observable that was needed in the subscribe that would be it. But I need other info (outside of the observable) as well so a new scope is needed. The example in this case was oversimplified, sorry for that. – John Landheer Aug 17 '12 at 15:00