Topic on Talk:Best practices for extensions/Flow

See Stable interface policy.

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:

• MUST NOT: directly instantiate a service class defined by core, instead of obtaining an instance via a service container.
• SHOULD NOT: directly instantiate anything that is not a plain value object.

Sounds good.

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:

• MUST NOT: directly instantiate a service class defined by core, instead of obtaining an instance via a service container, except when wrapping, replacing or defining a service that is documented to be an extension point.

Another way of phrasing this would be

• MUST NOT rely on the stability of a constructor signature of a class that is not defined to be a pure value object, or documented to be an extension point suitable for subclassing or direct construction by extensions.

The second option is not less restrictive, but perhaps more to the point.

I believe this is now covered by Stable interface policy.