[javascript] Passing parameters in Javascript onClick event

I'm trying to pass a parameter in the onclick event. Below is a sample code:

<div id="div"></div>

<script language="javascript" type="text/javascript">
   var div = document.getElementById('div');

   for (var i = 0; i < 10; i++) {
       var link = document.createElement('a');
       link.setAttribute('href', '#');
       link.innerHTML = i + '';
       link.onclick=  function() { onClickLink(i+'');};
       div.appendChild(link);
       div.appendChild(document.createElement('BR'));
       }

   function onClickLink(text) {
       alert('Link ' + text + ' clicked');
       return false;
       }
    </script>

However whenever I click on any of the links the alert always shows 'Link 10 clicked'!

Can anyone tell me what I'm doing wrong?

Thanks

This question is related to javascript closures

The answer is


This is happening because they're all referencing the same i variable, which is changing every loop, and left as 10 at the end of the loop. You can resolve it using a closure like this:

link.onclick = function(j) { return function() { onClickLink(j+''); }; }(i);

You can give it a try here

Or, make this be the link you clicked in that handler, like this:

link.onclick = function(j) { return function() { onClickLink.call(this, j); }; }(i);

You can try that version here


It is probably better to create a dedicated function to create the link so you can avoid creating two anonymous functions. Thus:

<div id="div"></div>

<script>
  function getLink(id)
  {
    var link = document.createElement('a');
    link.setAttribute('href', '#');
    link.innerHTML = id;
    link.onclick = function()
    {
      onClickLink(id);
    };
    link.style.display = 'block';
    return link;
  }
  var div = document.getElementById('div');
  for (var i = 0; i < 10; i += 1)
  {
    div.appendChild(getLink(i.toString()));
  }
</script>

Although in both cases you end up with two functions, I just think it is better to wrap it in a function that is semantically easier to comprehend.


Another simple way ( might not be the best practice) but works like charm. Build the HTML tag of your element(hyperLink or Button) dynamically with javascript, and can pass multiple parameters as well.

// variable to hold the HTML Tags 
var ProductButtonsHTML  ="";

//Run your loop
for (var i = 0; i < ProductsJson.length; i++){
// Build the <input> Tag with the required parameters for Onclick call. Use double quotes.

ProductButtonsHTML += " <input type='button' value='" + ProductsJson[i].DisplayName + "'  
onclick = \"BuildCartById('" + ProductsJson[i].SKU+ "'," + ProductsJson[i].Id + ")\"></input> ";

}

// Add the Tags to the Div's innerHTML.
document.getElementById("divProductsMenuStrip").innerHTML = ProductButtonsHTML;

or you could use this line:

 link.setAttribute('onClick', 'onClickLink('+i+')');

instead of this one:

link.onclick=  function() { onClickLink(i+'');};

link.onclick = function() { onClickLink(i+''); };

Is a closure and stores a reference to the variable i, not the value that i holds when the function is created. One solution would be to wrap the contents of the for loop in a function do this:

for (var i = 0; i < 10; i++) (function(i) {
    var link = document.createElement('a');
    link.setAttribute('href', '#');
    link.innerHTML = i + '';
    link.onclick=  function() { onClickLink(i+'');};
    div.appendChild(link);
    div.appendChild(document.createElement('BR'));
}(i));

This will work from JS without coupling to HTML:

document.getElementById("click-button").onclick = onClickFunction;

function onClickFunction()
{
    return functionWithArguments('You clicked the button!');
}

function functionWithArguments(text) {
    document.getElementById("some-div").innerText = text;
}

Try this:

<div id="div"></div>

<script language="javascript" type="text/javascript">
   var div = document.getElementById('div');

   for (var i = 0; i < 10; i++) {
       var f = function() {
           var link = document.createElement('a');
           var j = i; // this j is scoped to our anonymous function
                      // while i is scoped outside the anonymous function,
                      //  getting incremented by the for loop
           link.setAttribute('href', '#');
           link.innerHTML = j + '';
           link.onclick=  function() { onClickLink(j+'');};
           div.appendChild(link);
           div.appendChild(document.createElement('br')); // lower case BR, please!
       }(); // call the function immediately
   }

   function onClickLink(text) {
       alert('Link ' + text + ' clicked');
       return false;
   }
</script>

onclick vs addEventListener. A matter of preference perhaps (where IE>9).

// Using closures
function onClickLink(e, index) {   
    alert(index);
    return false;
}

var div = document.getElementById('div');

for (var i = 0; i < 10; i++) {
    var link = document.createElement('a');

    link.setAttribute('href', '#');
    link.innerHTML = i + '';
    link.addEventListener('click', (function(e) {
        var index = i;
        return function(e) {
            return onClickLink(e, index);
        }
    })(), false);
    div.appendChild(link);
    div.appendChild(document.createElement('BR'));
}

How abut just using a plain data-* attribute, not as cool as a closure, but..

function onClickLink(e) {       
    alert(e.target.getAttribute('data-index'));
    return false;
}

var div = document.getElementById('div');

for (var i = 0; i < 10; i++) {
    var link = document.createElement('a');

    link.setAttribute('href', '#');
    link.setAttribute('data-index', i);
    link.innerHTML = i + ' Hello';        
    link.addEventListener('click', onClickLink, false);
    div.appendChild(link);
    div.appendChild(document.createElement('BR'));
}