From f3641c0a9cf9dbf21e791e391a14a3063d95cff1 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 4 Sep 2018 00:17:11 +0100 Subject: [PATCH] resourceloader: Remove support for `addSource(id, url)` The PHP interface still supports it without deprecation, but there is no need for the client to support it given it is only ever called from the startup module, which always passes it an object. Follows-up 6815e355753c, which did the same for `state(key, val)`. Also simplify the implementation to avoid extra calls and optimise for the only signature. Mark the method as @private, and change the existing use of @protected to @private (doesn't really make sense in JavaScript). Change-Id: I24786f90bcfb256b2e7c37f7760bba1a5e399443 --- RELEASE-NOTES-1.32 | 2 ++ includes/resourceloader/ResourceLoader.php | 23 ++++++--------- resources/src/startup/mediawiki.js | 29 +++++++------------ .../resourceloader/ResourceLoaderTest.php | 4 ++- .../mediawiki/mediawiki.loader.test.js | 20 ++++++++++--- 5 files changed, 41 insertions(+), 37 deletions(-) diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index 0dca8f13d5..7fe91ce2a0 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -287,6 +287,8 @@ because of Phabricator reports. instead. * MediaWiki no longer supports a StartProfiler.php file. Define $wgProfiler via LocalSettings.php instead. +* The mw.loader.addSource() is now considered a private method, and no longer + supports the `id, url` signature. Use the `Object` parameter instead. === Deprecations in 1.32 === * HTMLForm::setSubmitProgressive() is deprecated. No need to call it. Submit diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 8f5d083ab5..9764549e84 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -1453,24 +1453,19 @@ MESSAGE; * - ResourceLoader::makeLoaderSourcesScript( [ $id1 => $loadUrl, $id2 => $loadUrl, ... ] ); * Register sources with the given IDs and properties. * - * @param string $id Source ID + * @param string|array $sources Source ID * @param string|null $loadUrl load.php url * @return string JavaScript code */ - public static function makeLoaderSourcesScript( $id, $loadUrl = null ) { - if ( is_array( $id ) ) { - return Xml::encodeJsCall( - 'mw.loader.addSource', - [ $id ], - self::inDebugMode() - ); - } else { - return Xml::encodeJsCall( - 'mw.loader.addSource', - [ $id, $loadUrl ], - self::inDebugMode() - ); + public static function makeLoaderSourcesScript( $sources, $loadUrl = null ) { + if ( !is_array( $sources ) ) { + $sources = [ $sources => $loadUrl ]; } + return Xml::encodeJsCall( + 'mw.loader.addSource', + [ $sources ], + self::inDebugMode() + ); } /** diff --git a/resources/src/startup/mediawiki.js b/resources/src/startup/mediawiki.js index ee3a656809..afa6f035cf 100644 --- a/resources/src/startup/mediawiki.js +++ b/resources/src/startup/mediawiki.js @@ -1635,7 +1635,7 @@ /** * Start loading of all queued module dependencies. * - * @protected + * @private */ work: function () { var q, batch, implementations, sourceModules; @@ -1713,27 +1713,20 @@ * * The #work() method will use this information to split up requests by source. * - * mw.loader.addSource( 'mediawikiwiki', '//www.mediawiki.org/w/load.php' ); + * mw.loader.addSource( { mediawikiwiki: 'https://www.mediawiki.org/w/load.php' } ); * - * @param {string|Object} id Source ID, or object mapping ids to load urls - * @param {string} loadUrl Url to a load.php end point + * @private + * @param {Object} ids An object mapping ids to load.php end point urls * @throws {Error} If source id is already registered */ - addSource: function ( id, loadUrl ) { - var source; - // Allow multiple additions - if ( typeof id === 'object' ) { - for ( source in id ) { - mw.loader.addSource( source, id[ source ] ); + addSource: function ( ids ) { + var id; + for ( id in ids ) { + if ( hasOwn.call( sources, id ) ) { + throw new Error( 'source already registered: ' + id ); } - return; + sources[ id ] = ids[ id ]; } - - if ( hasOwn.call( sources, id ) ) { - throw new Error( 'source already registered: ' + id ); - } - - sources[ id ] = loadUrl; }, /** @@ -1810,7 +1803,7 @@ * The reason css strings are not concatenated anymore is T33676. We now check * whether it's safe to extend the stylesheet. * - * @protected + * @private * @param {Object} [messages] List of key/value pairs to be added to mw#messages. * @param {Object} [templates] List of key/value pairs to be added to mw#templates. */ diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php index d9ad711190..d791b7c259 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php @@ -513,7 +513,9 @@ mw.example(); */ public function testMakeLoaderSourcesScript() { $this->assertEquals( - 'mw.loader.addSource( "local", "/w/load.php" );', + 'mw.loader.addSource( { + "local": "/w/load.php" +} );', ResourceLoader::makeLoaderSourcesScript( 'local', '/w/load.php' ) ); $this->assertEquals( diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js index ae5ddb587d..677905f81b 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js @@ -19,10 +19,10 @@ } } ) ); - mw.loader.addSource( - 'testloader', - QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/load.mock.php' ) - ); + mw.loader.addSource( { + testloader: + QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/load.mock.php' ) + } ); /** * The sync style load test, for @import. This is, in a way, also an open bug for @@ -546,6 +546,18 @@ assert.strictEqual( mw.loader.getState( 'test.empty' ), 'ready' ); } ); + QUnit.test( '.addSource()', function ( assert ) { + mw.loader.addSource( { testsource1: 'https://1.test/src' } ); + + assert.throws( function () { + mw.loader.addSource( { testsource1: 'https://1.test/src' } ); + }, /already registered/, 'duplicate pair from addSource(Object)' ); + + assert.throws( function () { + mw.loader.addSource( { testsource1: 'https://1.test/src-diff' } ); + }, /already registered/, 'duplicate ID from addSource(Object)' ); + } ); + // @covers mw.loader#batchRequest // This is a regression test because in the past we called getCombinedVersion() // for all requested modules, before url splitting took place. -- 2.20.1