the two targets of doclinks cannot be reached:
<!--T:161--> Use MediaWiki's validation/sanitization methods ''e.g.'' those in the <tvar name=1>{{class doclink|Html}}</tvar> and <tvar name=2>{{class doclink|Sanitizer}}</tvar> classes.
please correct or realign, thanks --Christian 🇫🇷 FR (talk) 10:33, 16 April 2024 (UTC)
I'm confused by this item in the section about Accessibility:
SHOULD: HTML element's IDs should only be rarely, for things to which you can navigate. Everything else should be a class – even if you expect to use it only once, use a class name instead.
Why would that be an accessibility consideration? Most accessibility uses for ids aren't related to navigation and can't be replaced by classes (for example, the target of an aria-described-by="" and such).
I can see that ids should not be used for styling, but that is not an accessibility concern. And I can see that ids must be unique, but that is a MUST and also affects much more than only accessibility.
Am I missing something here?
Correct. however those should generally be auto-generated ids (again to guarantee uniqueness), but yes, this is more of a code convention than an accessibility concern.
I'd generally like to remove all content under Best practices for extensions#Accessibility and instead link to Accessibility guide for developers to avoid quadruplicating more and more guidelines.
But none of that is written in terms of should/must, so now we're adding a fudge factor back into the process, whereas this entire document is meant to be a "one-stop-shop" where if you pass all the items you can be pretty sure you could as 'best practice.
@Jdforrester (WMF) Please feel free to edit it and define MUSTs and SHOULDs. Thanks!
OK, it's been a month, we can just revert your removal Andre if you don't have time to do the work?
Surely we can duplicate content (and likely make things go out of sync), however I don't think it's a good idea when there are already guidelines for an area to add yet another place with guidelines for that area. For the time being, feel free to go ahead.
Done.
There's no rating for this item under the Database section. Is it something that must be done, should be, or recommended?
I didn't see this at the time, but I added "should" in front of it last year in this edit as part of a general formatting copy-fix. I didn't intentionally make a choice there. I've changed it to "optional" now because 1) This is something virtually no extension does today so it seems only right to begin there given that these are meant to reflect current practice, and 2) There is a related item about uninstall instructions under "Documentation" which is also optional already.
Added.
One thing that should be avoided even more than global state (e.g. global variables/public static properties in classes) is local static vars and similar instances of hidden local state, especially when used for caching. It looks like a very good idea when writing it, and is absolute nightmare when debugging/testing, because it creates hidden un-clearable state which could be in any random configuration when you get to it.
There is an other document in the works at User:DKinzler (WMF)/Software Design Practices that covers exactly this concern very well, I find.
From the name and (extremely minimal) lede, it sounds like this page is making recommendations for all extensions. However, from the very first recommendation, it's clear that the intended scope is only extensions that are meant to be used generally, and especially those aiming for WMF deployment. This disconnect could lead readers to believe that an extension being developed exclusively for use on e.g. a single third-party wiki is either explicitly discouraged, or has to jump through unexpected hoops that implicitly discourage development of such extensions. I'm sure that such extensions shouldn't be encouraged, but I think it's a mistake to actually discourage their creation/use, explicitly or otherwise.
tl;dr This page's lede should be expanded to clarify what sorts of extensions it's intended for: all extensions, or only those intended for use outside of their originating communities/wikis. (Possibly individual recommendations should receive similar clarifications.)
The document is intended for all extensions, but I think it is worth recognizing that while something is universally a best practice, it may not be applicable for specific extensions. For example, disabling the parser cache is not a good thing, but some extensions do it, and that's okay for how they're used.
The first recommendation is "Use the standard issue tracker". I don't understand why you think that's specific to WMF deployment. In general, having MediaWiki extensions track bugs in Phabricator and use Gerrit (eventually GitLab) for code review should be considered a best practice, given all the benefits from doing so.
Not all MediaWiki wikis are owned/operated by/as public, FLOSS, and/or not-for-profit. Accordingly, all MUSTs are at best OPTIONALs for basically obvious reasons n.b. it's not reasonable to expect a private company with likely different expectations of licensing (the polar opposite of those three values, so the other end of the spectrum) to comply with "publish on WMF Gerrit" and "use WMF Phabricator" and "use PSR-4" and "localize" and etc. For example, even two other organizations that are reasonably 'open' and in a topical sphere to WMF (Miraheze, Fandom) don't do every one of those MUST things.
So, I agree that the scope should be clear, as should be the expectations associated with not developing with the requirements, objectives, and best practice as documented here in mind.
This document isn't targeted towards private companies who want to make their own extensions. I think it is reasonable to have a baseline for *best practices* to use localization (one of MediaWiki's Principles), PSR-4 (a PHP best practice), and use Gerrit/Phabricator to be along with the rest of the MediaWiki ecosystem.
I don't think it's targeted at them, but it can trivially be read as if it were. Separately, because it leans on the RFC text as it does (I am increasingly not a fan in the way it changes the meaning of those words from the RFC), someone who does look at any given line with or without context will surely not see the forest from the trees when "MUST" is the word used.
This document should cater to professional engineers because the advice it gives is valuable for anyone developing an extension, not just the open source or trivially-reached MediaWiki ecosystems. That's besides the already-provided organizations which are trivially-reached MediaWiki ecosystems and which themselves have an interest in observing these practices, yet clearly will not and do not have an interest in e.g. using WMF Gerrit.
GOLD/SILVER/BRONZE, or BEST/BETTER/CONSIDER/RECOMMENDED, or similar other words would be better on that point. Remember, these are best practices.
+1 to MUST/SHOULD/OPTIONAL vocabulary not really being useful for general guidelines. You MUST use Phabricator and Gerrit and MUST put your classes in an src directory... or else what? Plenty of MediaWiki extension do not follow these rules; plenty of Wikimedia extensions do not follow many of the MUST rules, even. And I struggle to see what moral or legal authority anyone would have to make such declarations for plugins for a FLOSS software.
There are a few security-related points that we'd probably enforce by dissociating problematic extensions from the project or warning users against them, but everything else is a recommendation. Maybe rename it as good / great / best or something like that? (The API best practices guideline uses gold/platinum, it doesn't have a third level though.) Or preserve MUST for the few things where, if we had an extension store, we would kick an extension out of it if it violated that.
+1 to MUST/SHOULD/OPTIONAL vocabulary not really being useful for general guidelines. You MUST use Phabricator and Gerrit and MUST put your classes in an
srcdirectory... or else what?
… or else you are not compliant with this document, and we would not call your extension "best practice", as judged by the standards of the MediaWiki ecosystem.
Plenty of MediaWiki extension do not follow these rules; plenty of Wikimedia extensions do not follow many of the MUST rules, even. And I struggle to see what moral or legal authority anyone would have to make such declarations for plugins for a FLOSS software.
Indeed. Why do you think this page is claiming it has such authority?
I specifically didn't speak to the RFC language since Topic:Ugpw2635x8nzzp3z (second discussion from the bottom) is pertinent on that point.
Should there be something here about using OOUI when possible to create interface elements, instead of using the Html class or (*gasp*) hardcoded HTML?
By the way, the document does seem to say that the Html class should be used instead of hardcoded HTML, but maybe that part could be clearer.
Given that OOUI is now deprecated, I think we should avoid giving that guidance right now. But sadly the alternative (Codex) isn't ready and a server-side-rendering system certainly isn't. Ah well.
I didn't know OOUI was actually deprecated. Not to get into in a side issue, but is it really officially deprecated, or is it just sort of implicit that, given that a competing library is being developed, OOUI is not going to get much attention any more?
In any case, this doesn't seem like a satisfactory answer. If an extension's interface is going to adhere to the Wikimedia Design Style Guide (which is also not mentioned in this document, by the way), OOUI is still the easiest way to achieve that, as far as I know. Let me ask this: if a new extension were created today for use on Wikipedia or other Wikimedia sites, and it contained form elements, how would its UI be coded so that it matched the style guide? Or would it simply have the default HTML gray buttons? (Doubtful.)
Re: OOUI's deprecation, see this wikitech-l thread.
My understanding is that practically a new MediaWiki extension would most likely use OOUI because it has the most comprehensive widget library and is well integrated into everything...Codex isn't there yet AIUI. I think leaving that as a TODO or even a note on the page saying it's in flux would be okay.
Codex is currently technically vapourware (there's not even an alpha release), but I have faith that it'll be usable for JS-only interfaces in the next few months (disclaimer: my team are one of those trialling it).
Codex-with-SSR (which could be used for non-JS users and to avoid FOUC) is ultra-vapourware with no plan let alone code written. It'll be a while.
It's a good idea for extensions to regularly receive translations from translatewiki.net (not that we really have a choice in the matter), but what does it mean to regularly submit them?
I've added:
For extensions hosted in Wikimedia Gerrit, translatewiki.net staff will generally proactively do this for you. They start an automatic process which subscribes to new messages from your extension, and also automatically exports and merges updated translations back into your repository once a day. If this hasn't happened within a week, contact TWN staff.
I copied and summarised this from the translatewiki.net page, which was previously linked from the same bullet point.
In a way similar to protected methods, constructor signatures of service-like objects are generally not stable. They are intended for use by wiring code, and can be expected to change without notice. Extensions (and all application logic) should ideally only ever directly construct plain value objects. However, we still have quite a number of things in core that are neither service objects nor value objects. So I propose to add the following to the "Don't" section:
Sometimes you need a service with non-default configuration, most commonly when doing cross-wiki things. (Random recent example: )
As long as that is in core, it could be acceptable. Extensions should never do that.
The Right Way (tm) is to introduce a factory service that you can ask for a service instance for the target wiki.
Eventually (tm) we'd want a full MediaWikiServices instance for the target wiki. That will be possible as soon as no service uses global state.
We won't realistically introduce factories for all the services which depend on the wiki (which is basically all the services), MediaWikiServices doesn't support using foreign wiki configuration yet (even if it will, that will only cover wikis in the same farm, which is not the only use case), and expecting extension authors to make core changes instead of doing simple workarounds in their extensions is not realistic or reasonable IMO (especially as long as we don't get our code review problems under control).
> MediaWikiServices doesn't support using foreign wiki configuration yet
Actually, it does. But not all services are completely isolated from global state.
> expecting extension authors to make core changes instead of doing simple workarounds in their extensions is not realistic or reasonable IMO
These workarounds are what is preventing core changes, because we break them when we change core. I would expect extension authors to request or propose core changes when they need them, yes.
Reconsidering what I just said: extensions may have a need to call constructors of services if they extend the service class, or if the replace the service instance. That is, they may need to call service constructors in wiring code.
This is fine as long as the service is declared to be extensible - which implies that the constructor signature is treated as public and stable. Constructor signatures of other services should not be considered stable.
The rationale for this restriction is to support better dependency injection: with DI, there is often a need to change constructor signatures. Which is generally no problem since DI says that the constructor is only called in the wiring code, so there is only one place that needs to be changed to accommodate the new signature. Making such changes backwards compatible is a burden that should be avoided when possible - that is, we should only pay that cost in cases where the service is intended as an extension interface.
> Actually, it does.
Actually, it doesn't :) There is no way for me to obtain a DI container for a foreign wiki, short of constructing that container myself using a fake configuration object that I also construct myself, which is a lot more nasty than constructing a specific service manually using a specific wiki ID and language object.
> I would expect extension authors to request or propose core changes when they need them, yes.
That's a reasonable expectations for WMF staff maintaining extensions used in Wikimedia production. It's a completely unreasonable expectation for people running their own wikis, possibly in their free time, their bug reports get mostly ignored and their patches spend months if not forever in Gerrit without anyone ever looking at them. The MediaWiki community just does not provide the level of volunteer support currently where we could in good conscience ask people to have their problems fixed in core.
More specifically to the foreign wiki issue, what would the core change be? Do we really want to create a factory class for everything that takes a wiki ID, when those classes won't be necessary once MediaWikiServices supports foreign wikis?
> There is no way for me to obtain a DI container for a foreign wiki, short of constructing that container myself using a fake configuration object
Right - the problem is that there is no clean way to get the config settings for another wiki. For this to be complete, it would have to include the enabled extensions as well, so we'll need injecteble hook runners.
> It's a completely unreasonable expectation for people running their own wikis, possibly in their free time, their bug reports get mostly ignored and their patches spend months if not forever in Gerrit without anyone ever looking at them.
This policy doesn't apply to people writing their own extension for their own use. But yes, their extension might break without warning.
> More specifically to the foreign wiki issue, what would the core change be? Do we really want to create a factory class for everything that takes a wiki ID, when those classes won't be necessary once MediaWikiServices supports foreign wikis?
I'm not particularly keen on doing that, but the code is trivial enough. We could have a helper class or a trait for the boiler plate, even.
This page is definitely meant for people writing their own extensions, so that they end up with maintainable code that others can reuse. Although I guess it would make sense to treat it as a policy (which it currently isn't) for Wikimedia-deployed extensions and merely as guidance for others.
You are right, of course - it is meant for them, but thy are not bound by it. Sorry for being imprecise.
The point is, don't add MUST NOTs that are not reasonable expectations for most of the intended audience (even if they are not bound by it).
I think they are reasonable expectations. The only way to get out of the we-can't-change-core-without-breaking-extensions dead end is to limit what extensions can do (or what they can expect to continue working with the next release).
But you are right about my original proposal being too restrictive. It should be more along the lines of:
Another way of phrasing this would be
The second option is not less restrictive, but perhaps more to the point.
Done, we now recommend PSR4 and a "/src" directory.
See comment at Manual talk:Coding conventions/PHP#Directory Structure. I agree; we should follow standard PHP naming conventions and use /src for at least PSR-4 extensions.
Done, per Stable interface policy.
I propose to add the following to the "Don't" section:
The reason for this is that protected methods are not part of our stable interface per the Deprecation policy, so such subclasses may break without warning. The relevant section of the deprecation policy reads:
[...] the stable part of the PHP API is comprised of all code that is explicitly marked public, and has been included in at least one stable release. In addition, some classes expect to be subclassed in extensions; in those cases protected functions also are included in the API. These classes should have a note in their documentation comment that they expect subclassing. If no note is present, it SHOULD be assumed that the class is not expected to be subclassed.
I'm tempted to even extend this to implementing interfaces. Not all interface declarations are intended as extension points, and not all of them should be considered stable. If we go that far, we should also add:
+1 to both suggestions from my side.
I, personally, find it a really helpful rule-of-thumb to consider all classes to be final, except otherwise stated. The effect of this restriction is pretty much the same as you suggest: one can not extend anything, except it is allowed. The only reason we are not literally marking all our code as final is that it would be cumbersome and error-prone. But we should still threat it like it is.
