Merge "resourceloader: Clean up and better document module list (un)packing"

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index f9b03c7..5ddb99b 100644 (file)
--- 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 7478266..370046a 100644 (file)
--- 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 a2e071e..aa93ca2 100644 (file)
--- 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 );
                                                }
                                        }
                                }