resourceloader: Combine base modules and page modules requests
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 12 Jul 2018 20:09:28 +0000 (13:09 -0700)
committerKrinkle <krinklemail@gmail.com>
Wed, 8 Aug 2018 02:56:50 +0000 (02:56 +0000)
This commit implements step 4 and step 5 of the plan outlined at T192623.

Before this task began, the typical JavaScript execution flow was:

* HTML triggers request for startup module (js req 1).
* Startup module contains registry, site config, and triggers
  a request for the base modules (js req 2).
* After the base modules arrive (which define jQuery and mw.loader),
  the startup module invokes a callback that processes RLQ,
  which is what will request modules for this page (js req 3).

In past weeks, we have:

* Made mediawiki.js independent of jQuery.
* Spun off 'mediawiki.base' from mediawiki.js – for everything
  that wasn't needed for defining `mw.loader`.
* Moved mediawiki.js from the base module request to being embedded
  as part of startup.js.

The concept of dependencies is native to ResourceLoader, and thanks to the
use of closures in mw.loader.implement() responses, we can download any
number of interdependant modules in a single request (or parallel requests).
Then, when a response arrives, mw.loader takes care to pause or resume
execution as-needed. It is normal for ResourceLoader to batch several modules
together, including their dependencies.

As such, we can eliminate one of the two roundtrips required before a
page can request modules. Specifically, we can eliminate "js req 2" (above),
by making the two remaining base modules ("jquery" and "mediawiki.base") an
implied dependency for all other modules, which ResourceLoader will naturally
fetch and execute in the right order as part of the batch request.

Bug: T192623
Change-Id: I17cd13dffebd6ae476044d8d038dc3974a1fa176

includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderStartUpModule.php
includes/specials/SpecialJavaScriptTest.php
maintenance/jsduck/eg-iframe.html
resources/Resources.php
resources/lib/jquery/jquery.migrate.js
resources/src/mediawiki.base/mediawiki.base.js
resources/src/startup/mediawiki.js
resources/src/startup/startup.js
tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js

index a0acf1f..637bae6 100644 (file)
@@ -1484,7 +1484,7 @@ MESSAGE;
        }
 
        /**
-        * Wraps JavaScript code to run after startup and base modules.
+        * Wraps JavaScript code to run after the startup module.
         *
         * @param string $script JavaScript code
         * @return string JavaScript code
index a4fd712..53c10df 100644 (file)
@@ -319,17 +319,6 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                return true;
        }
 
-       /**
-        * @param ResourceLoaderContext $context
-        * @return array
-        */
-       public function getPreloadLinks( ResourceLoaderContext $context ) {
-               $url = $this->getBaseModulesUrl( $context );
-               return [
-                       $url => [ 'as' => 'script' ]
-               ];
-       }
-
        /**
         * Internal modules used by ResourceLoader that cannot be depended on.
         *
@@ -353,10 +342,18 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                return [];
        }
 
+       /**
+        * @private For internal use by SpecialJavaScriptTest
+        * @since 1.32
+        * @return array
+        */
+       public function getBaseModulesInternal() {
+               return $this->getBaseModules();
+       }
+
        /**
         * Base modules implicitly available to all modules.
         *
-        * @since 1.32
         * @return array
         */
        private function getBaseModules() {
@@ -370,25 +367,6 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                return $baseModules;
        }
 
-       /**
-        * Get the load URL of the startup modules.
-        *
-        * This is a helper for getScript().
-        *
-        * @param ResourceLoaderContext $context
-        * @return string
-        */
-       private function getBaseModulesUrl( ResourceLoaderContext $context ) {
-               $rl = $context->getResourceLoader();
-               $derivative = new DerivativeResourceLoaderContext( $context );
-               $derivative->setModules( $this->getBaseModules() );
-               $derivative->setOnly( 'scripts' );
-               // Must setModules() before makeVersionQuery()
-               $derivative->setVersion( $rl->makeVersionQuery( $derivative ) );
-
-               return $rl->createLoaderURL( 'local', $derivative );
-       }
-
        /**
         * @param ResourceLoaderContext $context
         * @return string JavaScript code
@@ -399,35 +377,43 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        return '/* Requires only=script */';
                }
 
-               $out = file_get_contents( "$IP/resources/src/startup/startup.js" );
+               $startupCode = file_get_contents( "$IP/resources/src/startup/startup.js" );
 
-               // Keep in sync with maintenance/jsduck/eg-iframe.html and,
-               // keep in sync with 'fileHashes' in StartUpModule::getDefinitionSummary().
+               // The files read here MUST be kept in sync with maintenance/jsduck/eg-iframe.html,
+               // and MUST be considered by 'fileHashes' in StartUpModule::getDefinitionSummary().
                $mwLoaderCode = file_get_contents( "$IP/resources/src/startup/mediawiki.js" ) .
                        file_get_contents( "$IP/resources/src/startup/mediawiki.requestIdleCallback.js" );
                if ( $context->getDebug() ) {
                        $mwLoaderCode .= file_get_contents( "$IP/resources/src/startup/mediawiki.log.js" );
                }
 
-               $pairs = array_map( function ( $value ) {
+               $mapToJson = function ( $value ) {
                        $value = FormatJson::encode( $value, ResourceLoader::inDebugMode(), FormatJson::ALL_OK );
                        // Fix indentation
                        $value = str_replace( "\n", "\n\t", $value );
                        return $value;
-               }, [
+               };
+
+               // Perform replacements for mediawiki.js
+               $mwLoaderCode = strtr( $mwLoaderCode, [
+                       '$VARS.baseModules' => $mapToJson( $this->getBaseModules() ),
+               ] );
+
+               // Perform replacements for startup.js
+               $pairs = array_map( $mapToJson, [
                        '$VARS.wgLegacyJavaScriptGlobals' => $this->getConfig()->get( 'LegacyJavaScriptGlobals' ),
                        '$VARS.configuration' => $this->getConfigSettings( $context ),
-                       // This url may be preloaded. See getPreloadLinks().
-                       '$VARS.baseModulesUri' => $this->getBaseModulesUrl( $context ),
                ] );
+               // Raw JavaScript code (not for JSON)
                $pairs['$CODE.registrations();'] = str_replace(
                        "\n",
                        "\n\t",
                        trim( $this->getModuleRegistrations( $context ) )
                );
                $pairs['$CODE.defineLoader();'] = $mwLoaderCode;
+               $startupCode = strtr( $startupCode, $pairs );
 
-               return strtr( $out, $pairs );
+               return $startupCode;
        }
 
        /**
index b786c86..f858f5c 100644 (file)
@@ -103,65 +103,56 @@ class SpecialJavaScriptTest extends SpecialPage {
                $query['only'] = 'scripts';
                $startupContext = new ResourceLoaderContext( $rl, new FauxRequest( $query ) );
 
-               $query['raw'] = true;
-
                $modules = $rl->getTestModuleNames( 'qunit' );
 
                // Disable autostart because we load modules asynchronously. By default, QUnit would start
                // at domready when there are no tests loaded and also fire 'QUnit.done' which then instructs
-               // Karma to end the run before the tests even started.
+               // Karma to exit the browser process before the tests even finished loading.
                $qunitConfig = 'QUnit.config.autostart = false;'
                        . 'if (window.__karma__) {'
                        // karma-qunit's use of autostart=false and QUnit.start conflicts with ours.
-                       // Hack around this by replacing 'karma.loaded' with a no-op and call it ourselves later.
-                       // See <https://github.com/karma-runner/karma-qunit/issues/27>.
+                       // Hack around this by replacing 'karma.loaded' with a no-op and perfom its duty of calling
+                       // `__karma__.start()` ourselves. See <https://github.com/karma-runner/karma-qunit/issues/27>.
                        . 'window.__karma__.loaded = function () {};'
                        . '}';
 
                // The below is essentially a pure-javascript version of OutputPage::headElement().
-               $startup = $rl->makeModuleResponse( $startupContext, [
+               $code = $rl->makeModuleResponse( $startupContext, [
                        'startup' => $rl->getModule( 'startup' ),
                ] );
-               // Embed page-specific mw.config variables.
-               // The current Special page shouldn't be relevant to tests, but various modules (which
-               // are loaded before the test suites), reference mw.config while initialising.
-               $code = ResourceLoader::makeConfigSetScript( $out->getJSVars() );
-               // Embed private modules as they're not allowed to be loaded dynamically
-               $code .= $rl->makeModuleResponse( $embedContext, [
-                       'user.options' => $rl->getModule( 'user.options' ),
-                       'user.tokens' => $rl->getModule( 'user.tokens' ),
-               ] );
-               // Catch exceptions (such as "dependency missing" or "unknown module") so that we
-               // always start QUnit. Re-throw so that they are caught and reported as global exceptions
-               // by QUnit and Karma.
-               $modules = Xml::encodeJsVar( $modules );
-               $code .= <<<CODE
-(function () {
+               // The following has to be deferred via RLQ because the startup module is asynchronous.
+               $code .= ResourceLoader::makeLoaderConditionalScript(
+                       // Embed page-specific mw.config variables.
+                       // The current Special page shouldn't be relevant to tests, but various modules (which
+                       // are loaded before the test suites), reference mw.config while initialising.
+                       ResourceLoader::makeConfigSetScript( $out->getJSVars() )
+                       // Embed private modules as they're not allowed to be loaded dynamically
+                       . $rl->makeModuleResponse( $embedContext, [
+                               'user.options' => $rl->getModule( 'user.options' ),
+                               'user.tokens' => $rl->getModule( 'user.tokens' ),
+                       ] )
+                       // Load all the test suites
+                       . Xml::encodeJsCall( 'mw.loader.load', [ $modules ] )
+               );
+               $encModules = Xml::encodeJsVar( $modules );
+               $code .= ResourceLoader::makeInlineCodeWithModule( 'mediawiki.base', <<<JAVASCRIPT
        var start = window.__karma__ ? window.__karma__.start : QUnit.start;
-       try {
-               mw.loader.using( $modules )
-                       .always( function () {
-                               start();
-                       } )
-                       .fail( function ( e ) {
-                               setTimeout( function () {
-                                       throw e;
-                               } );
-                       } );
-       } catch ( e ) {
-               start();
-               throw e;
-       }
-}());
-CODE;
+       mw.loader.using( $encModules ).always( start );
+       mw.trackSubscribe( 'resourceloader.exception', function ( topic, err ) {
+               // Things like "dependency missing" or "unknown module".
+               // Re-throw so that they are reported as global exceptions by QUnit and Karma.
+               setTimeout( function () {
+                       throw e;
+               } );
+       } );
+JAVASCRIPT
+       );
 
                header( 'Content-Type: text/javascript; charset=utf-8' );
                header( 'Cache-Control: private, no-cache, must-revalidate' );
                header( 'Pragma: no-cache' );
                echo $qunitConfig;
-               echo $startup;
-               // The following has to be deferred via RLQ because the startup module is asynchronous.
-               echo ResourceLoader::makeLoaderConditionalScript( $code );
+               echo $code;
        }
 
        private function plainQUnit() {
index 2c7cd68..7913aab 100644 (file)
@@ -35,7 +35,9 @@
        </script>
        <script>
                // Mock startup.js
-               var mwNow = Date.now;
+               window.$VARS = {
+                       baseModules: []
+               };
 
                function startUp() {
                        mw.config = new mw.Map();
index c63eb96..a06a25f 100644 (file)
@@ -143,7 +143,6 @@ return [
                        'resources/lib/jquery/jquery.js',
                        'resources/lib/jquery/jquery.migrate.js',
                ],
-               'raw' => true,
                'targets' => [ 'desktop', 'mobile' ],
        ],
 
@@ -834,11 +833,14 @@ return [
        /* MediaWiki */
 
        'mediawiki.base' => [
-               // Keep in sync with maintenance/jsduck/eg-iframe.html
                'scripts' => [
+                       // This MUST be kept in sync with maintenance/jsduck/eg-iframe.html
                        'resources/src/mediawiki.base/mediawiki.errorLogger.js',
                        'resources/src/mediawiki.base/mediawiki.base.js',
                ],
+               // - These dependencies MUST NOT also have dependencies (would cause recursion).
+               // - These dependencies MUST also be returned from StartUpModule::getBaseModules().
+               'dependencies' => 'jquery',
                'targets' => [ 'desktop', 'mobile' ],
        ],
        'mediawiki.apihelp' => [
index 992a734..711e424 100644 (file)
@@ -3,6 +3,9 @@
  * Copyright jQuery Foundation and other contributors
  *
  * Patched for MediaWiki:
+ * - Qualify the global lookup for 'jQuery' as 'window.jQuery',
+ *   because within mw.loader.implement() for 'jquery', the closure
+ *   specifies '$' and 'jQuery', which are undefined.
  * - Add mw.track instrumentation for statistics.
  * - Disable jQuery.migrateTrace by default. They are slow and
  *   redundant given console.warn() already provides a trace.
@@ -20,7 +23,8 @@
        } else {
 
                // Browser globals
-               factory( jQuery, window );
+               // PATCH: Qualify jQuery lookup as window.jQuery. --Krinkle
+               factory( window.jQuery, window );
        }
 } )( function( jQuery, window ) {
 "use strict";
index 1c3dde6..9536b67 100644 (file)
@@ -22,7 +22,8 @@
                mwLoaderTrack = mw.track,
                trackCallbacks = $.Callbacks( 'memory' ),
                trackHandlers = [],
-               hasOwn = Object.prototype.hasOwnProperty;
+               hasOwn = Object.prototype.hasOwnProperty,
+               queue;
 
        /**
         * Object constructor for messages.
        // Alias $j to jQuery for backwards compatibility
        // @deprecated since 1.23 Use $ or jQuery instead
        mw.log.deprecate( window, '$j', $, 'Use $ or jQuery instead.' );
+
+       // Process callbacks for Grade A that require modules.
+       // Plain ones were already processed by startup.js.
+       queue = window.RLQ;
+       // Redefine publicly to capture any late arrivals
+       window.RLQ = {
+               push: function ( entry ) {
+                       mw.loader.using( entry[ 0 ], entry[ 1 ] );
+               }
+       };
+       while ( queue[ 0 ] ) {
+               window.RLQ.push( queue.shift() );
+       }
 }() );
index e2db9ea..351acc3 100644 (file)
@@ -7,6 +7,7 @@
  * @alternateClassName mediaWiki
  * @singleton
  */
+/* global $VARS */
 
 ( function () {
        'use strict';
                                 */
                                jobs = [],
 
+                               /**
+                                * @private
+                                * @property {Array} baseModules
+                                */
+                               baseModules = $VARS.baseModules,
+
                                /**
                                 * For #addEmbeddedCSS() and #addLink()
                                 *
                                return true;
                        }
 
+                       /**
+                        * Determine whether all direct and base dependencies are in state 'ready'
+                        *
+                        * @private
+                        * @param {string} module Name of the module to be checked
+                        * @return {boolean} True if all direct/base dependencies are in state 'ready'; false otherwise
+                        */
+                       function allWithImplicitReady( module ) {
+                               return allReady( registry[ module ].dependencies ) &&
+                                       ( baseModules.indexOf( module ) !== -1 || allReady( baseModules ) );
+                       }
+
                        /**
                         * Determine whether all dependencies are in state 'ready', which means we may
                         * execute the module or job now.
                         * @param {string} module Name of module that entered one of the states 'ready', 'error', or 'missing'.
                         */
                        function handlePending( module ) {
-                               var j, job, hasErrors, m, stateChange;
+                               var j, job, hasErrors, m, stateChange, fromBaseModule;
 
                                if ( registry[ module ].state === 'error' || registry[ module ].state === 'missing' ) {
+                                       fromBaseModule = baseModules.indexOf( module ) !== -1;
                                        // If the current module failed, mark all dependent modules also as failed.
                                        // Iterate until steady-state to propagate the error state upwards in the
                                        // dependency tree.
                                                stateChange = false;
                                                for ( m in registry ) {
                                                        if ( registry[ m ].state !== 'error' && registry[ m ].state !== 'missing' ) {
-                                                               if ( anyFailed( registry[ m ].dependencies ) ) {
+                                                               // Always propagate errors from base modules to regular modules (implicit dependency).
+                                                               // Between base modules or regular modules, consider direct dependencies only.
+                                                               if (
+                                                                       ( fromBaseModule && baseModules.indexOf( m ) === -1 ) ||
+                                                                       anyFailed( registry[ m ].dependencies )
+                                                               ) {
                                                                        registry[ m ].state = 'error';
                                                                        stateChange = true;
                                                                }
                                        }
                                }
 
+                               // The current module became 'ready'.
                                if ( registry[ module ].state === 'ready' ) {
-                                       // The current module became 'ready'. Set it in the module store, and recursively execute all
-                                       // dependent modules that are loaded and now have all dependencies satisfied.
+                                       // Save it to the module store.
                                        mw.loader.store.set( module, registry[ module ] );
+                                       // Recursively execute all dependent modules that were already loaded
+                                       // (waiting for execution) and no longer have unsatisfied dependencies.
                                        for ( m in registry ) {
-                                               if ( registry[ m ].state === 'loaded' && allReady( registry[ m ].dependencies ) ) {
+                                               // Base modules may have dependencies amongst eachother to ensure correct
+                                               // execution order. Regular modules wait for all base modules.
+                                               if ( registry[ m ].state === 'loaded' && allWithImplicitReady( m ) ) {
                                                        // eslint-disable-next-line no-use-before-define
                                                        execute( m );
                                                }
                                if ( !unresolved ) {
                                        unresolved = new StringSet();
                                }
+
+                               // Add base modules
+                               if ( baseModules.indexOf( module ) === -1 ) {
+                                       baseModules.forEach( function ( baseModule ) {
+                                               if ( resolved.indexOf( baseModule ) === -1 ) {
+                                                       resolved.push( baseModule );
+                                               }
+                                       } );
+                               }
+
                                // Tracks down dependencies
                                deps = registry[ module ].dependencies;
                                for ( i = 0; i < deps.length; i++ ) {
                                                if ( Array.isArray( script ) ) {
                                                        nestedAddScript( script, markModuleReady, 0 );
                                                } else if ( typeof script === 'function' ) {
-                                                       // Pass jQuery twice so that the signature of the closure which wraps
-                                                       // the script can bind both '$' and 'jQuery'.
-                                                       script( window.$, window.$, mw.loader.require, registry[ module ].module );
+                                                       if ( window.$ ) {
+                                                               // Pass jQuery twice so that the signature of the closure which wraps
+                                                               // the script can bind both '$' and 'jQuery'.
+                                                               script( window.$, window.$, mw.loader.require, registry[ module ].module );
+                                                       } else {
+                                                               // This is a special case for when 'jquery' itself is being loaded.
+                                                               // - The standard jquery.js distribution does not set `window.jQuery`
+                                                               //   in CommonJS-compatible environments (Node.js, AMD, RequireJS, etc.).
+                                                               // - MediaWiki's 'jquery' module also bundles jquery.migrate.js, which
+                                                               //   in a CommonJS-compatible environment, will use require('jquery'),
+                                                               //   but that can't work when we're still inside that module.
+                                                               script();
+                                                       }
                                                        markModuleReady();
 
                                                } else if ( typeof script === 'string' ) {
                                        // The module may already have been marked as erroneous
                                        if ( registry[ name ].state !== 'error' && registry[ name ].state !== 'missing' ) {
                                                registry[ name ].state = 'loaded';
-                                               if ( allReady( registry[ name ].dependencies ) ) {
+                                               if ( allWithImplicitReady( name ) ) {
                                                        execute( name );
                                                }
                                        }
index c609852..b2d86e2 100644 (file)
@@ -80,7 +80,7 @@ window.isCompatible = function ( str ) {
 };
 
 ( function () {
-       var NORLQ, script;
+       var NORLQ;
        // Handle Grade C
        if ( !isCompatible() ) {
                // Undo speculative Grade A <html> class. See ResourceLoaderClientHtml::getDocumentAttributes().
@@ -119,19 +119,20 @@ window.isCompatible = function ( str ) {
                // Process callbacks for Grade A
                // Must be after registrations and mw.config.set, which mw.loader depends on.
                var queue = window.RLQ;
-               window.RLQ = {
-                       push: function ( fn ) {
-                               if ( typeof fn === 'function' ) {
-                                       fn();
-                               } else {
-                                       // This callback has a requirement.
-                                       mw.loader.using( fn[ 0 ], fn[ 1 ] );
-                               }
+               // Redefine push(), but keep type as array for storing callbacks that require modules.
+               window.RLQ = [];
+               /* global RLQ */
+               RLQ.push = function ( fn ) {
+                       if ( typeof fn === 'function' ) {
+                               fn();
+                       } else {
+                               // This callback requires a module, handled in mediawiki.base.
+                               RLQ[ RLQ.length ] = fn;
                        }
                };
                while ( queue && queue[ 0 ] ) {
                        // Re-use our push()
-                       window.RLQ.push( queue.shift() );
+                       RLQ.push( queue.shift() );
                }
 
                // Clear and disable the Grade C queue
@@ -147,14 +148,5 @@ window.isCompatible = function ( str ) {
        // This embeds mediawiki.js, which defines 'mw' and 'mw.loader'.
        $CODE.defineLoader();
 
-       script = document.createElement( 'script' );
-       script.src = $VARS.baseModulesUri;
-       script.onload = function () {
-               // Clean up
-               script.onload = null;
-               script = null;
-               // Callback
-               startUp();
-       };
-       document.head.appendChild( script );
+       mw.requestIdleCallback( startUp );
 }() );
index de593d5..6b2738b 100644 (file)
                        },
                        function ( e, dependencies ) {
                                assert.strictEqual( Array.isArray( dependencies ), true, 'Expected array of dependencies' );
-                               assert.deepEqual( dependencies, [ 'test.module7' ], 'Error callback called with module test.module7' );
+                               assert.deepEqual(
+                                       dependencies,
+                                       [ 'jquery', 'mediawiki.base', 'test.module7' ],
+                                       'Error callback called with module test.module7'
+                               );
                        }
                );
                mw.loader.using(
                                dependencies.sort();
                                assert.deepEqual(
                                        dependencies,
-                                       [ 'test.module7', 'test.module8', 'test.module9' ],
+                                       [ 'jquery', 'mediawiki.base', 'test.module7', 'test.module8', 'test.module9' ],
                                        'Error callback called with all three modules as dependencies'
                                );
                        }