* Daniel Friesen <li...@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?
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.
> $('<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
_______________________________________________
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
