@Jdlrobson Okay, rephrased.

Bottom line:

• If they are special pages only available on mobile (regardless of skin), or only available on Minerva (regardless of target), then they should be restricted to that target or skin.
• If the variables are only needed on the special page itself, they should be moved from StartupModule to OutputPage. They currently add overhead to the universally shared payload for all page views (all users/pages/skins/targets).

So let's say MFNearbyEndpoint is added only for Special:Nearby to be used by mobile.special.nearby.scripts

What happens if that module is loaded on a page other than Special:Nearby? e.g. a new page called Special:NearbyMap ?

It feels like what is actually needed here is a way to tie config variables to ResourceLoader modules in the same way as messages, so that they don't get loaded on every page view.

Change 429243 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Restrict mobile-site config variables to mobile site

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

^ stripping these from the HTML makes total sense though if that's actually what you were getting at. We use onResourceLoaderGetConfigVars for all other variables because they are used in specific ResourceLoader modules.

Change 429243 abandoned by Jdlrobson:
Restrict mobile-site config variables to mobile site

Reason:
Arghh, as demonstrated this is a little messier than I thought - some of the variables there are used by RL modules. Will need to think some more.

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

The ResourceLoader cache is fragmented by mobile/desktop, and also fragmented by skin. These need not and should not be exposed. The solution involves a single conditional statement.

Raising priority because the situation is regressing instead of improving per T205355, which has now added another variable for wikis/ platforms/ skins/ users.

Per https://phabricator.wikimedia.org/T186062#4162793 i think this needs a bit of analysis before working on.

Not quite sure how to proceed here. It seems these configs are intentionally added in onResourceLoaderGetConfigVars to ensure they can be turned on/off promptly.
What alternative ways are there to this which still ship the config via JS?

@Jdlrobson I wasn't suggesting to use a different hook. Although for variables not needed on most articles, one should definitely use $special->getOutput()->addJsConfigVars() instead, but that's a separate issue.

The ResourceLoaderGetConfigVars hook is for variables that will be in the startup manifest for all pages and all users. And indeed, they have a short expiry. While the startup module does not vary by page or user, it does vary by skin, by domain, and by target..

This means you can and definitely should reduce the return value as much as possible based on skin, based on targets, and based on shouldDisplayMobileView(). These would not cause cache pollution.

Change 465667 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] ResourceLoaderGetConfigVars is passed skin

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

ResourceLoaderGetConfigVars doesn't have access to the skin. If it does vary, that should be made explicit ^
What do you think?

A pessimistic 5 was estimated by @nray @pmiazga, @Niedzielski and @Jdlrobson

@Krinkle can hopefully schedule this for next week provided https://gerrit.wikimedia.org/r/465667 lands in core!

Change 465667 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: ResourceLoaderGetConfigVars is passed skin

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

Change 470917 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] No Minerva variables should be loaded on a non-Minerva skin

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

Change 470918 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Don't ship wgMFDescription if you're not using it

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

Change 470919 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Config: Nearby and Upload config is loaded inside special page

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

After the above patches, the majority of config variables are taken care of. For the remaining ones, I'd ideally want to tie them to ResourceLoader module since they are needed by ResourceLoader modules that can and may be loaded outside of the mobile domain and/or skin.

These are:

			// Page.js
			'wgMFMobileFormatterHeadings' => $config->get( 'MFMobileFormatterHeadings' ),
			// extendSearchParams
			'wgMFSearchAPIParams' => $searchParams,
			'wgMFQueryPropModules' => $pageProps,
			// SearchGateway.js
			'wgMFSearchGenerator' => $config->get( 'MFSearchGenerator' ),
			// PhotoListGateway.js, SearchGateway.js
			'wgMFThumbnailSizes' => [
				'tiny' => MobilePage::TINY_IMAGE_WIDTH,
				'small' => MobilePage::SMALL_IMAGE_WIDTH,
			],
			// EditorOverlayBase.js
			'wgMFEditorOptions' => $config->get( 'MFEditorOptions' ),
			// Skin.js
			'wgMFLicense' => MobileFrontendSkinHooks::getLicense( 'editor' ),
			// schemaMobileWebSearch.js
			'wgMFSchemaSearchSampleRate' => $config->get( 'MFSchemaSearchSampleRate' ),
			// mobile.init
			'wgMFExperiments' => $config->get( 'MFExperiments' ),
			'wgMFEnableJSConsoleRecruitment' => $config->get( 'MFEnableJSConsoleRecruitment' ),
			// Browser.js
			'wgMFDeviceWidthTablet' => self::DEVICE_WIDTH_TABLET,
			// toggle.js
			'wgMFCollapseSectionsByDefault' => $config->get( 'MFCollapseSectionsByDefault' ),
			// BlockMessage.js
			'wgMFTrackBlockNotices' => $config->get( 'MFTrackBlockNotices' ),

Before I start writing a module that extends ResourceLoaderFileModule I wanted to check in with @Krinkle about whether that's sane or whether we could devise a generic hook that runs on a per module basis and would help everyone.

e.g. something like this:

public static function onResourceLoaderModuleGetConfigVars( $moduleName, &$vars ) {
  if ( $moduleName === 'mobile.startup' ) {
    $vars += [ 'MFStartupConfigVar' = > 2 ];
  }
}

Change 470918 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Don't ship wgMFDescription if you're not using it

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

Change 470919 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Config: Nearby and Upload config is loaded inside special page

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

Change 472490 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Fix SpecialUploads.php config.get call

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

Change 470917 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] No Minerva variables should be loaded on a non-Minerva skin

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

Change 472490 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Fix SpecialUploads.php config.get call

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

@Krinkle this is now blocked on T186062#4711154

To summarise:
Config values such as MFCollapseSectionsByDefault are needed by modules such as mobile.toggle. These modules can be loaded outside the mobile domain e.g. Minerva desktop
Thus, these config values need to be shipped every time mobile.toggle is loaded.

We have 3 options

1. Extend ResourceLoaderFileModule and ship the config with the module that needs it
2. Provide a mechanism in core for ResourceLoader modules to add config to certain RL modules e.g. via hook
3. Break these modules when operating in desktop mode.

Let me know what you think makes the most sense here and I'll continue on this path.

@Krinkle is out on vacation till 21st (E965) so moving out of sprint.

@Jdlrobson I'm not sure this needs to be blocked, on T210035.

If I understand correctly, the objective is to phase out some of Minerva's site-wide config in favour of bundling as JavaScript values with an existing file module.

Using site-wide configuration variables in that way is counter the design of ResourceLoader (given the blocking and shortly-cached nature of the startup module). When this use case first came up, we realised ResourceLoader accounts for it with the ability to extend FileModule::getScript.

That solution is well-tested and is also what others have adopted since. Examples can be found in core with ResourceLoaderMediaWikiUtilModule, JqueryMsgModule, and LanguageDataModule. And in extensions such as Wikibase, CodeMirror, VisualEditor, Cite, and others. A subclass requires around 10 lines of code to get started (P7854). Not nothing, but not too much, I think.

Roan recently proposed a declarative abstraction for this, given the use case is becoming more common; That would make it even easier to add data to a file module in the future. This abstraction layer, however, is not (yet) part of a quarter's roadmap. Once here, migration would be straight-forward. But, migrating would also be optional, because we extend FileModule::getScript for other cases too. It will remain a long-term supported way of using ResourceLoader.

If you encounter any issues, have questions, or would prefer guidance, I'm here to help :)

That solution is well-tested and is also what others have adopted since. Examples can be found in core with ResourceLoaderMediaWikiUtilModule, JqueryMsgModule, and LanguageDataModule. And in extensions such as Wikibase, CodeMirror, VisualEditor, Cite, and others. A subclass requires around 10 lines of code to get started (P7854). Not nothing, but not too much, I think.

Given so many are using it.. it does feel like something generic might be useful.

Object.keys(mw.config.values).filter((key)=>key.indexOf('wgMF') > -1 )

In the short term, MobileFrontend we could bundle all this config inside the mobile.startup module. Ideally I'd want to see config such as wgMFEditorOptions tied to the module it relates to meaning it's easy for config to shipped that is not actually being used.

Really it comes down to urgency on your part. I'd love to spend some time thinking about a generic solution, rather than fixing it this way, but I am open to doing so.

I often audit mobile/desktop pages and analyse the critical path between the start of the HTML and the first bit of JS execution that is "meaningful" to users. These variables appear early on as low-hanging fruit for removal, to reach that point of execution earlier.

We have a long tail of small issues like this that, individually, are hard to quantify as urgent.

Input on the future interface is welcome at change 471370. This would provide a 3-line declarative interface to the existing 10-line procedural interface. It's part f a larger effort I hope to finish in a quarter or two.

As I see it, the existing and proposed next-gen interface both provide an generic way for exporting any key-value pair. No part of the current approach would be specific to MobileFrontend or its variables, and existing uses demonstrate that.

I do recognise that the public PHP-interface to ResourceLoader isn't something people use much. Having said that, it's well-documented and I've provided an example snippet to get started with. If you CC me on the patch, I would be happy to help and see it through.

Also agreed regarding module attachment. The produced JavaScript statement could use mw.config as today, but we could also assign a module-owned property instead - like mw.x.y = %s', or store it privately via mw.x.setY(%s).