Merge "mw.loader: Don't unset 'require' after execution in debug mode"

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index 3122d42..c7715e5 100644 (file)
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1217,12 +1217,14 @@
 
                                pendingRequests.push( function () {
                                        if ( moduleName && hasOwn.call( registry, moduleName ) ) {
+                                               // Emulate runScript() part of execute()
                                                window.require = mw.loader.require;
                                                window.module = registry[ moduleName ].module;
                                        }
                                        addScript( src ).always( function () {
-                                               // Clear environment
-                                               delete window.require;
+                                               // 'module.exports' should not persist after the file is executed to
+                                               // avoid leakage to unrelated code. 'require' should be kept, however,
+                                               // as asynchronous access to 'require' is allowed and expected. (T144879)
                                                delete window.module;
                                                r.resolve();
 

diff --git a/tests/qunit/data/defineCallMwLoaderTestCallback.js b/tests/qunit/data/defineCallMwLoaderTestCallback.js
index afd886c..641071a 100644 (file)
--- a/tests/qunit/data/defineCallMwLoaderTestCallback.js
+++ b/tests/qunit/data/defineCallMwLoaderTestCallback.js
@@ -1 +1 @@
-module.exports = 'Define worked.';
+module.exports = 'Defined.';

diff --git a/tests/qunit/data/requireCallMwLoaderTestCallback.js b/tests/qunit/data/requireCallMwLoaderTestCallback.js
index 8bc087b..815a3b4 100644 (file)
--- a/tests/qunit/data/requireCallMwLoaderTestCallback.js
+++ b/tests/qunit/data/requireCallMwLoaderTestCallback.js
@@ -1,2 +1,6 @@
-var x = require( 'test.require.define' );
-module.exports = 'Require worked.' + x;
+module.exports = {
+       immediate: require( 'test.require.define' ),
+       later: function () {
+               return require( 'test.require.define' );
+       }
+};

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
index 41d800a..b69c9e8 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
@@ -664,7 +664,7 @@
                } );
        } );
 
-       QUnit.test( 'require() in debug mode', 1, function ( assert ) {
+       QUnit.test( 'require() in debug mode', function ( assert ) {
                var path = mw.config.get( 'wgScriptPath' );
                mw.loader.register( [
                        [ 'test.require.define', '0' ],
@@ -674,9 +674,15 @@
                mw.loader.implement( 'test.require.define', [ QUnit.fixurl( path + '/tests/qunit/data/defineCallMwLoaderTestCallback.js' ) ] );
 
                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' );
+                       var cb = require( 'test.require.callback' );
+                       assert.strictEqual( cb.immediate, 'Defined.', 'module.exports and require work in debug mode' );
+                       // Must use try-catch because cb.later() will throw if require is undefined,
+                       // which doesn't work well inside Deferred.then() when using jQuery 1.x with QUnit
+                       try {
+                               assert.strictEqual( cb.later(), 'Defined.', 'require works asynchrously in debug mode' );
+                       } catch ( e ) {
+                               assert.equal( null, String( e ), 'require works asynchrously in debug mode' );
+                       }
                }, function () {
                        assert.ok( false, 'Error callback fired while loader.using "test.require.callback" module' );
                } );