Merge "resourceloader: Remove support for `addSource(id, url)`"

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index be91861..ff59d43 100644 (file)
--- 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 8f5d083..9764549 100644 (file)
--- 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 ee3a656..afa6f03 100644 (file)
--- 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 d9ad711..d791b7c 100644 (file)
--- 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 ae5ddb5..677905f 100644 (file)
--- 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.