Page MenuHomePhabricator

VE plugins from user scripts suffer from race condition
Open, Needs TriagePublic

Description

The usual way to add plugins to VE from a user script suffers from a race condition, that probably will get worse with jQuery 3 (T124742: Upgrade to jQuery 3). What you usually (= commonly used and documented this way) do, is this:

In your common.js you add code like

mw.loader.using( 'ext.visualEditor.desktopArticleTarget.init' ).done( function() {
	mw.libs.ve.addPlugin( function() { // (1)
		return $.getScript( 'URL to plugin' ); // (2)
	} );
} );

where the referenced script is something like

function init() {
	//some code to define a tool for the toolbar
}

mw.loader.using( [ 'ext.visualEditor.core', 'more dependencies' ] ).then( init ); // (3)

This fails, when

  • not all dependencies from (3) are loaded before the user script (confirmed in a real scenario)
  • jQuery 3 is used (probably, didn't test yet)

This is what happens:

  • User clicks on "Edit" to load VE.
  • The function defined in (1) is executed, loading and executing the script
  • Note that executing the script doesn't always mean executing the init function, it will only be scheduled for execution in jQuery 3 (which will fire it asynchronously even if the dependencies are all ready) or when not all dependencies are ready yet.
  • The deferred created in (2) is resolved.
  • VE continues initialization (assuming no other plugins aren't ready yet), which will initialize the toolbar.
  • The init function is executed, but it's too late to define a tool for the toolbar (such tools will be included in the toolbar in some dialogs like for editing references, though, as these aren't created yet).
  • User is unhappy that the plugin is missing though he followed the documentation.

I can see two possible solutions:

Either update the documentation to advice the following code:

mw.loader.using( 'ext.visualEditor.desktopArticleTarget.init' ).done( function() {
	mw.libs.ve.addPlugin( function() {
		return mw.loader.using( [ 'ext.visualEditor.core', 'more dependencies' ] ).then( function() {
			return $.getScript( 'URL to plugin' );
		} );
	} );
} );

Then in the script you can just call the init function right away, which will be early enough. This has the following disadvantages:

  • Might take longer, as the script is loaded only after the VE modules, not parallel to them as before.
  • Updating the dependencies requires all users to update the code they use to load the script.

Or add two public functions to the VE initialization code to hold and release the deferred waiting for all plugins to load. Then the script could be written like

function init() {
	//some code to define a tool for the toolbar
	mw.libs.ve.releaseReady();
}

mw.libs.ve.holdReady();
mw.loader.using( [ 'ext.visualEditor.core', 'more dependencies' ] ).then( init );

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Third solution (probably best):

mw.loader.using( 'ext.visualEditor.desktopArticleTarget.init' ).done( function() {
	mw.libs.ve.addPlugin( function() {
		var d = $.Deferred();
		mw.hook( 'myCoolVEPlugin.ready' ).add( d.resolve );
		mw.loader.load( 'URL to plugin' );
		return d.promise();
	} );
} );

Then you can call mw.hook( 'myCoolVEPlugin.ready' ).fire() inside the module once you are ready.