Last modified: 2013-03-06 05:48:01 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T45905, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 43905 - Document ResourceLoaderModule 'loaderScripts' property
Document ResourceLoaderModule 'loaderScripts' property
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.19.3
All All
: Low enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: documentation
  Show dependency treegraph
 
Reported: 2013-01-12 18:40 UTC by Mark A. Hershberger
Modified: 2013-03-06 05:48 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Mark A. Hershberger 2013-01-12 18:40:35 UTC
In LocalSettings.php:

 $wgResourceModules['ext.abc'] = array(
    'loaderScripts' => 'extensions/abc-loader.js',
 );

In extensions/abc-loader.js:

 mw.loader.load("//example.com/js/sample.js");

In [[MediaWiki:Common.js]]:

 mw.loader.load( 'ext.abc', function() {
   console.log("works!");
 }

When I load a page, I can see sample.js is loaded, so ext.abc is defined at some time.  However the browser's console shows:

 Error: Unknown dependency: ext.abc
Comment 1 Krinkle 2013-01-12 18:55:33 UTC
(In reply to comment #0)
> In LocalSettings.php:
> 
>  $wgResourceModules['ext.abc'] = array(
>     'loaderScripts' => 'extensions/abc-loader.js',
>  );
> 
> In extensions/abc-loader.js:
> 
>  mw.loader.load("//example.com/js/sample.js");
> 
> In [[MediaWiki:Common.js]]:
> 
>  mw.loader.load( 'ext.abc', function() {
>    console.log("works!");
>  }
> 
> When I load a page, I can see sample.js is loaded, so ext.abc is defined at
> some time.  However the browser's console shows:
> 
>  Error: Unknown dependency: ext.abc

I don't know if you used 'loaderScripts' by accident instead of 'scripts', but you obviously don't know what 'loaderScripts' is.

loaderScripts are scripts to modify or extend the startup module (the module registry manifest). By design they aren't a module you can just load (unless your loader script implements this logic).

In the example you provide, the following is added to the startup module:

( function ( name, version, dependencies, group, source ) {
		mw.loader.load("//example.com/js/sample.js");
	} )("ext.abc", "20130112T184724Z", [], null, "local");

That doesn't make any sense as it ignores all parameters. And this is executed on every page everywhere unconditionally before ResourceLoader is even fully initialised.

In the wgResourceModules array, use 'scripts', not 'loaderScripts'. If you did intentionally use 'loaderScripts' and know what it does, feel free to re-open.
Comment 2 Mark A. Hershberger 2013-01-12 21:44:31 UTC
(In reply to comment #1)
> loaderScripts are scripts to modify or extend the startup module (the module
> registry manifest).

Right, this particular script needed to be loaded before the execution of Common.js -- for the actual example I'm using see: https://www.mediawiki.org/wiki/Thread:Project:Support_desk/How_to_insert_discussion_widget_from_Facebook_ot_VK.com_to_discussion_page?/reply_(12)

and substitute the VK script for the example script.  As I state in that thread, VK is undefined unless the script is loaded before hand.

> By design they aren't a module you can just load (unless
> your loader script implements this logic).

Ok, I can see the sense of that, but then the documentation should make that clearer.
Comment 3 Mark A. Hershberger 2013-01-12 21:53:50 UTC
Even the following doesn't seem to work:

  $wgResourceModules['ext.abc'] = array(
	'loaderScripts' => 'extensions/abc-loader.js',
  );

  $wgResourceModules['ext.abc.use'] = array(
	'dependencies' => 'ext.abc',
  );
Comment 4 Krinkle 2013-01-12 22:10:20 UTC
(In reply to comment #3)
> Even the following doesn't seem to work:
> 
>   $wgResourceModules['ext.abc'] = array(
>     'loaderScripts' => 'extensions/abc-loader.js',
>   );
> 
>   $wgResourceModules['ext.abc.use'] = array(
>     'dependencies' => 'ext.abc',
>   );

That's no different then the previous case. The module wasn't loadable because it isn't registered (this is something your custom startup module needs to do, which it can as it is given the name). So it can't be referred to anywhere unless it is registered by your startup module.

I'm not sure why you are implementing a custom startup module, but if you are you need to do take care of a lot of logic. It is a very low level hook.

Also, note that just because this runs from the startup module, that doesn't mean it will finish before common.js.

So whatever you're loading, it will not be reliably be available for whatever you do in Common.js.

Use async dependencies (mw.loader.using(..., callback), jQuery.getScript(.., callback);) instead and in place where you use them, not globally.
Comment 5 Mark A. Hershberger 2013-01-14 15:44:45 UTC
(In reply to comment #4)
> The module wasn't loadable because it isn't registered (this is something
> your custom startup module needs to do, which it can as it is given the
> name). So it can't be referred to anywhere unless it is registered by your
> startup module.

Then the whole loaderScripts parameter is out of place, or, at the very least, mis-designed and poorly documented.

Reopening this bug and re-titling it to focus on the design/documentation issues.
Comment 6 Krinkle 2013-01-14 16:43:11 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > The module wasn't loadable because it isn't registered (this is something
> > your custom startup module needs to do, which it can as it is given the
> > name). So it can't be referred to anywhere unless it is registered by your
> > startup module.
> 
> Then the whole loaderScripts parameter is out of place, or, at the very
> least, mis-designed and poorly documented.
>

I haven't needed to use it often, but this is highly exaggerated and uncalled for.

From observation this is what I got, please clarify:

* You don't know much about how ResourceLoader works internally
* You don't understand the purpose of the "loaderScripts" feature.
* You're trying to do something (what exactly?) in an extension that is related to the words "loader" and "scripts". So you look for documentation, don't find much but do find "loaderScripts".
* You decide to try and use it, and fail because the way it should be used is undocumented. It isn't doing what you want and so claim the feature is mis-designed?

It is not mis-designed, just (arguably) unfortunately named and (yes, most certainly) undocumented. However the feature itself is very simple and working just fine if you know what it is for and how to use it. There are no known bugs, yet (Thanks @mdale).

What you're looking for is not loaderScripts. loaderScripts is a feature to implement a custom module registry source and/or have dynamically determined dependencies and/or need to execute certain logic before the first mw.loader call.

It is not however in anyway related to using "mw.loader.load" and adding a dependency on a script to be loaded by a url.

If you explain what it is you're trying to do we can point you to a feature that is relevant to your problem and might actually solve it, too.
Comment 7 Mark A. Hershberger 2013-01-14 19:49:47 UTC
(In reply to comment #6)
> > Then the whole loaderScripts parameter is out of place, or, at the very
> > least, mis-designed and poorly documented.
> 
> I haven't needed to use it often, but this is highly exaggerated and uncalled
> for.

Truly, then: I apologize.  I think we're agreed that the documentation is lacking.  I should have left it at that.

> * You don't know much about how ResourceLoader works internally

True.

> * You don't understand the purpose of the "loaderScripts" feature.

True.  Though I would point out the documentation for it is could be improved.

> * You're trying to do something (what exactly?) in an extension that is
> related to the words "loader" and "scripts". So you look for documentation,
> don't find much but do find "loaderScripts".

You've already shown me how to do what I wanted.  I appreciate that.  I was trying to focus on the documentation issue and ask you to consider how this is designed.  It acts in unexpected ways.

> * You decide to try and use it, and fail because the way it should be used is
> undocumented. It isn't doing what you want and so claim the feature is
> mis-designed?

That it didn't do what I wanted was because of the documentation and lack of knowledge.  You were a great help in this.

When the option is used, it changes the behavior of RL in unexpected ways.  I think that is where the poor design comes in.

This design problem could just be a naming issue, but when I add "loaderScripts" to a working stanza and leave my later mw.loader.load that depends on that compoent unchanged, mw.loader.load can no longer find the stanza.

Should adding "loaderScripts" to a previously working stanza result in an emitted warning:

 Error: Unknown dependency: ext.abc

That is the misdesign I was talking about.  An error may be appropriate, but it should be coming from somewhere else.  If adding "loaderScripts" makes a dependency suddenly unknown, then something is wrong.

Maybe it is as easy as improving the error handling here.  The developer should be told something like "You used 'loaderScripts' but forgot to do X".
Comment 8 Krinkle 2013-03-06 05:48:01 UTC
(In reply to comment #7)
> Should adding "loaderScripts" to a previously working stanza result in an
> emitted warning:
> 
>  Error: Unknown dependency: ext.abc

Yes, because defining loaderScripts means you take over responsibility of registering the modules. If the files provided to loaderScripts don't actually do that, then it is a missing dependency as it is unregistered and unknown.


> Maybe it is as easy as improving the error handling here.  The developer
> should be told something like "You used 'loaderScripts' but forgot to do X".

That would require the front-end to know all the different ways and sources that can define modules in the back-end. To the frontend they are all the same (mw.loader.register). And it can't be evaluated in the backend because it is javascript code that does the registering if you have a custom loaderScript.

If you use the standard loaderScript (e.g. don't define that property in the module stanza) then it all works fine.

Error handling could be better but in this case the changes requirement to do that are not worth the overhead and cost, which is especially hard to justify in a framework that tries to be as efficient as it can (the loader shouldn't make loading harder, but faster and more efficient as without its presence).

I'm afraid that in this case it is simply a matter of using a feature that is too low-level (overr-riding the entire loader script) and as such is  allowed to cause all kinds of errors without a clear cause, because it bypasses that by design.

While the error is not very descriptive, it is very consistent and should be immediately noticeable if you happen to use loaderScripts by accident. Basic debugging should quickly lead one back to the change that caused it.

--

I improved the documentation a bit by removing mentions from common examples (to avoid accidental misuse), and in the places I did leave it, I elaborated the description.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links