Mutable variable is accessible from closure. How can I fix this?

Question

I am using Typeahead by twitter. I am running into this warning from Intellij. This is causing the "window.location.href" for each link to be the last item in my list of items.

How can I fix my code?

Below is my code:

AutoSuggest.prototype.config = function () {     var me = this;     var comp, options;     var gotoUrl = "/{0}/{1}";     var imgurl = '<img src="/icon/{0}.gif"/>';     var target;      for (var i = 0; i < me.targets.length; i++) {         target = me.targets[i];         if ($("#" + target.inputId).length != 0) {             options = {                 source: function (query, process) { // where to get the data                     process(me.results);                 },                  // set max results to display                 items: 10,                  matcher: function (item) { // how to make sure the result select is correct/matching                     // we check the query against the ticker then the company name                     comp = me.map[item];                     var symbol = comp.s.toLowerCase();                     return (this.query.trim().toLowerCase() == symbol.substring(0, 1) ||                         comp.c.toLowerCase().indexOf(this.query.trim().toLowerCase()) != -1);                 },                  highlighter: function (item) { // how to show the data                     comp = me.map[item];                     if (typeof comp === 'undefined') {                         return "<span>No Match Found.</span>";                     }                      if (comp.t == 0) {                         imgurl = comp.v;                     } else if (comp.t == -1) {                         imgurl = me.format(imgurl, "empty");                     } else {                         imgurl = me.format(imgurl, comp.t);                     }                      return "\n<span id='compVenue'>" + imgurl + "</span>" +                         "\n<span id='compSymbol'><b>" + comp.s + "</b></span>" +                         "\n<span id='compName'>" + comp.c + "</span>";                 },                  sorter: function (items) { // sort our results                     if (items.length == 0) {                         items.push(Object());                     }                      return items;                 }, // the problem starts here when i start using target inside the functions                 updater: function (item) { // what to do when item is selected                     comp = me.map[item];                     if (typeof comp === 'undefined') {                         return this.query;                     }                      window.location.href = me.format(gotoUrl, comp.s, target.destination);                      return item;                 }             };              $("#" + target.inputId).typeahead(options);              // lastly, set up the functions for the buttons             $("#" + target.buttonId).click(function () {                 window.location.href = me.format(gotoUrl, $("#" + target.inputId).val(), target.destination);             });         }     } }; 

With @cdhowie's help, some more code: i will update the updater and also the href for the click()

updater: (function (inner_target) { // what to do when item is selected     return function (item) {         comp = me.map[item];         if (typeof comp === 'undefined') {             return this.query;         }          window.location.href = me.format(gotoUrl, comp.s, inner_target.destination);         return item; }}(target))}; 

Answer 1

I liked the paragraph Closures Inside Loops from Javascript Garden

It explains three ways of doing it.

The wrong way of using a closure inside a loop

for(var i = 0; i < 10; i++) {     setTimeout(function() {         console.log(i);       }, 1000); } 

Solution 1 with anonymous wrapper

for(var i = 0; i < 10; i++) {     (function(e) {         setTimeout(function() {             console.log(e);           }, 1000);     })(i); } 

Solution 2 - returning a function from a closure

for(var i = 0; i < 10; i++) {     setTimeout((function(e) {         return function() {             console.log(e);         }     })(i), 1000) } 

Solution 3, my favorite, where I think I finally understood bind - yaay! bind FTW!

for(var i = 0; i < 10; i++) {     setTimeout(console.log.bind(console, i), 1000); } 

I highly recommend Javascript garden - it showed me this and many more Javascript quirks (and made me like JS even more).

p.s. if your brain didn't melt you haven't had enough Javascript that day.

Answer 2

You need to nest two functions here, creating a new closure that captures the value of the variable (instead of the variable itself) at the moment the closure is created. You can do this using arguments to an immediately-invoked outer function. Replace this expression:

function (item) { // what to do when item is selected     comp = me.map[item];     if (typeof comp === 'undefined') {         return this.query;     }      window.location.href = me.format(gotoUrl, comp.s, target.destination);      return item; } 

With this:

(function (inner_target) {     return function (item) { // what to do when item is selected         comp = me.map[item];         if (typeof comp === 'undefined') {             return this.query;         }          window.location.href = me.format(gotoUrl, comp.s, inner_target.destination);          return item;     } }(target)) 

Note that we pass target into the outer function, which becomes the argument inner_target, effectively capturing the value of target at the moment the outer function is called. The outer function returns an inner function, which uses inner_target instead of target, and inner_target will not change.

(Note that you can rename inner_target to target and you will be okay -- the closest target will be used, which would be the function parameter. However, having two variables with the same name in such a tight scope could be very confusing and so I have named them differently in my example so that you can see what's going on.)