I'm guessing the easiest thing to do here is to add Thanks as a dependency of DiscussionTools.

I’d rather avoid forcing all DiscussionTools users to install Thanks, which is only needed for a tiny sub-feature. Does it cause issues at run time? Then it should be refactored to avoid them (e.g. by conditionally registering the ResourceLoader module). If it causes no run time issues, then maybe we should tell people not to run tests without Thanks but not set it as a hard dependency?

I agree, not everyone needs Thanks (the extension, of course; everyone should be thanked somehow I hope!). It is only an issue with the tests, so you're right we could just tell developers to enable Thanks. After all, both Thanks and DiscussionTools are bundled with MediaWiki.

However, the only times I've come up against this issue is when helping a newcomer to run tests on core and trying to run tests in CI, and there are already twenty other things that don't quite work properly out of the box. This task was just me trying to reduce one small road-bump. :)

Loading the messages separately rather than as part of ext.discussionTools.init would make conceptual sense, I'm not sure if the overhead is wanted though.

In T403315#11134264, @Samwilson wrote:

Loading the messages separately rather than as part of ext.discussionTools.init would make conceptual sense, I'm not sure if the overhead is wanted though.

Making extra HTTP requests would slow things down and make caching impossible, which is a problem even if – as far as I understand – the JS code requires this message (as well as three other messages thanks-button-thanked, thanks-confirmation2 and thanks-thanked-notice) only upon user interaction. Instead of loading them dynamically, if something needs to be done, I’d

• either add a factory and extend the list of messages there, similarly to the reply tool, which dynamically adds ext.confirmEdit.CaptchaInputWidget as a dependency only if it’s available,
• or add a virtual package file similarly to controller/contLangMessages.json, but include these messages in user language rather than content language, and only if Thanks is available.

I think this is only a test issue, and not a user-facing problem. There are checks for whether Thanks is installed in the PHP code, and the JS code will only kick in when the PHP code generates "thanks" buttons on the page.

In T403315#11134390, @Tacsipacsi wrote:

Instead of loading them dynamically, if something needs to be done, I’d

• either add a factory and extend the list of messages there, similarly to the reply tool, which dynamically adds ext.confirmEdit.CaptchaInputWidget as a dependency only if it’s available,
• or add a virtual package file similarly to controller/contLangMessages.json, but include these messages in user language rather than content language, and only if Thanks is available.

Yeah, I would do this. Either option would work, and I don't think we have a convention, so I'd just do whichever seems easier.

Change #1184524 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/DiscussionTools@master] Extend addOptionalDependencies to include messages

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

I've extended the optionalDependencies system to be support generic optional keys (dependencies, messages, scripts, etc.)`

"optional": {
	"Thanks": {
		"packageFiles": [
			"thanks.js"
		],
		"messages": [
			"cancel",
			"thanks-button-thank",
			"thanks-button-thanked",
			"thanks-confirmation2",
			"thanks-thanked-notice"
		]
	}
},

This might be useful to upstream to core, although if we supported private modules (T225842), there would be no cost to separating this out into ext.discussionTools.thanks, and then we could just have optionalDependencies again.

Change #1184524 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Extend addOptionalDependencies to extend any property

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