Rolling my own Require

Question

Forward

It has come to my attention that this problem was a "Function Scoping - Not Block Scoping" answer, the difference being for ... in opposed to for ... i<n ... i++ The solution could be wrapping the for(var p in ...) { with a function to give them their own scope (much like was done with Array.prototype.forEach). Thank you for your help.

Problem

I'm rolling my own tiny Require.js solution as opposed to the actual Require.js library (no I haven't taken a look at their source).

My callback function never seems to be executed, but I can't figure out why. It's probably a simple logical error, but you know how it goes when you stare at your own code for too long. (Everything looks logically sound)

Usage

Used as follows:

require(["LibraryA", "LibraryB", "LibraryC"], function() {
    //code which requires libraries a-c here
});

Code

var require = (function() {
    var scriptsLoaded = [];
    return function(paths, callback) {
        var pathsDoneLoading = {};
        for(var i = 0; i < paths.length; i++) {
            pathsDoneLoading[paths[i]] = false;
        }
        for(var p in pathsDoneLoading) {
            for(var i = 0; i < scriptsLoaded.length; i++) {
                if(p === scriptsLoaded[i]) {
                    pathsDoneLoading[p] = true;
                }
            }
        }
        for(var p in pathsDoneLoading) {
            if(pathsDoneLoading[p] === false) {
                var script = document.createElement("script");
                script.src = p + ".js";
                script.onload = function() {
                    scriptsLoaded.push(p);
                    pathsDoneLoading[p] = true;
                    for(var p in pathsDoneLoading) {
                        if(pathsDoneLoading[p] !== true) {
                            return;
                        }
                    }
                    callback();
                };
                document.documentElement.appendChild(script);
            }
        }
    }
})();

Plunker

https://plnkr.co/edit/OFWTUpmV3sIJAjhGhcLO

Shouldn't the callback be called after all the script elements have been created? Right now it looks like you want to call it after every script has been loaded — Maria Ines Parnisari, Jan 13 '16 at 15:47
You're defining `require` as a self-executing method? Considering dropping the trailing `();` — Sean3z, Jan 13 '16 at 15:47
My thought was, "Return if one of the paths isn't done" which led me to assume "callback by default if we made it here" — Andrue Anderson, Jan 13 '16 at 15:48
@Sean3z it's a IIFE because I need a persistent list of scripts already loaded.. I suppose there are other patterns I could have used, any suggestions? — Andrue Anderson, Jan 13 '16 at 15:49
This problem looks interesting, can you post a reproducible example in Plnkr? https://plnkr.co/edit/ — Maria Ines Parnisari, Jan 13 '16 at 15:55
Possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — JJJ, Jan 13 '16 at 16:12

score 2 · Accepted Answer · answered Jan 13 '16 at 15:57

It's the old for loop variable catching problem.

During your loop where it's iterating through what paths are loading (for (var p in pathsDoneLoading) {) the p is being bound to the function inside. The problem is that p changes and persists after the loop has complete so every single function is adding the last key in the pathsDoneLoading object to the scriptsLoaded array (and setting the matching value in pathsDoneLoading to true).

An example of the problem:

var obj = { a: 1, b: 2, c: 3 };
var funcs = [];
for (var key in obj) {
  funcs.push(function() {
    return key; // Remember, `key` is bound to the function, not the block
  });
}

funcs.forEach(function(f) {
  document.querySelector('pre').innerText += f() + '\n';
});

<pre></pre>

To fix this, you can wrap it in an IIFE to create a new binding per p value.

for (var p in pathsDoneLoading) {
  (function(p) {
    if (pathsDoneLoading[p] === false) { // or `if (!pathsDoneLoading[p])`
    ...
  })(p);
}

That way every time you generate an onload handler, they have a unique binding to the intended path.

Our previous example fixed with this solution:

var obj = { a: 1, b: 2, c: 3 };
var funcs = [];
for (var key in obj) {
  (function(key) {
    funcs.push(function() {
      return key; // Remember, `key` is bound to the function, not the block
    });
  })(key);
}

funcs.forEach(function(f) {
  document.querySelector('pre').innerText += f() + '\n';
});

<pre></pre>

score 0 · Answer 2 · answered Jan 13 '16 at 16:28

Simply put, you'll need capture the for loop iterator because it's a moving target and by the time onload fires, it will have changed.

    var require = (function() {
    var scriptsLoaded = [];
    return function(paths, callback) {
        var pathsDoneLoading = {};
        for(var i = 0; i < paths.length; i++) {
            pathsDoneLoading[paths[i]] = false;
        }
        for(var p in pathsDoneLoading) {
            for(var i = 0; i < scriptsLoaded.length; i++) {
                if(p === scriptsLoaded[i]) {
                    pathsDoneLoading[p] = true;
                }
            }
        }
        for(var p in pathsDoneLoading) {
            if(pathsDoneLoading[p] === false) {
                var script = document.createElement("script");
                script.src = p + ".js";

                // Wrapping in a closure
                script.onload = (function(Vp) {
                    scriptsLoaded.push(Vp);
                    pathsDoneLoading[Vp] = true;
                    for(var prop in pathsDoneLoading) {
                        if(pathsDoneLoading[prop] !== true) {
                            return;
                        }
                    }
                    callback();

                // Execute the closure function and send in your 
                // var(s) as argument(s) to capture it's current state
                }(p));
                document.documentElement.appendChild(script);
            }
        }
    }
})();

Thanks for the answer. By the way, you're not returning anything when you invoke the `onload` IIFE, and your `(p)` parameter syntax is incorrect, but I did implement this solution in my code other than that, and it worked. — Andrue Anderson, Jan 13 '16 at 16:41
Cool. Just trying to keep things simple for you and not over-write your original code. Javascript is rather forgiving. The point to focus on is the p to Vp relationship. — bob, Jan 13 '16 at 17:24

Rolling my own Require

2 Answers2