From 2dc03ee268dc6777e475b4319254c81a1bbb1ed4 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 4 Aug 2016 16:04:37 -0700 Subject: [PATCH] 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 --- resources/src/mediawiki/mediawiki.js | 15 ++-- .../resources/mediawiki/mediawiki.test.js | 69 ++++++++++--------- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index ef4f6df12e..11784fb721 100644 --- 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 dd43c553bc..3f426e9cad 100644 --- 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' ); } ); } ); -- 2.20.1