Krinkle has uploaded a new change for review.
https://gerrit.wikimedia.org/r/313636
Change subject: mw.loader: Don't unset 'require' after execution in debug mode
......................................................................
mw.loader: Don't unset 'require' after execution in debug mode
Follows-up bc4e07b6f6.
In production mode, 'module' and 'require' are lexically provided
through closure to the executing script and will continue to be
accessible through the script's life-time.
In debug mode, the script executes without wrapper directly from
disk (and as such, in the global scope) and variables are provided
as global variables. Since a variable can only be set to one value,
we swap the object that 'module' points to after each file. And
to avoid leakage, it is removed in-between files and after the
last one. Aside from it being technically infeasible right now,
async access to 'module' is discouraged regardless.
Late access to 'require', however, is not uncommon and should
work as expected.
Bug: T144879
Change-Id: I6ede10fd42676bb035ea26c693c78bcdc1438a7d
---
M resources/src/mediawiki/mediawiki.js
M tests/qunit/data/defineCallMwLoaderTestCallback.js
M tests/qunit/data/requireCallMwLoaderTestCallback.js
M tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
4 files changed, 21 insertions(+), 9 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/36/313636/1
diff --git a/resources/src/mediawiki/mediawiki.js
b/resources/src/mediawiki/mediawiki.js
index 3122d42..c7715e5 100644
--- 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
--- 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
--- 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
--- 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' );
} );
--
To view, visit https://gerrit.wikimedia.org/r/313636
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ede10fd42676bb035ea26c693c78bcdc1438a7d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
