From c0c221bfe449bacc91d186afb1d910353c7b675a Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 9 Dec 2014 00:29:19 +0000 Subject: [PATCH] resourceloader: Refactor empty value trimming for mw.loader.register We already did this, but it was rather convoluted with lots of if/elseif sequences checking all the possible values. Remove this logic from ResourceLoaderStartUpModule. Simplying it simply create the array and pass it to ResourceLoader::makeLoaderRegisterScript. In makeLoaderRegisterScript, we apply a filter to the array(s) that trim empty values. While at it: * As with other registration properties' default values (like for dependencies, group, and skip) also use 'null' for the default value of 'source'. The mediawiki.js client was already compatible with this, and the server omitted it if it was the last value in the list. But in all other cases it explicitly outputs "local". Use null instead of simplicity sake. This also gains us a few characters in the output, and a relatively larger win after gzip since there's lots more re-using of "null". * Remove stray casting of $version to int. This only happened in case of registering a single module (which don't do anywhere), and is redundant. Change-Id: I1f321e7b8bd3b5cffc550b51169957a3da9b971d --- includes/resourceloader/ResourceLoader.php | 30 ++++++++- .../ResourceLoaderStartUpModule.php | 65 +++---------------- .../ResourceLoaderStartupModuleTest.php | 8 +-- 3 files changed, 41 insertions(+), 62 deletions(-) diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 8602fb02ed..3f4e1723b3 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -1188,6 +1188,27 @@ class ResourceLoader { ); } + /** + * Remove empty values from the end of an array. + * + * Values considered empty: + * + * - null + * - empty array + * + * @param Array $array + */ + private static function trimArray( Array &$array ) { + $i = count( $array ); + while ( $i-- ) { + if ( $array[$i] === null || $array[$i] === array() ) { + unset( $array[$i] ); + } else { + break; + } + } + } + /** * Returns JS code which calls mw.loader.register with the given * parameters. Has three calling conventions: @@ -1221,7 +1242,7 @@ class ResourceLoader { if ( is_array( $name ) ) { // Build module name index $index = array(); - foreach ( $name as $i => $module ) { + foreach ( $name as $i => &$module ) { $index[$module[0]] = $i; } @@ -1237,16 +1258,19 @@ class ResourceLoader { } } + array_walk( $name, array( 'self', 'trimArray' ) ); + return Xml::encodeJsCall( 'mw.loader.register', array( $name ), ResourceLoader::inDebugMode() ); } else { - $version = (int) $version; + $registration = array( $name, $version, $dependencies, $group, $source, $skip ); + self::trimArray( $registration ); return Xml::encodeJsCall( 'mw.loader.register', - array( $name, $version, $dependencies, $group, $source, $skip ), + $registration, ResourceLoader::inDebugMode() ); } diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php index 3b09059243..2eccf81228 100644 --- a/includes/resourceloader/ResourceLoaderStartUpModule.php +++ b/includes/resourceloader/ResourceLoaderStartUpModule.php @@ -264,61 +264,16 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { continue; } - if ( - !count( $data['dependencies'] ) && - $data['group'] === null && - $data['source'] === 'local' && - $data['skip'] === null - ) { - // Modules with no dependencies, group, foreign source or skip function; - // call mw.loader.register(name, timestamp) - $registrations[] = array( $name, $data['version'] ); - } elseif ( - $data['group'] === null && - $data['source'] === 'local' && - $data['skip'] === null - ) { - // Modules with dependencies but no group, foreign source or skip function; - // call mw.loader.register(name, timestamp, dependencies) - $registrations[] = array( - $name, - $data['version'], - $data['dependencies'] - ); - } elseif ( - $data['source'] === 'local' && - $data['skip'] === null - ) { - // Modules with a group but no foreign source or skip function; - // call mw.loader.register(name, timestamp, dependencies, group) - $registrations[] = array( - $name, - $data['version'], - $data['dependencies'], - $data['group'] - ); - } elseif ( $data['skip'] === null ) { - // Modules with a foreign source but no skip function; - // call mw.loader.register(name, timestamp, dependencies, group, source) - $registrations[] = array( - $name, - $data['version'], - $data['dependencies'], - $data['group'], - $data['source'] - ); - } else { - // Modules with a skip function; - // call mw.loader.register(name, timestamp, dependencies, group, source, skip) - $registrations[] = array( - $name, - $data['version'], - $data['dependencies'], - $data['group'], - $data['source'], - $data['skip'] - ); - } + // Call mw.loader.register(name, timestamp, dependencies, group, source, skip) + $registrations[] = array( + $name, + $data['version'], + $data['dependencies'], + $data['group'], + // Swap default (local) for null + $data['source'] === 'local' ? null : $data['source'], + $data['skip'] + ); } // Register modules diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php index 6bfa40fdb3..3fddc1edc1 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php @@ -133,7 +133,7 @@ mw.loader.addSource( { 1388534400, [], null, - "local", + null, "return true;" ], [ @@ -141,7 +141,7 @@ mw.loader.addSource( { 1388534400, [], null, - "local", + null, "return !!( window.JSON \u0026\u0026 JSON.parse \u0026\u0026 JSON.stringify);" ], [ @@ -345,7 +345,7 @@ mw.loader.addSource( { 'mw.loader.addSource({"local":"/w/load.php"});' . 'mw.loader.register([' . '["test.blank",1388534400],' -. '["test.min",1388534400,[0],null,"local",' +. '["test.min",1388534400,[0],null,null,' . '"return!!(window.JSON\u0026\u0026JSON.parse\u0026\u0026JSON.stringify);"' . ']]);', $module->getModuleRegistrations( $context ), @@ -376,7 +376,7 @@ mw.loader.addSource( { 0 ], null, - "local", + null, "return !!( window.JSON \u0026\u0026 JSON.parse \u0026\u0026 JSON.stringify);" ] ] );', -- 2.20.1