Page MenuHomePhabricator

ResourceLoaderWikiModule with dependencies and group=user not loading
Open, MediumPublic

Description

Not sure if this is intentional, but ResourceLoaderWikiModule seems to be behaving weirdly with group=user when dependencies are present.

Minimal steps for reproduction: add to LocalSettings.php:

class TestUserResourceLoaderModule extends ResourceLoaderWikiModule {
        public function getDependencies( ResourceLoaderContext $context = null ) {
		return ["jquery.i18n"];
	}
}
$wgHooks['ResourceLoaderRegisterModules'][] = static function ( ResourceLoader $rl ) {
	$rl->register( 'testuser', [
		'class' => TestUserResourceLoaderModule::class,
		'scripts' => [ 'MediaWiki:Testuser.js' ],
		'group' => 'user'
	] );
};
$wgHooks['BeforePageDisplay'][] = static function ( OutputPage $out ) {
	$out->addModules( "testuser" );
};

Expected behaviour:
RL module is registered and run on all pages. Code in MediaWiki:Testuser.js is executed. Module state should become ready after page load.

Actual behaviour:
RL module is registered but not run. mw.loader.moduleRegistry says the state is loaded. MediaWiki:Testuser.js gets executed only when mw.loader.load("testuser") is run.

If we remove the dependency in TestUserResourceLoaderModule, then the behaviour is as expected.

I came across this issue while working on T36958 – though the issue in that patch is different (user gadgets load, but the dependencies specified don't).

Event Timeline

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

Also if I change GadgetResourceLoaderModule's getGroup() to return 'user', now the gadgets do load and run but without their dependencies.

Krinkle triaged this task as Medium priority.Feb 22 2022, 4:00 PM
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.

I'm able to reproduce the issue, using the following slightly altered snippet. The key part is that it is a rare dependency that isn't otherwise queued already as module or indirect dependency. I went with jquery.chosen because it doesn't require the ULS extension, and is similarly not used loaded by default.

LocalSettings snippet
class TestUserResourceLoaderModule extends ResourceLoaderWikiModule {
	public function getDependencies( ResourceLoaderContext $context = null ) {
		return ['jquery.chosen'];
	}
}
$wgHooks['ResourceLoaderRegisterModules'][] = static function ( ResourceLoader $rl ) {
	$rl->register( 'testuser', [
		'class' => TestUserResourceLoaderModule::class,
		'scripts' => [ 'MediaWiki:Testuser.js' ],
		'group' => 'user',
	] );
};
$wgHooks['BeforePageDisplay'][] = static function ( OutputPage $out ) {
	$out->addModules( 'testuser' );
};

The output contains:

<script>
RLSTATE={.., "testuser":"loading", .. };
</script>
<script>
RLQ.push(function(){
  mw.loader.load("/w/load.php?lang=en\u0026modules=testuser\u0026skin=vector\u0026user=Admin\u0026version=1y3te");
});
</script>

Which is correct and as expected. The "user" group of modules is reserved and has as special characteristic that it may vary by user and thus needs an additional query parameter, as well as a dedicated version hash that must be embedded in each page response and not cached anywhere (especially not in the startup module). This is why the request is not made through the regular declarative module list (internally RLPAGEMODULES processed by mw.loader.load) but rather explicitly with the URL already fully-formed. And the declarative loader is informed of this act by seeding its state as loading instead of as registered, which avoids double loading or other split-brain errors.

However, because the loader is never in change of processing the module request, it also means it never resolves its dependencies and queues them up for loading. Hence when the testuser request comes back with an implementation, this implementation is stored for execution later because its dependencies have not yet arrived, and unfortunately never will because their request never started.

This is not a problem in practice because user scripts by their very nature are standalone scripts with no place to declare their dependencies. Rather, user scripts use mw.loader.using() to load their dependencies.

This is a reserved group for internal use by core, where it is working as intended. As such, I do not consider this a bug.

I understand that this example is simplified from where you encountered it, but the example of loading MediaWiki:Testuser.js for example would not warrant group=user since this content does not vary by user. If we did have such page vary by user, we'd likely want to bunde that in the existing user module rather than create a new one.

If we have a need in core or elsewhere for user scripts to be loaded with declared dependencies, we can of course find a way to make that work. We'd presumably do that by removing some of these optimisations and instead making the mw.loader client slightly heavier and make it capable of performing these requests directly. Perhaps by injecting the override version map similar to how we inject loader states already.

If not, then we would likely close this as declined.

Change 765274 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] resourceloader: Document optimisation in WikiModule::isKnownEmpty

https://gerrit.wikimedia.org/r/765274

Change 765274 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Document optimisation in WikiModule::isKnownEmpty

https://gerrit.wikimedia.org/r/765274

Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.
Krinkle added a subscriber: Krinkle.