Load visualeditor modules in a closure mapping $/mw/ve to their globals
OpenPublic

Description

This may introduce a really large number of scopes and have undesirable performance effects, but we need to come up with a solution nonetheless.


Version: unspecified
Severity: enhancement

bzimport set Reference to bz48885.
TrevorParscal created this task.Via LegacyMay 28 2013, 9:25 AM
Krinkle added a comment.Via ConduitMay 28 2013, 9:44 AM

For the record, inside ResourceLoader context $ is a local variable provided by the ResourceLoader built-in closure.

See also bug 48886.

Krinkle added a comment.Via ConduitJul 7 2014, 11:51 PM

I suggest for standalone we introduce an module-intro/outro that is used to wrap around individual modules (not per file).

So e.g. dist/ve.core.js would be:

  • intro

( function ( ve, $ ) {

  • include ve.js
  • include ve.A.js
  • include ve.B.js
  • include ve.C.js
  • outro

}( VisualEditor, jQuery ) );

And in MediaWiki the same would effectively happen but on-demand by ResourceLoader (since we don't want to use a build system for MW, but instead work on the raw source files directly and have them build dynamically).

See bug 48886 for details, but the end result would be:

  • startup:

register(
..,
[ 'ext.visualEditor', 1234, ...., ['VisualEditor']
);

  • mw.loader: script( $, global[propKeys..].. );
  • load.php response when loading visualeditor:

mw.loader.implement( 'ext.visualEditor.core', function ( $, ve ) {

  • include ve.js
  • include ve.A.js
  • include ve.B.js
  • include ve.C.js

} );

Krinkle added a comment.Via ConduitJul 8 2014, 12:17 AM

Rephrasing bug.

Problems:

  • We're repeatedly referencing $/mw from global scope instead of securing the reference. This is worse for performance and for proper functioning (e.g. jQuery might be redefined at a later time causing version mismatches, this is why ResourceLoader maps $)
  • We're using $ and mw directly instead of mapping jQuery and mediaWiki.
  • In standalone, our code executes in the global scope. This is bad.

Solution:

  • For standalone, build the dist/ files with a closure mapping, securing and caching '$' from 'jQuery'. And for VE modules other than ve.base, it would also map 've' to 'VisualEditor' (bug 67642)
  • In MediaWiki, ResourceLoader already provides a closure which we'd extend to also map 've' to 'VisualEditor'.
Jdforrester-WMF changed the title from "VisualEditor: Load visualeditor modules in a closure mapping $/mw/ve to their globals" to "Load visualeditor modules in a closure mapping $/mw/ve to their globals".Via WebNov 23 2014, 11:08 PM
Jdforrester-WMF lowered the priority of this task from "High" to "Normal".
Jdforrester-WMF added a project: VisualEditor.
Jdforrester-WMF set Security to None.
Jdforrester-WMF moved this task to Next up: TechDebt on the VisualEditor workboard.Via WebNov 23 2014, 11:16 PM
Jdforrester-WMF placed this task up for grabs.Via WebJan 15 2015, 12:11 AM
Jdforrester-WMF moved this task to Backlog on the VisualEditor workboard.

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.