Discussion :

[Darkaurora]

Bonjour à tous, j'aimerais recevoir les critiques d'une fonction que j'utilise de temps en temps. En fait je ne sais pas si il est intéressant de m'en servir, si elle respecte les bonnes pratiques de développement ou si tout simplement elle est bien développé mais en tout cas je tiens à préciser que j'ai pris plaisir à la créer

Il s'agit simplement d'un loader de script qui permet de charger des scripts (obvious) dans le bon ordre (c'est la ou je voulais en venir ). Bref voici le code et l'utilisation.
loader.js:

Code jQuery :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
 
var loadScript = (function ($) {
  if ($) {
    jQuery.loadScript = function (url, options) {
      options = $.extend(options || {}, {
        dataType: "script",
        async: false,
        url: url
      });
 
      return jQuery.ajax(options);
    };
 
    return function (urls, path) {
      var queue = urls.map(function (script) {
          return $.loadScript((typeof path !== 'string' ? '' : path) + script + '.js');
      });
 
      $.when.apply(null, queue)
        .done(function () {
          var strUrl = '';
 
          for (var i = 0; i < arguments.length; i++) {
            strUrl += ' "' + urls[i] + '"' + (i !== arguments.length-1 ? ',' : '');
          }
 
          console.log('%c' + strUrl + (urls.length > 1 ? ' have' : ' has') + ' been loaded', 'color: blue;');
        })
 
        .fail(function (a, b, c) {
          for (var i = 0; i < urls.length; i++) {
            if (this.url.split('?')[0] === urls[i]) {
              throw new Error('failed to load ' + urls[i] + ', Message "' + b + ': ' + c + '"');
            }
          }
        });
    }
  }
}($));

appel.html

Code HTML :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
 
<script src="js/loader.js"></script>
<script type="text/javascript">
  loadScript(
    ['leaflet-src', 'NonTiledLayer', 'NonTiledLayer.WMS', 'google', 'tooltip', 'map.handler', 'local', 'utils', 'intersects', 'leaflet.contextmenu'], 
    'js/map/'
  );
</script>

[Watilin]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

        async: false,

Là tu fais un appel Ajax synchrone donc. Qui bloque le thread UI. C'est voulu ?
Tout le monde n'a pas mon opinion mais pour moi on ne bloque le thread UI que quand on a besoin de bloquer le thread UI. Le cas d'école, c'est quand on veut faire une opération de dernière minute (typiquement une sauvegarde) avant la fermeture de la page, sur l'évènement unload (lire à ce sujet : Don't let the door hit you…). Toute autre forme d'Ajax synchrone est à éviter, mais je le répète, ce n'est que mon humble avis. Tant que tu construis ta page selon le principe d'amélioration progressive, je ne vois pas d'inconvénient à laisser l'utilisateur utiliser la page avant que les scripts aient tous fini de charger.

Du coup si tu touches à cet aspect synchrone, il faudra revoir la façon dont les urls sont mappés dans queue car, en l'état, elles sont toutes mappés en même temps donc les chargements sont tous initiés en même temps.

Autre chose, j'aurais passé jQuery plutôt que $ à ma fonction wrapper, comme ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
var loadScript = (function ($) {
 
   ...
 
}(jQuery));

Tu déclares loadScript dans l'espace global. Pourquoi ?

Et sinon, j'aurais pris la peine de donner des noms un peu plus explicites aux arguments a, b et c.

Le reste me paraît élégant. C'est toujours agréable de travailler avec les promises


Sur la question « s'il est intéressant de s'en servir », oui bien sûr, son usage est légitime. En gros elle fonctionne comme l'attribut defer, qui existe depuis IE4, en assurant un comportement correct là où les navigateurs ne respectent pas toujours la spec (entre autres IE8 et Safari 4 – détails : The script defer attribute).

Edit: En fait, il semblerait que la donne n'ait pas beaucoup changé depuis 2007…

[Darkaurora]

Là tu fais un appel Ajax synchrone donc. Qui bloque le thread UI. C'est voulu ?

Oui c'est voulu en fait il existe une partie des sources qui utilise les scripts chargés par loadScript. Sans cette synchronisation le reste des scripts plantent. Pour palier a cet inconvénient (car je suis d'accord avec toi) il faudrait peut être un callback ou un autre procédé qui temporise.

Tu déclares loadScript dans l'espace global. Pourquoi ?

Tout simplement pour utiliser la fonction par la suite, je ne sais pas comment faire autrement

Sinon merci pour ces remarques, c'est en partie grâce à vous que j'ai appris ce que je sais du JS et je n'ai pas envie de m'arrêter la

[Watilin]

Oui c'est voulu en fait il existe une partie des sources qui utilise les scripts chargés par loadScript. Sans cette synchronisation le reste des scripts plantent. Pour palier a cet inconvénient (car je suis d'accord avec toi) il faudrait peut être un callback ou un autre procédé qui temporise.

Vite fait je dirais une fonction récursive, enfin indirectement récursive, puisqu'elle s'auto-ajouterait en callback de l'évènement load du prochain script, une fonction asynchroniquement récursive si tu vois ce que je veux dire…

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
var queue = [ url1, url2, ... ];
 
(function loadNextScript() {
   if (queue.length) {
      var newScript = document.createElement("script");
      newScript.src = queue.shift();
      newScript.onload = loadNextScript; // récursivité
      document.body.appendChild(newScript);
   }
}()); // appel immédiat

Bon j'ai pas testé ce bout de code, mais tu saisis l'idée. Grâce à shift qui fait sortir les éléments par la gauche du tableau, on respecte l'ordre des urls, et on est sûr qu'un script ne commence pas à charger avant que le précédent ait terminé. Après pour être complet il faudrait gérer les erreurs, et là ça devient intéressant de le faire avec des promises. Hmm, je vais me pencher dessus…

Tout simplement pour utiliser la fonction par la suite, je ne sais pas comment faire autrement

Étant donné que tu l'ajoutes à l'espace de noms jQuery, tu as déjà une manière d'y accéder par $.loadScript. À propos, en relisant ton code, je ne suis pas sûr de comprendre les endroits où tu alternes $ et jQuery. Par exemple ici :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
if ($) {
  jQuery.loadScript = ...

[Darkaurora]

C'est vrai qu'en le relisant tout à l'heure je me suis fait la même remarque que j'ai corrigé tout de suite après d'ailleurs. Cependant la fonction jQuery.loadScript et loadScript ne sont pas du tout les mêmes fonctions en fait j'utilise jQuery.loadScript dans loadScript

Pour ce qui est de la récursivité ça ne suffira pas, certes les scripts vont se charger les uns à la suite des autres mais pour l'exemple suivant je soulève une erreur qui interrompt l’exécution du main script:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
<script src="js/loader.js"></script>
<script type="text/javascript">
  loadScript(
    ['leaflet-src', 'NonTiledLayer', 'NonTiledLayer.WMS', 'google', 'tooltip', 'map.handler', 'local', 'utils', 'intersects', 'leaflet.contextmenu'], 
    'js/map/'
  );
 
var map = L.map('map').setView([51.505, -0.09], 13);
</script>

La lib leaflet n'étant pas chargé une erreur est levé et ça plante. Je pensais plus à quelque chose de cette manière:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
<script src="js/loader.js"></script>
<script type="text/javascript">
  loadScript(
    ['leaflet-src', 'NonTiledLayer', 'NonTiledLayer.WMS', 'google', 'tooltip', 'map.handler', 'local', 'utils', 'intersects', 'leaflet.contextmenu'], 
    'js/map/',
    function () {
      var map = L.map('map').setView([51.505, -0.09], 13);
  });
</script>

Le problème c'est que ça serait très moche avec un code conséquent. Grosso modo l'utilité d'une telle fonction (de mon point de vue) est de chargé facilement toutes les dépendances ou plugin d'une classe avant de les utiliser.

[danielhagnoul]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
<script>
    ( function loadScript( $, queue ){
        if ( queue && queue.length ){
            var jqXHR = $.getScript( queue.shift() )
 
            setTimeout( function(){
                // Si l'objet différé existe dans l'état "attendre", 
                // alors exécute la méthode fail()
 
                if ( jqXHR && jqXHR.state() === "pending" ){
                jqXHR.abort();
                }
            }, 3000 ); // 3s
 
            jqXHR.done( function( data, textStatus, jqXHR ){
                // succès de la transaction, on doit traiter le contenu de data
                console.log( data, textStatus, jqXHR );
 
            });
 
            jqXHR.fail( function( jqXHR, textStatus, errorThrown ){
                // échec de la transaction, gérer la catastrophe
                console.log( jqXHR, textStatus, errorThrown );
 
            });
 
            jqXHR.always( function( jqXHR, textStatus ){
                // la transaction est terminée
                // nettoyage
                // exécution d'un code dépendant
                //console.log( jqXHR, textStatus );
 
                loadScript( $, queue );
            });
        }
    }( jQuery, [ "config.js", "data.js" ] ));
</script>

[Watilin]

Cependant la fonction jQuery.loadScript et loadScript ne sont pas du tout les mêmes fonctions en fait j'utilise jQuery.loadScript dans loadScript

En effet. J'ai fait une bourde Mais alors, tu as deux fonctions, loadScript et $.loadScript, qui sont toutes les deux accessibles depuis l'extérieur et qui ne font pas la même chose. Ça me dérange quand même un petit peu.

@Daniel : c'est une réécriture complète que tu nous proposes là