From bc4e07b6f63b0865a14aef981366a79d10329c87 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Fri, 22 Jan 2016 11:29:28 -0800 Subject: [PATCH] resourceloader: Implement modern module loading (2/2) * Send 'module' and 'require' parameters to module closures. This depends on Ia925844cc22f143 being deployed one cycle earlier. * Patch Moment and OOjs to ensure these libraries continue to expose their module as globals as well. AMD/UMD-compatible libraries only expose a global *OR* an export, not both. We need both for back-compat. * Update pluralRuleParser to make use of module export to allow usage via require(). To test, check out the patch and run: > mw.loader.load('moment'); > mw.loader.require('moment')() > mw.loader.require('moment')('2011-04-01').fromNow() Bug: T108655 Change-Id: Idbd054880ee70d659ec760aef8fcb38d0704a394 --- includes/resourceloader/ResourceLoader.php | 2 +- resources/Resources.php | 2 ++ .../mediawiki.libs/CLDRPluralRuleParser.js | 1 + resources/src/mediawiki/mediawiki.js | 9 ++++++++- resources/src/moment-global.js | 2 ++ resources/src/oojs-global.js | 2 ++ tests/phpunit/includes/OutputPageTest.php | 2 +- .../resourceloader/ResourceLoaderTest.php | 8 ++++---- tests/qunit/QUnitTestResources.php | 8 ++++++++ .../data/defineCallMwLoaderTestCallback.js | 1 + .../data/requireCallMwLoaderTestCallback.js | 2 ++ .../resources/mediawiki/mediawiki.test.js | 20 +++++++++++++++++++ .../suites/resources/test.sinonjs/index.js | 3 +++ 13 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 resources/src/moment-global.js create mode 100644 resources/src/oojs-global.js create mode 100644 tests/qunit/data/defineCallMwLoaderTestCallback.js create mode 100644 tests/qunit/data/requireCallMwLoaderTestCallback.js create mode 100644 tests/qunit/suites/resources/test.sinonjs/index.js diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 0aa08bece7..086ab17cdb 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -1098,7 +1098,7 @@ MESSAGE; $scripts = self::filter( 'minify-js', $scripts ); } } else { - $scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" ); + $scripts = new XmlJsCode( "function ( $, jQuery, require, module ) {\n{$scripts}\n}" ); } } elseif ( !is_array( $scripts ) ) { throw new MWException( 'Invalid scripts error. Array of URLs or string of code expected.' ); diff --git a/resources/Resources.php b/resources/Resources.php index 42a074666e..cb7adbe4d9 100644 --- a/resources/Resources.php +++ b/resources/Resources.php @@ -734,6 +734,7 @@ return [ 'moment' => [ 'scripts' => [ 'resources/lib/moment/moment.js', + 'resources/src/moment-global.js', 'resources/src/moment-local-dmy.js', ], 'languageScripts' => [ @@ -2278,6 +2279,7 @@ return [ 'oojs' => [ 'scripts' => [ 'resources/lib/oojs/oojs.jquery.js', + 'resources/src/oojs-global.js', ], 'targets' => [ 'desktop', 'mobile' ], 'dependencies' => [ diff --git a/resources/src/mediawiki.libs/CLDRPluralRuleParser.js b/resources/src/mediawiki.libs/CLDRPluralRuleParser.js index 31c8fef92a..549a9ab3da 100644 --- a/resources/src/mediawiki.libs/CLDRPluralRuleParser.js +++ b/resources/src/mediawiki.libs/CLDRPluralRuleParser.js @@ -591,5 +591,6 @@ function pluralRuleParser(rule, number) { /* pluralRuleParser ends here */ mw.libs.pluralRuleParser = pluralRuleParser; +module.exports = pluralRuleParser; } )( mediaWiki ); diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index 9d799db154..4aad2bac90 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -1189,11 +1189,18 @@ * @param {string} [moduleName] Name of currently executing module * @return {jQuery.Promise} */ - function queueModuleScript( src ) { + function queueModuleScript( src, moduleName ) { var r = $.Deferred(); pendingRequests.push( function () { + if ( moduleName && hasOwn.call( registry, moduleName ) ) { + window.require = mw.loader.require; + window.module = registry[ moduleName ].module; + } addScript( src ).always( function () { + // Clear environment + delete window.require; + delete window.module; r.resolve(); // Start the next one (if any) diff --git a/resources/src/moment-global.js b/resources/src/moment-global.js new file mode 100644 index 0000000000..ba01a240f6 --- /dev/null +++ b/resources/src/moment-global.js @@ -0,0 +1,2 @@ +// Back-compat: Export module as global +window.moment = module.exports; diff --git a/resources/src/oojs-global.js b/resources/src/oojs-global.js new file mode 100644 index 0000000000..de156f0bb1 --- /dev/null +++ b/resources/src/oojs-global.js @@ -0,0 +1,2 @@ +// Back-compat: Export module as global +window.OO = module.exports; diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php index 2f63ca83b7..8d4a34775c 100644 --- a/tests/phpunit/includes/OutputPageTest.php +++ b/tests/phpunit/includes/OutputPageTest.php @@ -169,7 +169,7 @@ class OutputPageTest extends MediaWikiTestCase { [ [ 'test.quux', ResourceLoaderModule::TYPE_COMBINED ], "" ], diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php index 2dfed62c18..65cd6edaf9 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php @@ -188,7 +188,7 @@ class ResourceLoaderTest extends ResourceLoaderTestCase { 'messages' => [ 'example' => '' ], 'templates' => [], - 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) { + 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery, require, module ) { mw.example(); }, { "css": [ @@ -207,7 +207,7 @@ mw.example(); 'messages' => new XmlJsCode( '{}' ), 'templates' => [], - 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) { + 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery, require, module ) { mw.example(); } );', ] ], @@ -235,7 +235,7 @@ mw.example(); 'messages' => [ 'example' => '' ], 'templates' => [], - 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) { + 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery, require, module ) { mw.example(); }, {}, { "example": "" @@ -250,7 +250,7 @@ mw.example(); 'messages' => new XmlJsCode( '{}' ), 'templates' => [ 'example.html' => '' ], - 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) { + 'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery, require, module ) { mw.example(); }, {}, {}, { "example.html": "" diff --git a/tests/qunit/QUnitTestResources.php b/tests/qunit/QUnitTestResources.php index b7161b1cfa..a2d76e01b0 100644 --- a/tests/qunit/QUnitTestResources.php +++ b/tests/qunit/QUnitTestResources.php @@ -8,7 +8,15 @@ return [ 'test.sinonjs' => [ 'scripts' => [ + 'tests/qunit/suites/resources/test.sinonjs/index.js', 'resources/lib/sinonjs/sinon-1.17.3.js', + // We want tests to work in IE, but can't include this as it + // will break the placeholders in Sinon because the hack it uses + // to hijack IE globals relies on running in the global scope + // and in ResourceLoader this won't be running in the global scope. + // Including it results (among other things) in sandboxed timers + // being broken due to Date inheritance being undefined. + // 'resources/lib/sinonjs/sinon-ie-1.15.4.js', ], 'targets' => [ 'desktop', 'mobile' ], ], diff --git a/tests/qunit/data/defineCallMwLoaderTestCallback.js b/tests/qunit/data/defineCallMwLoaderTestCallback.js new file mode 100644 index 0000000000..afd886c334 --- /dev/null +++ b/tests/qunit/data/defineCallMwLoaderTestCallback.js @@ -0,0 +1 @@ +module.exports = 'Define worked.'; diff --git a/tests/qunit/data/requireCallMwLoaderTestCallback.js b/tests/qunit/data/requireCallMwLoaderTestCallback.js new file mode 100644 index 0000000000..8bc087b007 --- /dev/null +++ b/tests/qunit/data/requireCallMwLoaderTestCallback.js @@ -0,0 +1,2 @@ +var x = require( 'test.require.define' ); +module.exports = 'Require worked.' + x; diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js index ce4ea8b147..dd43c553bc 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js @@ -1127,4 +1127,24 @@ }, /is not loaded/, 'Requesting non-existent modules throws error.' ); } ); + QUnit.asyncTest( 'mw.loader require in debug mode', 1, function ( assert ) { + var path = mw.config.get( 'wgScriptPath' ); + mw.loader.register( [ + [ 'test.require.define', '0' ], + [ 'test.require.callback', '0', [ 'test.require.define' ] ] + ] ); + 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' ); + 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' ); + } ); + } ); + }( mediaWiki, jQuery ) ); diff --git a/tests/qunit/suites/resources/test.sinonjs/index.js b/tests/qunit/suites/resources/test.sinonjs/index.js new file mode 100644 index 0000000000..b1be9d1890 --- /dev/null +++ b/tests/qunit/suites/resources/test.sinonjs/index.js @@ -0,0 +1,3 @@ +// Hack: Disable 'module.exports' from ResourceLoader +// (Otherwise Sinon assumes context as Node.js instead of a browser) +module.exports = null; -- 2.20.1