Page MenuHomePhabricator

WMDE: Review stable code (hooks, mw object, undocumented) and mark (un)stable interfaces before 1.42.0
Closed, ResolvedPublic

Description

The stable interface policy was introduced in September 2023 during the 1.41 MediaWiki release. As a result of the introduction of this policy I am asking teams to take the opportunity to review their existing stable APIs and take appropriate action to limit unsupported APIs where necessary and possible.

TODO

  • Please read the guidelines around what constitutes stable code
  • Please remove any functions on the mw object that you do not wish to consider stable. Note "Code that was never part of a public MediaWiki release, and never consumed according to Codesearch and Global Search MAY be changed (including marked explicitly as @private) or removed without deprecation, since the code has never become part of the stable interface or used."
  • Please add @private or @internal documentation tags to hook events that you wish to limit usage of.

Event Timeline

thiemowmde subscribed.

I'm afraid I don't understand how the WMDE-TechWish products are affected. I had a brief look at the codebases but couldn't spot any suspicious code. Please provide e.g. example code snippets or a CodeSearch link.

@thiemowmde here are some examples of stable code that is not clearly documented (this is not a complete list):

From glance I don't think Wikidata adds any code to mw object, but if it does that should also be evaluated and documented appropriately.

@thiemowmde to clarify the untagging of RevisionSlider and AdvancedSearch - have those been checked or are you saying WMDE is not maintaining them (in which case please update https://mediawiki.org/wiki/Maintainers and let me know so I can find the appropriate maintainers?)

The ticket claims to be about "code on mw object". But the WMDE-TechWish products I'm responsible for don't look like they have code that matches this description. The Wikibase examples also don't match the description (as you said yourself) but use the hook system as documented. What's wrong with that? What's the problem this ticket aims to solve?

Jdlrobson renamed this task from WMDE: Review code on mw object and mark (un)stable interfaces before 1.42.0 to WMDE: Review code on mw object and hooks contract and mark (un)stable interfaces before 1.42.0.Oct 4 2023, 4:41 PM
Jdlrobson renamed this task from WMDE: Review code on mw object and hooks contract and mark (un)stable interfaces before 1.42.0 to WMDE: Review stable code (hooks, mw object, undocumented) and mark (un)stable interfaces before 1.42.0.EditedOct 4 2023, 4:45 PM

@thiemowmde could you answer my question around RevisionSlider and AdvancedSearch?

@thiemowmde I've updated the title to hopefully be less confusing, per the stable policy any hooks are stable:

> The function signature of the callback function for hooks fired using the mw.hook object in MediaWiki core code SHOULD be considered stable unless marked as @private or @internal. Consumers can expect that the hook will be fired in the circumstances described in its documentation.

This task gives you the opportunity to decide if you intended existing hooks to be stable since they were written before the policy was introduced. If you are fine with all your existing mw.hook(..).fire calls being considered stable contracts you can resolve this ticket without any changes.

ItamarWMDE subscribed.

As a side note the tag Wikidata-Campsite is no longer actively maintained, please add Wikidata Dev Team for any issues regarding the live Wikidata Instance or the Wikibase Extensions (WikibaseLexeme, EntitySchema, WikibaseClient, etc.). If WikibaseRepo is involved, please also tag Wikibase Product Platform Team WPP for now as well and we can resolve how to tackle the work together.

FYI, the 1.42 branch point is in a matter of hours; it doesn't appear that this will be done by then?

I've clearly miscalculated remaining time. I intended to do it this week. In a matter of hours will not happen indeed, no.

I still try to understand what needs to be done here? I can see that mw:Stable interface policy/Frontend talks about hooks, and a few WMDE products happen to contain mw.hook calls. However, …

  • Does mw:Stable interface policy/Frontend even apply to extensions the same way it applies to core? I mean, aren't extensions the consumers of the stable interfaces MediaWiki core provides? Most policy documents phrase it this way. mw:Development policy for example explicitly says "changes to MediaWiki core […] must follow the Stable interface policy." While I understand that some central extensions effectively behave as if they are part of "the core experience on the Wikimedia cluster", I don't think that applies to everything.
  • It feels like it makes a lot of sense to consider all code in an extension unstable by default, unless otherwise noted. In case this is not backed by the current documents, what's the best way to state that in the documentation of the extension? Is a sentence in the top-level README enough?
  • I would love to follow best-practices, but have a hard time finding real-world examples in other, typically well maintained products like VisualEditor and DiscussionTools. I mean, if such central codebases don't follow the policy, how can we expect other teams to do so?

Furthermore, some of the code examples on mw:Stable interface policy/Frontend appear to call mw.hook.fire() and mw.hook.add(), but code like this doesn't exist. Is this an obsolete signature or a mistake?

Does mw:Stable interface policy/Frontend even apply to extensions the same way it applies to core

Yes. Anything on the mw object is subject to the deprecation policy. The reason for this is that it is not fair to expect gadget developers to understand where code comes from, just how to access stable code (via the mw object). Likewise, any hook.fire sets up a stable interface.

For example mw.wikibase can be considered stable as of the 1.42 cut (which was the deadline we set) and mw.hook( 'wikibase.entityPage.entityLoaded' ) is also stable.

In case this is not backed by the current documents, what's the best way to state that in the documentation of the extension? Is a sentence in the top-level README enough?

You can add the @internal tag to the doc block of any public method to opt out per https://www.mediawiki.org/w/index.php?title=Stable_interface_policy/Frontend#JavaScript_code or move the code off the mw object, as I did here: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/987459

If this is not acceptable please comment in the next 7 days so we have a record of the intent to change this, otherwise we can decline this ticket.

Note: If you do need to fix these APIs to not be stable, please make sure to backport any applicable patches to 1.42 branch.

Change #1019899 had a related patch set uploaded (by WMDE-leszek; author: WMDE-leszek):

[mediawiki/extensions/Wikibase@master] Declare mw.wbTemplate and mw.wbTemplates as not stable

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

Change #1019900 had a related patch set uploaded (by WMDE-leszek; author: WMDE-leszek):

[mediawiki/extensions/Wikibase@master] Mark Wikibase hooks as not stable

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

Change #1019900 abandoned by WMDE-leszek:

[mediawiki/extensions/Wikibase@master] Mark Wikibase hooks as not stable

Reason:

would have been a wrong and unwanted change

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

hello @Jdlrobson a random and likely silly question. It seems https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/1019899 might do it/most of it, as wikibase puts most of its frontend stuff to own "global object" not to mw.
Wikibase JS code does use quite a few "options" transferred from the backend via mw.config - I am not sure how it to interpret the policy in this regard. Should we actively mark wikibase's options that could be found in mw.config as options that gadget authors should not rely on? What'd be your advice on this?

The frontend stable policy currently doesn't cover mw.config values. We already have cases where config variables are not always available e.g. on mobile wgCategories is notably absent, so I think code should be providing fallbacks or error handling when those don't exist. e.g. mw.config.get('wgCategories', [] ) or if (mw.config.get('wgCategories') === null ) return;

I think this would be a good thing to clarify in the policy - so would encourage you to propose text via the talk page (https://www.mediawiki.org/wiki/Talk:Stable_interface_policy/Frontend) that makes sense for your team relating to the lack of stability of config variables to make this clearer!

Change #1019899 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Declare mw.wbTemplate and mw.wbTemplates as not stable

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

Change #1021788 had a related patch set uploaded (by WMDE-leszek; author: WMDE-leszek):

[mediawiki/extensions/Wikibase@REL1_42] Declare mw.wbTemplate and mw.wbTemplates as not stable

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

Change #1021788 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@REL1_42] Declare mw.wbTemplate and mw.wbTemplates as not stable

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

It looks like the task, admittedly way over time, has been sorted from the Wikibase side.
I'll let @thiemowmde assess whether it needs to remain open for other WMDE maintained extensions, or whether it is all sorted and can be closed by now.

Advanced-Search is the only WMDE-TechWish product that contains client-side mw.hook hooks. These have always been documented as public and stable. We also added @stable tags to the code now, see https://gerrit.wikimedia.org/r/1021410.

We consider this done.