Self-Invoking Function Scope with Dynamically Binded Content

Question

I have wrapped my JavaScript in a self-invoking function to keep everything contained. Hoever, I have run into an issue where my links, which are dynamically built on the fly, return an error "Function not defined" when they are clicked. Let me include my relevant code:

(function(window, document, $) {

    function buildChapterList() {
        for(var i=0; i<Object.keys(theChapterCues).length; i++) {
            chapterListHTML = "<li><a class='fun-blue' href='javascript:void(0)' onClick=\"skipToChapter('"+ i +"')\">"+ theChapterCues[i]["title"] +"</a></li>";
            $(chapterListHTML).appendTo("ul#chapter-list");
            $("body").bind("DOMNodeInserted", function() {
                $(this).find('#chapter-list li').first().addClass("active");
            });
        }
    }
    function skipToChapter(theChapter) {
        if (theChapter == 0) {
            theVideo.currentTime=0;
        } else {
            var thisChapterStart = parseInt(cues[theChapter]["chapterStart"]+1);
            theVideo.currentTime=thisChapterStart/frameRate;
        }
    }

}(this, this.document, this.jQuery));

When clicking on one of the generated links I receive the following error:

Uncaught ReferenceError: skipToChapter is not defined

Am I missing something on scope? Is this a binding issue? Any advice would be greatly appreciated. Thanks!

Yeah, your function only lives in the scope of your anonymous. You need to make it available in the global scope or find another way to bind the event (would be easier to find an alternative way if you could provide a fiddle) — Robert Falkén, Oct 10 '14 at 14:16

MrCode · Answer 1 · 2014-10-10T14:34:00.803

3

The skipToChapter function is not in the global scope, so can't be called from an inline click handler. Instead you should assign the click handler when you build the link, in an unobtrusive way like below. If you assign it like this, then skipToChapter is in scope, and you don't have to make it a global, and you remove the need for undesirable inline event handlers.

function buildChapterList() {
    for(var i=0; i<Object.keys(theChapterCues).length; i++) {
        chapterListHTML = $("<li><a class='fun-blue' href='javascript:void(0)'>"+ theChapterCues[i]["title"] +"</a></li>");
        (function(i){
            chapterListHTML.find('a').click(skipToChapter.bind(this, i));
        })(i);
        chapterListHTML.appendTo("ul#chapter-list");
        $("body").bind("DOMNodeInserted", function() {
            $(this).find('#chapter-list li').first().addClass("active");
        });
    }
}

edited Oct 10 '14 at 14:34

answered Oct 10 '14 at 14:28

MrCode

59,851
9
76
106

Thank you for the explanation. This is a very clean solution and looks like a good practice to keep going forward. – Yuschick Oct 10 '14 at 14:42
This solution defines a function inside a loop, which is not a good practice. – ncardeli Oct 10 '14 at 14:52
A function in a loop is bad practice? Coming from someone that posted an answer using globals and inline event handlers.....OK. – MrCode Oct 10 '14 at 14:54
@MrCode sometimes a direct to the point solution is what the op needs. I posted that short answer and continued working on a more complete one. Anyway, I just tried to point out that functions inside loops are not the best practices to the op, as he is learning. – ncardeli Oct 10 '14 at 15:05

score 2 · Accepted Answer · edited May 23 '17 at 12:20

skipToChapter function is only visible within the outer anonymous function. You can read about Javascript scope here: http://robertnyman.com/2008/10/09/explaining-javascript-scope-and-closures/

The quick and dirty way of solving the problem would be defining skipToChapter outside of the anonymous function or as a member of the window object.

For example:

window.skipToChapter = function(theChapter) {
    if (theChapter == 0) {
        theVideo.currentTime=0;
    } else {
        var thisChapterStart = parseInt(cues[theChapter]["chapterStart"]+1);
        theVideo.currentTime=thisChapterStart/frameRate;
    }
}

However, be aware that this is not the best practice for binding a function to an event as it makes skipToChapter global and uses inline event handlers (http://robertnyman.com/2008/11/20/why-inline-css-and-javascript-code-is-such-a-bad-thing/).

A better approach would be:

function buildChapterList() {
    for(var i=0; i<Object.keys(theChapterCues).length; i++) {
        chapterListHTML = "<li><a class='fun-blue' href='javascript:void(0)' data-index='" + i + "'>"+ theChapterCues[i]["title"] +"</a></li>";
        var $chapterListHTML = $(chapterListHTML);
        $chapterListHTML.appendTo("ul#chapter-list");
        $chapterListHTML.find('a').click(skipToChapter);
        $("body").bind("DOMNodeInserted", function() {
            $(this).find('#chapter-list li').first().addClass("active");
        });
    }
}
function skipToChapter() {
    var theChapter = $(this).data('index');
    if (theChapter == 0) {
        theVideo.currentTime=0;
    } else {
        var thisChapterStart = parseInt(cues[theChapter]["chapterStart"]+1);
        theVideo.currentTime=thisChapterStart/frameRate;
    }
}

Read this answer for more information on event binding on dinamically created elements, with jQuery: Event binding on dynamically created elements?

This worked for me but can't mark it as the answer just yet. Could you elaborate any more on why this happened and why this fixes it or point me to an article I could read? — Yuschick, Oct 10 '14 at 14:19
My 'global' variables are all enclosed within my anon function, just not included in the code in my original post. And I'm just now learning about better alternatives to inline handlers. So a moral +1 for learning something new. — Yuschick, Oct 10 '14 at 14:45
@Bergi, I improved my answer to include a solution following best practices. Please reconsider your downvote. — ncardeli, Oct 10 '14 at 14:53
@Yuschick actually using this answer, your `skipToChapter` function is now global, which is what Bergi is refering to. — MrCode, Oct 10 '14 at 14:54

Self-Invoking Function Scope with Dynamically Binded Content

2 Answers2