Last modified: 2014-11-17 10:34:43 UTC
After enabling *all* gadgets on commons.beta, I found far too many instances of gadgets that would require they be wrapped in mw.loader.using( 'mediawiki.util', function ( $ ) { ... }); to be reasonable. The only other instance of a dependency problem I found like this was with <http://commons.wikimedia.beta.wmflabs.org/wiki/MediaWiki:Gadget-Flickrfixr.js>. mw.util is used in enough gadgets, though, that gadget-loading itself should be wrapped with that dependency.
They shouldn't be wrapped in mw.loader.using. The only scripts may have to be wrapped is MediaWiki:Common.js and user/common.js (due to there not being a module registry where one can declare dependencies). For core, extension and gadget modules dependencies should be declared in the registry. So for gadgets that would be MediaWiki:Gadgets-definition. Then the dependencies will be loaded in the same request (and never more than once).
I've done a few modules as example on wmflabs. The syntax is ulgy (Resources.php[1] is prettier), but a prettified gadget editor is coming in the RL2/Gadgets 2.0 branch) Krinkle [1] http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/resources/Resources.php?revision=108222&view=markup#l542
http://www.mediawiki.org/wiki/ResourceLoader/Migration_guide_(users) Doesn't seem to mention how to do dependencies.
Declaring dependencies is not part of the code migration. The moment mw.util came into existance is also when ResourceLoader, dependencies, mw.loader et al came into existance. Just like a dependency on 'jquery.cookie', 'jquery.ui.dialog' had to be declared last year, it's no different for 'mediawiki.util' or 'mediawiki.user'. It seems to become part of migration for 1.18 because in 1.18 modules load faster and missing dependencies are more obvious. In 1.17 modules loaded less asynchronous so modules could lack a 'jquery.cookie' dependency declaration because some random extension's module (e.g. Vector's collapsible sidebar portlets) was also loading it and it happened to load just before the gadget. And that is as bad as it sounds, a concurrency problem. Contrary to PHP developers, the current status quo around JS is mostly * "it doesn't throw exceptions all over so we need not do anything" * "if I copy the line that does X to a different environment and context it does the same" (e.g. copying 2 lines from a gadget without looking at the rest of the gadget or the registry in MediaWiki:Gadgets-definition) Nobody is to blame for that but it needs to change now that gadgets are modules and not just a few lines of code. The reason gadgets are missing those dependencies is probably because things were copied and pasted out of context and it "happened to work". The syntax for dependencies is documented on Extension:Gadgets [1] - Krinkle [1] https://www.mediawiki.org/wiki/Extension:Gadgets#Format
Since people are going to look at that page (at least I hope), it wouldn't hurt to mention this there too.
(In reply to comment #5) > Since people are going to look at that page (at least I hope), it wouldn't hurt > to mention this there too. Sure an extra reminder doesn't hurt. Especially now that we know most people forgot to do this when 1.17 was deployed in 2010. Both for backwards compatibility with 1.17 and 1.18[1], I'd recommend not adding any modules to the list of modules that can be assumed present in any part of the execution flow. Krinkle [1] I mention compatibility with 1.17 and 1.18 because all gadgets currently missing them on live wmf wikis are already potentially breaking for some users. It's not a regression. Just a race condition more likely to happen on code that was already supposed to have it in the first place.
Without some sort of change, then this will create a problem for all wikis that have gadgets. I've already told enwiki what is needed (https://en.wikipedia.org/w/index.php?diff=471706835) but repeating this for all wikis is a lot of work and will result in a lot of complaints.
(In reply to comment #1) > The only scripts may have to be wrapped is MediaWiki:Common.js and > user/common.js (due to there not being a module registry where one > can declare dependencies). But... Since MediaWiki:Common.css is *itself* a module, surely a dependency on mediawiki.util can be declared when *it* loads? I think the numer of MW installations not using mw.util can be counted on one hand. Why make it a (default-on) option like $mwPreloadMwUtil?
(In reply to comment #9) > I think the numer of MW installations not using mw.util can be counted on one hand. I doubt that. There has never been an automatic dependency on anything. If people "migrate" their gadgets to ResourceLoader and leave them broken by not declaring dependencies or some other important part of how ResourceLoader works, thats... unfortunate. But I'm sure that the gadgets we are "fixing" on labs are currently broken in production as well, just not as often (due to the race condition). Any gadget missing a dependency or a document ready hook will break at some point in some scenarios. And I get these reports all the time on IRC in #wikimedia-stewards or #mediawiki or wherever. 9/10 times is a copy/paste error not including the proper context or dependencies. Nothing new. Remember the report last year from IE6/IE7 users about $.cookie being undefined in Vector.js on en.wikipedia ? (I think Edokter discovered that one) That was very odd as it only happened to some users, but it turns out not to be a bug. That was a race condition and a missing dependency, IE6/7 loaded a bit slower. It was fixed then by not using $.cookie but the proper fix wouldl've been to simply add that module to do the dependencies. (In reply to comment #9) > But... Since MediaWiki:Common.css is *itself* a module, surely a dependency on > mediawiki.util can be declared when *it* loads? Sure it could be added as a dependency. And we probably should since so many users forgot to do it, but this is a very bad habbit to go into. I'd rather avoid it. I think its safe to say that there are no high usage site scripts or gadgets currently in production missing dependencies or document-ready hooks, otherwise we would've had complaints already as that code is technically broken as it is. Also, I'd recommend that whatever fixes we do on Labs, instead do them on the live wikis right away. Those fixes are not 1.18 of 1.19 specific. If a gadget isn't declaring their dependencies, it's probably wise to check it out further as there's a good chance there's other copy/paste errors. [1] https://meta.wikimedia.org/wiki/User:Krinkle/Le_Tour_de_Wik%C3%AD/1-17-allwikis
(In reply to comment #10) > > I doubt that. There has never been an automatic dependency on anything. That's because we never had ResourceLoader before. > Remember the report last year from IE6/IE7 users about $.cookie being undefined > in Vector.js on en.wikipedia ? (I think Edokter discovered that one) That was > very odd as it only happened to some users, but it turns out not to be a bug. > That was a race condition and a missing dependency, IE6/7 loaded a bit slower. > It was fixed then by not using $.cookie but the proper fix wouldl've been to > simply add that module to do the dependencies. I made a comment to that bug (bug 29384) just today. I think we can close that now we know what casues it. > (In reply to comment #9) > > But... Since MediaWiki:Common.js is *itself* a module, surely a dependency on > > mediawiki.util can be declared when *it* loads? > > Sure it could be added as a dependency. And we probably should since so many > users forgot to do it, but this is a very bad habbit to go into. I'd rather > avoid it. Note that my suggestion would only apply to MediaWiki:Common.js/User:common.js, *because* it is the only module that has no mechanism for declaring dependancies (I just loathe the inline wrapping method). Declaring dependencies in MediaWiki:gadgets-defenition is fine with me, as was evidently necessary from the start. > I think its safe to say that there are no high usage site scripts or gadgets > currently in production missing dependencies or document-ready hooks, otherwise > we would've had complaints already as that code is technically broken as it is. I doubt that. There are plenty of gadgets on enwiki that use ResourceLoader without declaring there dependencies. With declaring those becoming mandatory, that is when we will see the most problem. And I do agree they need to be declared. But that is easy to fix. My only beef remains common.js. If we aren't going to pre-declare mediawiki.util for common.js, I still like to know if wrapping the entire file is absolutely necessary? Is mw.loader.using() asynchronous or synchronous. The documentation says to use "two or three parameter", but the source reveals that both callback parameters are optional. So would simply putting the line "mw.loader.using('some.module');" not be adequate? If not, would it be hard to make .using synchronous and make it simply use return(); on completion if no callback is passed?
(In reply to comment #11) > (In reply to comment #10) > > > > I doubt that. There has never been an automatic dependency on anything. > > That's because we never had ResourceLoader before. Exactly. Before ResourceLoader, mw.util didn't exist. When it came into existence people should have known that it required declaring a dependency. In "most" cases this was done, but the people that forgot or didn't know about it likely didn't see any problem because another module happened to cover for them, in time. Basically I mean that with ResourceLoader came dependencies, not in two stages, but as one thing. Again, I'm not blaming anyone for forgetting that, but that is how it is. > I made a comment to that bug (bug 29384) just today. I think we can close that > now we know what casues it. Yep, done. Thanks. > > > Sure it could be added as a dependency. And we probably should since so many > > users forgot to do it, but this is a very bad habbit to go into. I'd rather > > avoid it. > > Note that my suggestion would only apply to MediaWiki:Common.js/User:common.js, > *because* it is the only module that has no mechanism for declaring > dependancies .. > > > I think its safe to say that there are no high usage site scripts or gadgets > > currently in production missing dependencies or document-ready hooks, otherwise > > we would've had complaints already as that code is technically broken as it is. > > I doubt that. There are plenty of gadgets on enwiki that use ResourceLoader > without declaring there dependencies. With declaring those becoming mandatory, > that is when we will see the most problem. And I do agree they need to be > declared. But that is easy to fix. My only beef remains common.js. We've discussed this on IRC in the mean time. Declaring dependencies was never optional and this system has not changed in 1.19 or 1.18, since it's introduction with ResourceLoader 1.0 in MediaWiki 1.17. So whatever gadget has been migrated to use ResourceLoader modules should have including declaring dependencies in that migration, that's simply part of the proces, just as important as anything else. Basically all 1.19 does is load javascript faster, making it more likely that code that uses a module but doesn't declare it as a dependency fail. Which is a unfortunately more common that it should be (it shouldn't exist at all), but in the end it's a downstream/user problem. I believe we can get this easily fixed before deployment of 1.19 so let's try that first. It's just adding of dependency declarations in the registry. > If we aren't going to pre-declare mediawiki.util for common.js, I still like to > know if wrapping the entire file is absolutely necessary? No, not per se. Code that uses code from another module needs to be declared in such a way that it will be executed after that other module is loaded. In a scenario where no module registry is available this means using mw.loader.using()'s "success"-callback. You can wrap part of a script or all of it, doesn't matter. > [..] Is mw.loader.using() asynchronous or synchronous. Both mw.loader.load and mw.loader.using are asynchronous. So: > mw.log('foo') > mw.loader.using( 'module-x', function () { > // module-x is loaded and ready for use > mw.log('quux') > }); > mw.log('bar') Will, assuming module-x hasn't been loaded yet, result in "foo" > "bar" > "quux". > The documentation [of mw.loader.using] says to use "two or three > parameter", but the source reveals that both callback parameters are optional. Yes, it says to use two or three, because using no callback at all is quite a pointless use of mw.loader.using; If you're not using the module inside code, why use using()... > So would simply putting the line "mw.loader.using('some.module');" not be > adequate? If not, would it be hard to make .using synchronous and make it > simply use return(); on completion if no callback is passed? No, it's either impossible or "bad" use of javascript. The asynchronous nature and event-driven flow is what allows things to happen the way they do. It makes no sense to block javascript execution because in one script there is a dependency on something and it loads the module on the first line. If you've got a bunch of code that depends on one or more modules the solution is to request those modules to be loaded (asynchronously, allowing other scripts to do their thing in the mean time) and while that is request is running the callback is remembered, and as soon as the module request is done it'll come back to that function and execute it.
Getting back at the subject at hand... I imagine some wikis would want to use mw.util in common.js and not bother with wrapping it. Would an option $wgIncludeUtilJavaScript (which would load mw.util as a dependency of common.js) be feasable as a convenient shorthand to wrapping it in mw.loader.using?
I think we've spend more than enough hours on this report. Summary from my perspective: * This root bug itself is invalid: Referring to code defined in a module that you don't load fails, obviously. Simple. This has been the case forever and never changed. * If one is unwilling to declare dependencies on other modules than don't write any code from other modules. * Before 1.18 scripts were loaded slower, and the slower things go, the less race conditions. * Any script that throws errors for "you" in 1.19-svn at beta.wmflabs.org but works under 1.18wmf1 will be throwing errors for other users under 1.18wmf1 already. There is no regression, only faster load order and higher likelihood of scripts failing that lack dependency declarations or loader callbacks -- * 1.19 is not branched or deployed, wikis have sufficient time to take a good look at their gadgets and check if they have any gadgets using other modules that aren't marked as "depending" on them. Marking WONTFIX as such.
I'm not sure how closing this will match with "Let's ensure that the Wiki* experience is consistent" -- a statement that RobLa agreed with. See http://thread.gmane.org/gmane.science.linguistics.wikipedia.technical/58397/focus=58453 We have code that worked (despite the lack of dependencies being named) at least 75% of the time and not fixing this bug would mean it suddenly fails 100% of the time. If we want to let people know that they shouldn't expect this to behave in the future, that's fine. Because it worked in the past (for whatever reason) we should clearly communicate that behavior people have come to expect will be deprecated in the future while allowing it to work in this upcoming release.
Fixed in r110254.
Would it be possible to grab JS files from all wikis and fix these missing dependencies (or at least make a list to be manually fixed) automatically? Then the fixes could be made and $wgPreloadJavaScriptMwUtil phased out?
With bug 33711/r110542 now loading mw.util as a dependency of mw.page.startup in the top load queue, this bug seems deprecated.
