From: Timo Tijhof Date: Thu, 1 Mar 2018 00:23:02 +0000 (-0800) Subject: resourceloader: Clean up and better document module list (un)packing X-Git-Tag: 1.31.0-rc.0~375^2~5 X-Git-Url: https://git.cyclocoop.org/%28%28?a=commitdiff_plain;h=dc8550ea24cd80eb2cca16a8de51ab1be5ca0e31;p=lhc%2Fweb%2Fwiklou.git resourceloader: Clean up and better document module list (un)packing * Move buildModulesString() call from doRequest() to batchRequest() This keeps all module string "packing" logic located to the same function, which is batchRequest(). It also means that the moduleMap object will not leave the function, which helps in maintenance given it's very internal. * Add comments to all the methods referring to each other. * Explain why buildModulesString() is only a partial port, and the rest is inlined in batchRequest(). * Minor changes to the JS and PHP implementation to better match each other. - '$groups' -> '$moduleMap'. - Remove redundant '$str'. Bug: T188076 Change-Id: I7b0790606c456e492ab682faeb80f7e7fce7d9f8 --- diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index f9b03c71ed..5ddb99bdc1 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -1532,27 +1532,31 @@ MESSAGE; /** * Convert an array of module names to a packed query string. * - * For example, [ 'foo.bar', 'foo.baz', 'bar.baz', 'bar.quux' ] - * becomes 'foo.bar,baz|bar.baz,quux' + * For example, `[ 'foo.bar', 'foo.baz', 'bar.baz', 'bar.quux' ]` + * becomes `'foo.bar,baz|bar.baz,quux'`. + * + * This process is reversed by ResourceLoaderContext::expandModuleNames(). + * See also mw.loader#buildModulesString() which is a port of this, used + * on the client-side. + * * @param array $modules List of module names (strings) * @return string Packed query string */ public static function makePackedModulesString( $modules ) { - $groups = []; // [ prefix => [ suffixes ] ] + $moduleMap = []; // [ prefix => [ suffixes ] ] foreach ( $modules as $module ) { $pos = strrpos( $module, '.' ); $prefix = $pos === false ? '' : substr( $module, 0, $pos ); $suffix = $pos === false ? $module : substr( $module, $pos + 1 ); - $groups[$prefix][] = $suffix; + $moduleMap[$prefix][] = $suffix; } $arr = []; - foreach ( $groups as $prefix => $suffixes ) { + foreach ( $moduleMap as $prefix => $suffixes ) { $p = $prefix === '' ? '' : $prefix . '.'; $arr[] = $p . implode( ',', $suffixes ); } - $str = implode( '|', $arr ); - return $str; + return implode( '|', $arr ); } /** diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php index 7478266e0c..370046aab4 100644 --- a/includes/resourceloader/ResourceLoaderContext.php +++ b/includes/resourceloader/ResourceLoaderContext.php @@ -98,9 +98,12 @@ class ResourceLoaderContext implements MessageLocalizer { } /** - * Expand a string of the form jquery.foo,bar|jquery.ui.baz,quux to - * an array of module names like [ 'jquery.foo', 'jquery.bar', - * 'jquery.ui.baz', 'jquery.ui.quux' ] + * Expand a string of the form `jquery.foo,bar|jquery.ui.baz,quux` to + * an array of module names like `[ 'jquery.foo', 'jquery.bar', + * 'jquery.ui.baz', 'jquery.ui.quux' ]`. + * + * This process is reversed by ResourceLoader::makePackedModulesString(). + * * @param string $modules Packed module name list * @return array Array of module names */ diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index a2e071e11a..aa93ca2b75 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -1511,8 +1511,15 @@ } /** - * Converts a module map of the form { foo: [ 'bar', 'baz' ], bar: [ 'baz, 'quux' ] } - * to a query string of the form foo.bar,baz|bar.baz,quux + * Converts a module map of the form `{ foo: [ 'bar', 'baz' ], bar: [ 'baz, 'quux' ] }` + * to a query string of the form `foo.bar,baz|bar.baz,quux`. + * + * See `ResourceLoader::makePackedModulesString()` in PHP, of which this is a port. + * On the server, unpacking is done by `ResourceLoaderContext::expandModuleNames()`. + * + * Note: This is only half of the logic, the other half has to be in #batchRequest(), + * because its implementation needs to keep track of potential string size in order + * to decide when to split the requests due to url size. * * @private * @param {Object} moduleMap Module map @@ -1533,14 +1540,14 @@ * Make a network request to load modules from the server. * * @private - * @param {Object} moduleMap Module map, see #buildModulesString + * @param {string} moduleStr Module list for load.php `module` query parameter * @param {Object} currReqBase Object with other parameters (other than 'modules') to use in the request * @param {string} sourceLoadScript URL of load.php */ - function doRequest( moduleMap, currReqBase, sourceLoadScript ) { + function doRequest( moduleStr, currReqBase, sourceLoadScript ) { // Optimisation: Inherit (Object.create), not copy ($.extend) var query = Object.create( currReqBase ); - query.modules = buildModulesString( moduleMap ); + query.modules = moduleStr; query = sortQuery( query ); addScript( sourceLoadScript + '?' + $.param( query ) ); } @@ -1660,7 +1667,7 @@ // but don't create empty requests if ( maxQueryLength > 0 && !$.isEmptyObject( moduleMap ) && l + bytesAdded > maxQueryLength ) { // This url would become too long, create a new one, and start the old one - doRequest( moduleMap, currReqBase, sourceLoadScript ); + doRequest( buildModulesString( moduleMap ), currReqBase, sourceLoadScript ); moduleMap = {}; l = currReqBaseLength + 9; mw.track( 'resourceloader.splitRequest', { maxQueryLength: maxQueryLength } ); @@ -1673,7 +1680,7 @@ } // If there's anything left in moduleMap, request that too if ( !$.isEmptyObject( moduleMap ) ) { - doRequest( moduleMap, currReqBase, sourceLoadScript ); + doRequest( buildModulesString( moduleMap ), currReqBase, sourceLoadScript ); } } }