Steps to replicate the issue (include links if applicable):
mw.loader.using(['mediawiki.util', 'jquery.ui', 'ext.gadget.morebits']).then(function() { importScript('MediaWiki:Gadget-Twinkle.js'); });
What happens?:
What should have happened instead?:
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| resourceloader: convert mw.loader.using to native Promise | mediawiki/core | master | +16 -20 |
It does return a rejected promise if non-existent modules are requested:
> await mw.loader.using(['mediawiki.util', 'jquery.ui', 'ext.gadget.morebits'])
Uncaught Error: Unknown module: ext.gadget.morebits
at sortDependencies (load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector-2022:6:804)
at Object.resolve (load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector-2022:7:488)
at mw.loader.using
at <anonymous>:1:17However, as jQuery promises don't have a default rejection handler (unlike ES6 promises which do), clients should always use a .catch(), otherwise rejections would go unnoticed.
This was originally left out intentionally. The idea is as follow:
mw.loader.load emits a browser console warning today when passing it an unknown module. We do that because that method is usually called implicilty and declaratively. For example by enabling a gadget, or some other user preference, or by a server-side handler calling $outputPage->addModules(). There is no error handler in that case since the loading process is recoverable and so our only communication is the browser console (more or less).
It feels appropiate to do this because these are modules of interest to the system as a whole, and there is no case in which such mistake is intentional or otherwise "wrong" to warn for.
mw.loader.using() is generally called procedurally, which means there is a natural way to communicate the error, through the error callback. As @SD0001 showed, that's used as mw.loader.using( … ).catch( function ( error ) { … } );.
In this case, the error is more localised, and could in some cases be expected or handled more locally within the program (for example, by showing an error message within the user interface from the Promise-catch handler). In that case, the browser console warning might feel unexpected, especially if there's nothinng anyone can do to "fix" the warning. For example, it is not uncommon to have code that supports multiple methods of working, and it's permitted to "try" to load something that fails. Having said that, we no longer have ES5-ES6 separation in the ResourceLoader registry, and we now consider mw.loader.getState() a public method to check whether a module is defined. That probably removes the need for "working as intended" code where using() would fail.
This idea sounds good to me. We can add a warning similar to the one we emit for load() when an unknown module is asked for, and emit it from using() as well!
The mw.loader.load() method is for queuing modules to the side, without any progammatic handling. Warnings from these are shown in the console by default.
The mw.loader.using() method is for programmatic usage within another script. It is the responsibility of the caller to handle errors, this can include network errors, syntax errors, timeouts, and indeed uncaught errors such as "Error: Unknown module".
Making these go the console was previously considered in T346419, and is intentionally not done as this would cause noise in all the cases where the method is used correctly, where the error is already handled. This is no different from any other try-catch construct, where it would not be expected for the browser to log errors about caught exceptions, only uncaught ones.
It is difficult for the API provider, such as ResourceLoader in this case, to know whether or not an error handler will be attached in the future. The browser, does, however have something like this already.
Consider:
/** @return {Promise} */ function example() { return Promise.reject( new Error( 'Boo' ) ); } example(); // [console] Uncaught (in promise) Error: Boo example().catch( function () {} ); // [console] – (No uncaught error)
This mechanism is not applied by the browser to mw.loader.using() today, because we use jQuery.Deferred. The following script emulates how it would behave it we used Promise instead:
var _using = mw.loader.using; mw.loader.using = function ( dependencies, ready, error ) { var p = new Promise( ( resolve, reject ) => { _using( dependencies ).then( resolve, reject ); } ); if ( ready ) p = p.then( ready ); if ( error ) p = p.catch( error ); return p; }; mw.loader.using( 'ooui7' ); // [console] Uncaught (in promise) Error: Unknown module: ooui7 mw.loader.using( 'ooui7', () => console.log('My yep'), () => console.warn('My nope') ); // [console] My nope // [console] – (No uncaught error) mw.loader.using( 'ooui7' ).then( () => console.log('My yep') ).catch( () => console.warn('My nope') ); // [console] My nope // [console] – (No uncaught error) try { await mw.loader.using( 'ooui7' ); } catch (e) { console.warn('My nope'); } // [console] My nope // [console] – (No uncaught error)
I'm tagging this as a task suitable for interested volunteers there.
The objective is to update mw.loader.using(), which is defined in MediaWiki core's mediawiki.base.js, from jQuery.Deferred to instead use Promise.
This approach solves the reported because the browsers naturally report uncaught promise rejections to the browser console.
This approach avoids the problem described in T346419#9174761, because browsers do not report rejections for promises that have a catch handler.
Any solution that avoids the situation with script does not run without any explanation is fine.
Change #1027253 had a related patch set uploaded (by SD0001; author: SD0001):
[mediawiki/core@master] resourceloader: convert mw.loader.using to native Promise