r96964 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96963‎ | r96964 | r96965 >
Date:13:55, 13 September 2011
Author:catrope
Status:ok
Tags:
Comment:
Address one of the fixmes on r88053: because addScript() is asynchronous, calling it repeatedly can lead to scripts loading out of order. The specific bug was that if you had a module with two script files, A and B (in that order; scripts are always loaded in the order in which they are specified for multi-file modules) and A is 40M while B is 1K, then B would be executed before A if the module is loaded after document ready (through e.g. mw.loader.using) and the files are not in the browser cache.

So instead of calling addScript() repeatedly in a loop, this commit calls addScript() recursively from its own callback, using a helper function called nestedAddScript(). This also means the if block that checks if the scripts array is empty can be removed, because nestedAddScript() handles empty arrays correctly.
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -546,22 +546,24 @@
547547 if ( $.isFunction( callback ) ) {
548548 callback();
549549 }
 550+ },
 551+ nestedAddScript = function( arr, callback, i ) {
 552+ // Recursively call addScript() in its own callback
 553+ // for each element of arr.
 554+ if ( i >= arr.length ) {
 555+ // We're at the end of the array
 556+ callback();
 557+ return;
 558+ }
 559+
 560+ addScript( arr[i], function() {
 561+ nestedAddScript( arr, callback, i + 1 );
 562+ } );
550563 };
551564
552565 if ( $.isArray( script ) ) {
553 - var done = 0;
554 - if ( script.length === 0 ) {
555 - // No scripts in this module? Let's dive out early.
556 - markModuleReady();
557 - }
558 - for ( var i = 0; i < script.length; i++ ) {
559 - registry[module].state = 'loading';
560 - addScript( script[i], function() {
561 - if ( ++done === script.length ) {
562 - markModuleReady();
563 - }
564 - } );
565 - }
 566+ registry[module].state = 'loading';
 567+ nestedAddScript( script, markModuleReady, 0 );
566568 } else if ( $.isFunction( script ) ) {
567569 script( $ );
568570 markModuleReady();
@@ -703,7 +705,7 @@
704706 function addScript( src, callback ) {
705707 var done = false, script;
706708 if ( ready ) {
707 - // jQuery's getScript method is NOT better than doing this the old-fassioned way
 709+ // jQuery's getScript method is NOT better than doing this the old-fashioned way
708710 // because jQuery will eval the script's code, and errors will not have sane
709711 // line numbers.
710712 script = document.createElement( 'script' );

Follow-up revisions

RevisionCommit summaryAuthorDate
r97275REL1_18 MFT r96964reedy14:31, 16 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r88053Added direct file loading functionality to debug mode for both scripts and st...tparscal12:15, 14 May 2011

Status & tagging log