See also:

• T232148: Provide a single-module registration mechanism for skin modules in ResourceLoader
• T225842: Allow ResourceLoader modules to be "private", bundle them with their dependent modules
• T253582: Let ResourceLoader only register skin modules for the current skin

Per T281431 this would allow us to remove 10 of the 23 modules inside Minerva.

For better understanding of how the image module works, I went to https://meta.wikimedia.org/w/load.php?modules=skins.minerva.content.styles.images and manually cleaned up the CSS included, which with formatting is

toast.mw-notification-type-error,
.mw-notification.mw-notification-type-error {
	background-image: url(/w/load.php?modules=skins.minerva.content.styles.images&image=toast.mw-notification-type-error,.mw-notification.mw-notification-type-error&format=rasterized&skin=fallback&version=12akb);
	background-image: linear-gradient(transparent,transparent),url(/w/load.php?modules=skins.minerva.content.styles.images&image=toast.mw-notification-type-error,.mw-notification.mw-notification-type-error&format=original&skin=fallback&version=12akb)
}
a.external {
	background-image: url(/w/load.php?modules=skins.minerva.content.styles.images&image=a.external&format=rasterized&lang=qqx&skin=fallback&version=12akb);
	background-image: linear-gradient(transparent,transparent),url(/w/load.php?modules=skins.minerva.content.styles.images&image=a.external&format=original&lang=qqx&skin=fallback&version=12akb)
}
a.external--reference {
	background-image: url(/w/load.php?modules=skins.minerva.content.styles.images&image=a.external&variant=reference&format=rasterized&lang=qqx&skin=fallback&version=12akb);
	background-image: linear-gradient(transparent,transparent),url(/w/load.php?modules=skins.minerva.content.styles.images&image=a.external&variant=reference&format=original&lang=qqx&skin=fallback&version=12akb)
}

this comes from the module definition

"skins.minerva.content.styles.images": {
	"class": "ResourceLoaderImageModule",
	"selectorWithoutVariant": "{name}",
	"selectorWithVariant": "{name}--{variant}",
	"defaultColor": "#36c",
	"useDataURI": false,
	"variants": {
		"reference": {
			"color": "#69f"
		}
	},
	"images": {
		"toast.mw-notification-type-error,.mw-notification.mw-notification-type-error":
			"resources/skins.minerva.content.styles.images/error.svg",
		"a.external": {
			"file": {
				"ltr": "resources/skins.minerva.content.styles.images/link-external-ltr.svg",
				"rtl": "resources/skins.minerva.content.styles.images/link-external-rtl.svg"
			},
			"variants": [
				"reference"
			]
		}
	}
},

So the image module just adds background images based on the selectors (and possible variants) configured?
What about adding this as an extra option to ResourceLoaderFileModule, to configure multiple sub image modules? Eg in a normal ResourceLoaderFileModule

"some.module.name": {
	"styles": [
		"resources/file.css",
		"resources/other.less"
	],
	"images": [
		{
			"class": "ResourceLoaderImageModule",
			"selector": ".skin-icon-bad-selector-{name}",
			"images": {
				"error": "resources/icons/error.svg",
				"warning": "resources/icons/warning.svg"
			}
		},
		{
			"class": "ResourceLoaderImageModule",
			"selector": ".skin-icon-good-selector-{name}",
			"images": {
				"success": "resources/icons/success.svg",
				"neutral": "resources/icons/neutral.svg"
			}
		}
	]
},

or something? Just an example, these are dummy entries. This can maintain support for different images needed different selector structures, because internally the array entries under "images" would each be converted to their own ResourceLoaderImageModule and the styles from that would just be added. This should also eventually work with ResourceLoaderOOUIIconPackModule too.

If this approach would be okay, I can try to spend some time working on it with MinervaNeue since there are no core modules that use ResourceLoaderImageModule.

We could also introduce a new module type, ResourceLoaderFileAndImageModule (name tbd) that just delegates to ResourceLoaderFileModule and ResourceLoaderImageModule, instead of needing to have ResourceLoaderFileModule handle images too. The new module would likewise internally just have an array of image modules to include.

Extra notes:

• update ResourceLoaderContext to try to retrieve images from file module too
• inherit local base path from file module if not specified

I don't think there is any reason why ResourceLoaderImageModule could not extend ResourceLoaderFileModule. I don't think we should need to change the format in skin.json.

Merging the definitions of skins.minerva.mainMenu.styles and skins.minerva.mainMenu.styles would be a good experiment to start with (ResourceLoaderOOUIIconPackModule extends ResourceLoaderImageModule).

In T248912#7406455, @Jdlrobson wrote:

I don't think there is any reason why ResourceLoaderImageModule could not extend ResourceLoaderFileModule. I don't think we should need to change the format in skin.json.

Merging the definitions of skins.minerva.mainMenu.styles and skins.minerva.mainMenu.styles would be a good experiment to start with (ResourceLoaderOOUIIconPackModule extends ResourceLoaderImageModule).

I proposed the structure above so that you could define images for multiple selector types in the same module, rather than needing different modules when the names are different patterns. Eg to allow merging skins.minerva.icons.page.issues.uncolored and skins.minerva.icons.page.issues.default.color because they are only used together, but are separate because they have different selectors.

Oh I see. Yes, I think eventually we'll want to find a way to consolidate icons with different patterns, but this bug was simply for the cases where Icon/Image modules could be merged with file modules. That in itself would be a huge win, I reckon we could shave away quite a few modules with that alone. I think the different patterns is more an artifact of the lack of standardization on the icon front which will hopefully be addressed in future by the move to Vue.js

In T248912#7406971, @Jdlrobson wrote:

Oh I see. Yes, I think eventually we'll want to find a way to consolidate icons with different patterns, but this bug was simply for the cases where Icon/Image modules could be merged with file modules. That in itself would be a huge win, I reckon we could shave away quite a few modules with that alone. I think the different patterns is more an artifact of the lack of standardization on the icon front which will hopefully be addressed in future by the move to Vue.js

So you would prefer that I *not* implement the proposal above of allowing multiple sub-modules for images with different formats, but just the same current input structure but combined for images and files?

So you would prefer that

I have no preference but this seems like it might be a separate but equally valid task. @Krinkle and @Catrope who maintain ResourceLoader would be best to provide input on that subject as I assume they'd be doing the code review here.

This ticket is just about making ResourceLoaderImageModule extend ResourceLoaderFileModule, which is more pressing to me and my team. However, the two problems might be related.