JavaScript closure issue on dynamic element creation in a loop

Question

During the execution of the page, a string gets composed containing the code that would need to be executed as the handler for the click event. The string could look like:

var handler = '"myFunction1(12, 20);myOtherFunction();"';

OR

var handler = '"myFunction1(12, 20);"'

When I create the buttons dynamically, and try to attach the events in a loop, it gets attached to the last button only. I can sense a closure issue but what am I missing?

Here is the code.

var buttons = [],
arg1 = 12,
arg2 = 20;

var butt1 = { Text: 'Bye', onClick: "anotherFunction();" },
butt = { Text: 'Hello', onClick: "myNewFunction();" },
butt2 = { Text: 'Bye333' };

buttons.push(butt1);
buttons.push(butt);
buttons.push(butt2);


function myNewFunction() {
   alert('my New  Function');
};

function myCloseFunction(arg1, arg2) {
    alert('close: ' + arg1 + ' other: ' + arg2)
}

function anotherFunction() {
    alert('Say Goodbye');
}

 window.onload = function () {
 var buttonContainer = document.getElementById('controlDiv'),
    closeOnClick = "myCloseFunction(" + arg1 + ", " + arg2 + ")",
    button;

for (var i = 0; i < buttons.length; i++) {
    buttons[i].onClick = (buttons[i].onClick == null) ? closeOnClick : (closeOnClick + ';\n' + buttons[i].onClick);
    button = document.createElement("INPUT");
    button.type = 'button';
    button.onclick = new Function(buttons[i].onClick);

    if (i > 0)
        buttonContainer.innerHTML += '&nbsp';
    buttonContainer.appendChild(button);
   }
};

The HTML page is simple:

<!DOCTYPE html>

<html lang="en">
<head>
    <meta charset="utf-8" />
    <title></title>
    <script src="Script.js" type="text/javascript"></script>
</head>
<body>
    <h1>hello</h1>
    <div id="controlDiv"></div>
</body>
</html>

Sensing a closure issue I have tried various options to close over any variables but was not successful. e.g.

button.onclick = (function (action, i) {
    var name1 = 'hello' + i,
        a123 = new Function('action', 'return function ' + name1 + '(){action();}')(action);
        return a123;
    } (new Function(buttons[i].onClick), i));

`buttons[i].onClick = (buttons[i].onClick == null) ? closeOnClick : (closeOnClick + ';\n' + buttons[i].onClick);` By the time your event gets fired (due to a click) `i` is set to `buttons.length` — jmiraglia, May 13 '13 at 20:10
jasonmmiraglia: I have tried the following as well with no success: button.onclick = (function (i) {return new Function(buttons[i].onClick);}(i)); Is that what you mean? — Umair Ishaq, May 13 '13 at 20:15
It is hard to tell what you are trying to accomplish here. Have you read http://stackoverflow.com/questions/3037598/how-to-get-around-the-jslint-error-dont-make-functions-within-a-loop? Do you really need to dynamically build functions, or are dynamic buttons with regular functions sufficient? — Aaron Kurtzhals, May 13 '13 at 20:33
Aaron: I am trying to create some input elements dynamically and attach dynamic functions as event handlers. The issue that should be addressed is why is var i not being closed over properly. — Umair Ishaq, May 13 '13 at 20:40

score 1 · Accepted Answer · answered May 13 '13 at 20:46

1

buttonContainer.innerHTML += '&nbsp'; - This line (or rather re-setting the innerHTML) is the problem. I don't think that the innerHTML property contains any events. Works as expected here.

answered May 13 '13 at 20:46

undefined

2,051
3
26
47

thanks a lot. This code was from some legacy codebase that I have to work with and I am glad you caught this pesky issue. Thanks a lot. – Umair Ishaq May 13 '13 at 20:56

score 0 · Answer 2 · answered May 13 '13 at 20:31

I'd do it like this:

var buttons = [],
    container = document.getElementById('controlDiv');

    buttons.push(new Button('Bye', 'anotherFunction()'));

function Button(text, clickEvent){
    var obj = document.createElement('input'),
        self = this;

    this.text = text;
    this.clickEvent = clickEvent;

    obj.type = 'button';
    container.appendChild(obj);

    obj.addEventListener('click', handleClick);

    function handleClick(e){
        if(self.clickEvent){
            closeOnClick();
            window[self.clickEvent];
        } else {
            closeOnClick();
        }
    };
}

jason: Much appreciated. I was constrained when it comes to rewriting code as it is part of a large legacy project and is heavily used. — Umair Ishaq, May 13 '13 at 20:58

JavaScript closure issue on dynamic element creation in a loop

2 Answers2