0
\$\begingroup\$

I have JavaScript module for loading dynamically script files by dependency graph:

var module = {};

module.loadScript = function (url, callback) {
    var script = document.createElement('script');
    script.type = 'text/javascript';
    script.src = url;
    script.async = true;
    script.charset = 'utf-8';
    document.body.appendChild(script);
    console.time(script.src + ' Load');

    module.addEvent(script, 'load', function () {
        console.timeEnd(this.src + ' Load');
        document.body.removeChild(this);

        if (typeof callback === "function") {
            callback();
        }
    });
};

module.loadJsTree = function (tree, callback) {
    _loadJsTreeInternal(tree, callback, 0, 0);
};

function _loadJsTreeInternal (tree, callback, _totalCount, _loadedCount) {
    tree = tree || {};

    var branches = Object.keys(tree);
    _totalCount += branches.length;

    branches.forEach(function (branch, index) {
        console.log("Start load ", branch, tree[branch]);

        module.loadScript(branch, function () {
            _loadedCount++;

            if (tree[branch] !== null && typeof tree[branch] === 'object') {
                _loadJsTreeInternal(tree[branch], callback, _totalCount, _loadedCount);
            }
            else if (_loadedCount == _totalCount) {
                if (typeof callback === "function") {
                    callback();
                }
            }
        });
    });
}

//RUN SCRIPT

var scripts = {
    '/script1.js': null,
    '/script2.js': {
        '/script2.plugin1.js': {
            '/script2.plugin1.plugin1.js': null
        },
        '/script2.plugin2.js': null
    }
};

module.loadJsTree(scripts, function () { alert("ALL SCRIPTS READY!") });

Do you see any other improvements / issues?

\$\endgroup\$
  • \$\begingroup\$ We're not a free code writing service. Your request for us to write code for you: "How can I rewrite it, to make callback function fires when all scripts loaded?" is off-topic here. \$\endgroup\$ – Peilonrayz Aug 17 '17 at 11:54
  • \$\begingroup\$ count how many scripts you are adding and when loaded (either succesfull or failure) decrease the counter again. if the counter hits 0, you have no more scripts that need to be loaded and you can call the callback, if not, don't do anything \$\endgroup\$ – Icepickle Aug 17 '17 at 11:57
  • \$\begingroup\$ Thanks for your reply Icepickle. But where do I need to place counter variable? I don't think that global variable is a good idea. \$\endgroup\$ – sDima Aug 17 '17 at 12:35
  • 1
    \$\begingroup\$ it can be a property of your loadJsTree function, functions are objects too, which means they can contain properties, and a counter could be good there \$\endgroup\$ – Icepickle Aug 17 '17 at 12:54
  • \$\begingroup\$ I completely rewrote the code, can you watch it again? Is that what you meant? \$\endgroup\$ – sDima Aug 17 '17 at 13:35
2
\$\begingroup\$

Your code is already good, but there are just a few things to note:

  • You are not in control which order the browser will complete the loading of the scripts you have added to the DOM, that might be completely random, especially when its running on a remote server.

  • You are sending the _totalLoaded and _totalLoading as parameters. Though this is working, you don't have to do that. You can define properties on a function, eg: on module.loadJsTree.

  • You don't need 2 properties, 1 property starting from 0 that is incremented before loading, and the callback decreases it again after the loading. As no decrements will be called while you are attaching your scripts to the DOM, you are perfectly safe (it will complete the foreach loop, having all DOM scripts attached before it can call the callback that decreases them again)

A similar system can be created like this (note, this is not with loading scripts, but just setting timeouts, however the principle is pretty much the same, also note that the order of loads is not necessarily the same as I typed them, it's important to node that keys on an object don't have a guaranteed specified sequence.

var module = {};

module.randomLoadWithCallback = (time, callback) => {
  console.log(`${time} ms wait before callback`);
  setTimeout(() => callback(), time);
};

module.loadTree = function _treeLoader(tree, callback) {
  if (!_treeLoader.count) {
    _treeLoader.count = 0;
  }
  const completedCallback = () => {
    _treeLoader.count--;
    console.log(_treeLoader.count);
    _treeLoader.count === 0 && callback && callback.apply && callback();
  };
  if (tree) {
    Object.keys(tree).forEach(item => {
      _treeLoader.count++;
      let subTree = tree[item];
      module.randomLoadWithCallback(item, () => {
        console.log(`completed loading ${item}`);
        _treeLoader(subTree, callback);
        completedCallback();
      });
    });
  }
};

const timeEntryTree = {
  50: {
    55: {
      700: null
    },
    1000: null,
    100: {
      50: null,
      40: null,
      30: {
        10: {
          5: null,
          0: null,
          1: null
        }
      }
    }
  }
};

module.loadTree(timeEntryTree, () => console.log('completely loaded'));

Some points to note would be the different declaration of the loadTree function:

module.loadTree = function _treeLoader(tree, callback) {

To the outside world, this function is accessible through module.loadTree, however inside the function, I can access it through _treeLoader.

Another thing to note, would be the that you can assign properties to functions, which can be handy sometimes, in our case, to assign it with a counter:

if (!_treeLoader.count) {
  _treeLoader.count = 0;
}

In the completedCallback, this counter will be decreased, and if it reaches 0, it will fire the eventual real callback

const completedCallback = () => {
  _treeLoader.count--;
  console.log( _treeLoader.count );
  _treeLoader.count === 0 && callback && callback.apply && callback();
};

async/await implementation

Since you expressed interest in loading through Promise, I thought I would offer 1 version that loads the promises through the async / await pattern.

It's important to note, that this pattern is not fully implemented in all browsers (eg: Internet Explorer).

As you didn't specify if ES6 would be viable for you, I don't know if it fits your use case, though you could use babeljs to transpile it to browser compatible JavaScript.

const module = {
  loadDependency: async function(dependencyTimeout) {
    return new Promise( (resolve, reject) => {
      setTimeout( () => resolve(), dependencyTimeout );
    });
  },
  loadTree: async function(tree) {
    return new Promise( async(resolve, reject) => {
      if (tree) {
        for (let item in tree) {
          console.log( `loading ${item}` );
          await this.loadDependency( item );
          await this.loadTree( tree[item] );
        }
      }
      resolve();
    });
  }
};

let times = {
  50: null,
  55: {
    60: null,
    75: {
      10: null,
      100: null
    }
  }
};

module.loadTree( times ).then( () => console.log( 'loading completed' ) );
console.log('loading started');

Another important node is that this code will wait for each loadDependency to complete. However, it will not block any of your code. Hence, you will see loading started in the console before any other message.

\$\endgroup\$
2
\$\begingroup\$

I have JavaScript module for loading dynamically script files by dependency graph:

But... why? There are libraries out there that already do this. Take for instance, RequireJS. Most libraries out there ship as UMD modules (defined as CommonJS, AMD, and global), you can easily drop them into RequireJS-like or Browserify-like workflows.

Also, if you already know the order at which the modules come in (because of your deps graph), why not take a step further and just use ES modules together with a bundler like Rollup? Together with Uglify, you can strip away unnecessary bloat. Aside from the tooling, ES modules explicitly define their dependencies. You don't need an external deps map.

Async loading of modules all at once only prevent render-blocking and make your site look fast. It does not speed up loading of the site. What makes sites faster is smaller or lesser dependencies, or async loading plus loading only what's needed at the moment.

Consider using the tools existing in the ecosystem instead of building your own.

\$\endgroup\$
  • 1
    \$\begingroup\$ Sometimes it's the challenge ;) Not everybody likes using tools, if they don't have a small idea how they work internally. I have written many things that in the end were just for me, trying to find out how certain things tick, maybe the OP feels the same \$\endgroup\$ – Icepickle Aug 17 '17 at 15:08
  • \$\begingroup\$ "Together with X, you can strip away unnecessary bloat." Sounds more like you're adding the bloat. \$\endgroup\$ – Mast Aug 17 '17 at 16:13
  • \$\begingroup\$ @Mast "Sounds more like you're adding the bloat" - and how would that be? Rollup and Uglify are build-time tools, not runtime libraries. \$\endgroup\$ – Joseph Aug 17 '17 at 16:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.