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