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

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index 0dca8f1..7fe91ce 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.