mw.loader: Add 'require' as Promise value for using()

Discourage use of the global mw.loader.require method since it's contextless,
and doesn't allow for future expansion where we might detect missing declared
dependencies or could track stacking context.

In regular execution context, there is already a local require() method.

For ad-hoc loading through mw.loader.using() there is now a reference to a
require function.

This also discourages code from assuming that the internal implement() method
is synchronous - which may change after T142129.

Change-Id: Ia040729901b1e77da8d3bf4830bb076f8fa8c6e9

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index ef4f6df..11784fb 100644 (file)
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1852,6 +1852,7 @@
                                 * The reason css strings are not concatenated anymore is bug 31676. We now check
                                 * whether it's safe to extend the stylesheet.
                                 *
+                                * @protected
                                 * @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.
                                 */
@@ -1887,12 +1888,15 @@
                                 *         OO.compare( [ 1 ], [ 1 ] );
                                 *     } );
                                 *
+                                * Since MediaWiki 1.23 this also returns a promise.
+                                *
+                                * Since MediaWiki 1.28 the promise is resolved with a `require` function.
+                                *
                                 * @param {string|Array} dependencies Module name or array of modules names the
                                 *  callback depends on to be ready before executing
                                 * @param {Function} [ready] Callback to execute when all dependencies are ready
                                 * @param {Function} [error] Callback to execute if one or more dependencies failed
-                                * @return {jQuery.Promise}
-                                * @since 1.23 this returns a promise
+                                * @return {jQuery.Promise} With a `require` function
                                 */
                                using: function ( dependencies, ready, error ) {
                                        var deferred = $.Deferred();
@@ -1913,7 +1917,7 @@
                                        dependencies = resolve( dependencies );
                                        if ( allReady( dependencies ) ) {
                                                // Run ready immediately
-                                               deferred.resolve();
+                                               deferred.resolve( mw.loader.require );
                                        } else if ( anyFailed( dependencies ) ) {
                                                // Execute error immediately if any dependencies have errors
                                                deferred.reject(
@@ -1922,7 +1926,9 @@
                                                );
                                        } else {
                                                // Not all dependencies are ready: queue up a request
-                                               request( dependencies, deferred.resolve, deferred.reject );
+                                               request( dependencies, function () {
+                                                       deferred.resolve( mw.loader.require );
+                                               }, deferred.reject );
                                        }
 
                                        return deferred.promise();
@@ -2064,7 +2070,6 @@
                                 *
                                 * @protected
                                 * @since 1.27
-                                * @return {Array}
                                 */
                                require: function ( moduleName ) {
                                        var state = mw.loader.getState( moduleName );

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index dd43c55..3f426e9 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -968,7 +968,7 @@
                mw.loader.load( target );
        } );
 
-       QUnit.asyncTest( 'mw.loader() executing race (T112232)', 2, function ( assert ) {
+       QUnit.test( 'mw.loader() executing race (T112232)', 2, function ( assert ) {
                var done = false;
 
                // The red herring schedules its CSS buffer first. In T112232, a bug in the
@@ -987,10 +987,10 @@
                );
 
                mw.loader.load( [ 'testRaceRedHerring', 'testRaceLoadMe' ] );
-               mw.loader.using( 'testRaceLoadMe', function () {
+               return mw.loader.using( 'testRaceLoadMe', function () {
                        assert.strictEqual( done, true, 'script ran' );
                        assert.strictEqual( mw.loader.getState( 'testRaceLoadMe' ), 'ready', 'state' );
-               } ).always( QUnit.start );
+               } );
        } );
 
        QUnit.test( 'mw.hook', 13, function ( assert ) {
@@ -1085,49 +1085,52 @@
                );
        } );
 
-       QUnit.test( 'mw.loader.require', 6, function ( assert ) {
-               var module1, module2, module3, module4;
-
+       QUnit.test( 'mw.loader require()', 6, function ( assert ) {
                mw.loader.register( [
-                       [ 'test.module.require1', '0' ],
-                       [ 'test.module.require2', '0' ],
-                       [ 'test.module.require3', '0' ],
-                       [ 'test.module.require4', '0', [ 'test.module.require3' ] ]
+                       [ 'test.require1', '0' ],
+                       [ 'test.require2', '0' ],
+                       [ 'test.require3', '0' ],
+                       [ 'test.require4', '0', [ 'test.require3' ] ]
                ] );
-               mw.loader.implement( 'test.module.require1', function () {} );
-               mw.loader.implement( 'test.module.require2', function ( $, jQuery, require, module ) {
+               mw.loader.implement( 'test.require1', function () {} );
+               mw.loader.implement( 'test.require2', function ( $, jQuery, require, module ) {
                        module.exports = 1;
                } );
-               mw.loader.implement( 'test.module.require3', function ( $, jQuery, require, module ) {
+               mw.loader.implement( 'test.require3', function ( $, jQuery, require, module ) {
                        module.exports = function () {
                                return 'hello world';
                        };
                } );
-               mw.loader.implement( 'test.module.require4', function ( $, jQuery, require, module ) {
-                       var other = require( 'test.module.require3' );
+               mw.loader.implement( 'test.require4', function ( $, jQuery, require, module ) {
+                       var other = require( 'test.require3' );
                        module.exports = {
                                pizza: function () {
                                        return other();
                                }
                        };
                } );
-               module1 = mw.loader.require( 'test.module.require1' );
-               module2 = mw.loader.require( 'test.module.require2' );
-               module3 = mw.loader.require( 'test.module.require3' );
-               module4 = mw.loader.require( 'test.module.require4' );
-
-               assert.strictEqual( typeof module1, 'object', 'export of module with no export' );
-               assert.strictEqual( module2, 1, 'export a number' );
-               assert.strictEqual( module3(), 'hello world', 'export a function' );
-               assert.strictEqual( typeof module4.pizza, 'function', 'export an object' );
-               assert.strictEqual( module4.pizza(), 'hello world', 'module can require other modules' );
-
-               assert.throws( function () {
-                       mw.loader.require( '_badmodule' );
-               }, /is not loaded/, 'Requesting non-existent modules throws error.' );
+               return mw.loader.using( [ 'test.require1', 'test.require2', 'test.require3', 'test.require4' ] )
+               .then( function ( require ) {
+                       var module1, module2, module3, module4;
+
+                       module1 = require( 'test.require1' );
+                       module2 = require( 'test.require2' );
+                       module3 = require( 'test.require3' );
+                       module4 = require( 'test.require4' );
+
+                       assert.strictEqual( typeof module1, 'object', 'export of module with no export' );
+                       assert.strictEqual( module2, 1, 'export a number' );
+                       assert.strictEqual( module3(), 'hello world', 'export a function' );
+                       assert.strictEqual( typeof module4.pizza, 'function', 'export an object' );
+                       assert.strictEqual( module4.pizza(), 'hello world', 'module can require other modules' );
+
+                       assert.throws( function () {
+                               require( '_badmodule' );
+                       }, /is not loaded/, 'Requesting non-existent modules throws error.' );
+               } );
        } );
 
-       QUnit.asyncTest( 'mw.loader require in debug mode', 1, function ( assert ) {
+       QUnit.test( 'mw.loader require() in debug mode', 1, function ( assert ) {
                var path = mw.config.get( 'wgScriptPath' );
                mw.loader.register( [
                        [ 'test.require.define', '0' ],
@@ -1136,13 +1139,11 @@
                mw.loader.implement( 'test.require.callback', [ QUnit.fixurl( path + '/tests/qunit/data/requireCallMwLoaderTestCallback.js' ) ] );
                mw.loader.implement( 'test.require.define', [ QUnit.fixurl( path + '/tests/qunit/data/defineCallMwLoaderTestCallback.js' ) ] );
 
-               mw.loader.using( 'test.require.callback', function () {
-                       QUnit.start();
-                       var exported = mw.loader.require( 'test.require.callback' );
+               return mw.loader.using( 'test.require.callback' ).then( function ( require ) {
+                       var exported = require( 'test.require.callback' );
                        assert.strictEqual( exported, 'Require worked.Define worked.',
                                'module.exports worked in debug mode' );
                }, function () {
-                       QUnit.start();
                        assert.ok( false, 'Error callback fired while loader.using "test.require.callback" module' );
                } );
        } );