ded/script.js

Bug&Fix: Callbacks to unique dependencies (with same files) are not fired

tsptsp opened this issue · 2 comments

Hi there,

I've encountered a bug in the current script.js version (unfortunately there is no version number anywhere).

How to reproduce

Create two separate script loader calls, which actually load the same scripts but with different dependency names. This happened for me when I loaded some controllers remotely, using $script.js for the controllers' dependency management. Both controllers are completely distinct, but have the same script dependencies.

Controller1:

$script(["file1.js","file2.js"], "controller1dependency");
$script.ready("controller1dependency", callback1);

Controller2:

$script(["file1.js","file2.js"], "controller2dependency");
$script.ready("controller2dependency", callback2);

Both controllers are loaded asynchronously, but before both file1.js and file2.js are fetched by $script.js. This is important: If the callback is added after both files are loaded, the callback works (instantly).

Behavior

Only one callback is called by script.js - the second one is ignored (though the callback function is registered in the internal delays object).

Reason

Seems that script.js is not saving the link between the dependency and the scripts to load for each dependency. Therefore, when the last script file of a dependency is fetched, it calls the callback of exactly this dependency - ignoring all the other dependencies which are waiting for this script.

Workaround

None that I could see.

Fix

I've created a small fix which works for me: Add a scriptsById variable in the head of the script.js (just where the doc variable is set) and replace the $script function with the following:

function $script(paths, idOrDone, optDone) {
    paths = paths[push] ? paths : [paths]
    var idOrDoneIsDone = idOrDone && idOrDone.call
      , done = idOrDoneIsDone ? idOrDone : optDone
      , id = idOrDoneIsDone ? paths.join('') : idOrDone
      , queue = paths.length
    function loopFn(item) {
      return item.call ? item() : list[item]
    }
    function callback() {
      if (!--queue) {
        list[id] = 1

        //Delete the loaded script from the scriptsById object - the callback is automatically called
        delete scriptsById[id];
        done && done()

        //FIX: We've to check every open script if it is loaded and if the corresponding id needs to be set to loaded
        for (var itemid in scriptsById) {
          var counter = 0;
          for (var scriptid in scriptsById[itemid]) {
            var scripturl = scriptsById[itemid][scriptid];
            if (scripts[scripturl] > 1) counter++;
          }
          if (counter >= scriptsById[itemid].length) {
            list[itemid] = 1;
            delete scriptsById[itemid];
          }
        }

        for (var dset in delay) {
          every(dset.split('|'), loopFn) && !each(delay[dset], loopFn) && (delay[dset] = [])
        }


      }
    }
    setTimeout(function () {
      //Save the path dependencies for every dependency
      scriptsById[id] = paths;


      each(paths, function (path) {
        if (path === null) return callback()
        if (scripts[path]) {
          id && (ids[id] = 1)
          return scripts[path] == 2 && callback()
        }
        scripts[path] = 1
        id && (ids[id] = 1)
        create(!validBase.test(path) && scriptpath ? scriptpath + path + '.js' : path, callback)
      })
    }, 0)
    return $script
  }

Happy for your feedback - and sorry that I didn't create a pull request out of that, I'm just too unexperienced in GIT ;)

Cheers,
Thomas

ded commented

hi there @tsptsp — great test case. i'll have a closer look to see if i can reproduce myself.

ded commented

hi @tsptsp I believe this has been fixed with a recent patch to allowing the ability to call a duplicate file before the first has completed loading