On Mon, 19 Mar 2012 02:07:54 -0700, Dmitriy Sintsov questpc@rambler.ru wrote:
- Daniel Friesen lists@nadir-seen-fire.com [Mon, 19 Mar 2012 01:35:21
-0700]:
Autoloading classes is not possible. Even if every browser supported getters and we could use them to dynamically load classes, this would require synchronous http calls. Which are absolutely HORRIBLE because they block the entire JS thread and in addition typically even block the browser's UI.
How does ResourceLoader figures that function loadEditPrototypes() was defined in 'ext.jqgmap.edit' when I call this function from 'ext.jqgmap'? And if there was no extra http call, then why the list of scripts in Chrome debugger does not include 'JQGmap.edit.js' among the scripts before mw.loader.using() and loadEditPrototypes() call was performed?
RL doesn't know anything about loadEditPrototypes, I don't know what would make you think that it does.
I can't really debug and say what's wrong without your code actually running somewhere.
You shouldn't be dropping the closure, you don't want local things in the global scope. You also shouldn't be using the global $ and mw
directly. Prototypes are not local things. They are carried out with function they belong to. I define few prototypes in 'ext.jqgmap.view' then I want to add more prototypes in 'ext.jqgmap.edit'. But not always, only when the edit mode is active. However you are right about minor local variables.
Everything should be using a pattern like: ( function( $, mw ) {
} )( jQuery, mediaWiki );
Ok, I'll try the closure call with two arguments in every module, didn't think about that. I'll report if that will fix the abrupt termination of module "chain" execution.
var jqgmap = []; for ( var mapIndex in jqgmap ) {
This is VERY bad JavaScript coding practice. Please use $.each().
I know that I can use $.each() here. I don't do that for few reasons: for..in is easier to debug (step by step key) and also because for..in was problematic only for objects {}, not for arrays [] AFAIK. However I might be wrong. Maybe I'll switch to $.each() however the code was working with for..in perfectly well before I refactored it into multiple modules. There was only one module. However it was growing so I had to split it into view and edit submodules.
Completely wrong. for..in is object iteration. It is not to be used for array iteration. When you use for..in to iterate over an array you're actually iterating over object keys. Unset things may be left out, if there are any properties set you'll iterate over those when you shouldn't, if someone sets something on Array.prototype. (eg: Someone implements .forEach in an older browser) you'll iterate over that as well, and it's perfectly valid for a browser to iterate over the keys in an order that's different from the actual order of items in the array.
$('<div class="jqgmap_code" id="jqgmap_code' + this.Idx + '">'+ '<input type="checkbox"></input>' + mw.msg(
'jqgmap-show-code'
)
'<input type="checkbox"></input>' + mw.msg('jqgmap-show-code'
)
'<div class="jqgmap_preview"></div>' + '<div class="jqgmap_preview"></div>' + '</div>')I should really start poking people about this one. `"<div>" . wfMsg( 'foo' ) . "</div>"` is bad in PHP code, and it's just as bad inside
JS.
You should be creating your html with NO + concatenation, and then
using
a .find() and .text() to insert things where they belong. That, or use multiple $() calls with .append() to create the full structure. Likewise you shouldn;t be inserting this.Idx that way into the attribute. It should be added with something like .attr( "jqgmap_code" + this.Idx );
I might consider refactoring of jQuery DOM nodes creation, however the loading of 'ext.jqgmap.edit' module does not work now so I cannot continue until I'll figure out why. This code is not ideal however it's not the reason of the fault. This code was working perfectly well before splitting into multiple modules and introducing mw.loader.using('ext.jqgmap.edit',function(){...}); Dmitriy