Change 981592 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/Wikibase@master] TemplateModule: Don't use mw.Map(), use a native Map() instead

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

Change 981594 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/PageTriage@master] util.main: Don't use mw.Map(), use a native Map() instead

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

Change 981602 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/core@master] mediawiki.js: Deprecate mw.Map(), move uses to native Map() instead

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

Change 981594 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] util.main: Don't use mw.Map(), use a native Map() instead

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

We should probably also add in tests to ensure that anything using mw.Map works with normal JS Maps. mw.Message apparently can't handle normal JS Maps, so that ended up breaking PageTriage (see T353571 for details).

I think we should be slightly slower with this deprecation, we should add a .has(...) method to mw.Map convert all functions to support both mw.Map and native Maps, wait for gadgets, userscripts and core code to shift to using .has(...) before pulling the rug and changing everything to only use native Maps

Change 983528 had a related patch set uploaded (by Chlod Alejandro; author: Chlod Alejandro):

[mediawiki/extensions/PageTriage@master] Revert "util.main: Don't use mw.Map(), use a native Map() instead"

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

Change 983518 had a related patch set uploaded (by Chlod Alejandro; author: Chlod Alejandro):

[mediawiki/core@master] [WIP] Add .has alias for mw.Map#exists, migrate uses

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

I thought mw.Map probably isn't covered by the frontend stable interface policy since it's a private class, but I realized that mw.messages, mw.user.options, and mw.user.tokens all use mw.Map. When the time comes that we shift those to using native JavaScript Maps, we will break a lot of gadgets which call .exists. We should tread carefully when executing this deprecation.

Per my comments on T353571 and the echo by Sohom above, I've filed a possible solution to avoid breaking things: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/983518. This renames .exists to .has, keeping a (deprecated) alias at .exists to prevent breaking current uses. The hope here is that anyone using .exists will migrate to .has right now, and then mw.messages, mw.user.options, and mw.user.tokens can be switched over to native JavaScript Maps in a breaking change. This makes the transition seamless, since eventually all uses call a function that also exists in native JavaScript Map, and it means no breaking happens to gadgets at any point (unless they're unmaintained and failed to transition).

Note that there's two issues with this patch (which is why I marked it as WIP):

• There's currently no change to RELEASE-NOTES as I'm not experienced enough in hacking core to know where in RELEASE-NOTES to place this; and
• I could not add an mw.log.deprecate to .exists because (theoretically) mw.log.deprecate may not have been loaded by the time a Map is instantiated and .exists is called. mw.Map is instantiated in startup/mediawiki.js, mw.log.deprecate in mediawiki.base/log.js. Don't want to risk calling mw.log.deprecate within this function, nor do I think adding an if ( mw.log.deprecate ) guard is a good solution.

Hopefully someone more experienced in hacking core can advise what to do here, or pick up/supersede this patch.

Change 983528 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Revert "util.main: Don't use mw.Map(), use a native Map() instead"

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

Change 983529 had a related patch set uploaded (by Chlod Alejandro; author: Chlod Alejandro):

[mediawiki/extensions/PageTriage@wmf/1.42.0-wmf.9] Revert "util.main: Don't use mw.Map(), use a native Map() instead"

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

Change 983529 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@wmf/1.42.0-wmf.9] Revert "util.main: Don't use mw.Map(), use a native Map() instead"

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

Mentioned in SAL (#wikimedia-operations) [2023-12-18T14:53:54Z] <urbanecm@deploy2002> Started scap: Backport for [[gerrit:983229|Configure and enable StatsLib for production (T343024)]], [[gerrit:983529|Revert "util.main: Don't use mw.Map(), use a native Map() instead" (T353571 T353076)]]

Mentioned in SAL (#wikimedia-operations) [2023-12-18T14:55:12Z] <urbanecm@deploy2002> cwhite and urbanecm and chlod: Backport for [[gerrit:983229|Configure and enable StatsLib for production (T343024)]], [[gerrit:983529|Revert "util.main: Don't use mw.Map(), use a native Map() instead" (T353571 T353076)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2023-12-18T15:04:19Z] <urbanecm@deploy2002> Finished scap: Backport for [[gerrit:983229|Configure and enable StatsLib for production (T343024)]], [[gerrit:983529|Revert "util.main: Don't use mw.Map(), use a native Map() instead" (T353571 T353076)]] (duration: 10m 20s)

I thought mw.Map probably isn't covered by the frontend stable interface policy since it's a private class, but I realized that mw.messages, mw.user.options, and mw.user.tokens all use mw.Map. When the time comes that we shift those to using native JavaScript Maps, we will break a lot of gadgets which call .exists. We should tread carefully when executing this deprecation.

mw.Map itself is private – or at least it used to be private until the JSDoc migration in https://gerrit.wikimedia.org/r/q/Ia709de48cbc0f00ff832eb4666e6abcffdb6f710 where it seems to have (inadvertently?) lost its @private annotation, cc @Jdlrobson.

For mw.messages and mw.user.{options,tokens} which are indeed stable interfaces, we should be better off temporarily monkey-patching them so that each supports .has. In terms of JS cost, it should be a net positive as the three shims are together smaller than the mw.Map implementation.

Change 983518 had a related patch set uploaded (by Chlod Alejandro; author: Chlod Alejandro):
[mediawiki/core@master] [WIP] Add .has alias for mw.Map#exists, migrate uses
https://gerrit.wikimedia.org/r/983518

I doubt if this is a good idea – mw.Map is part of the startup module which has a short cache TTL. We should try to reduce its size, not increase it by adding a shim. It would take forever for gadgets and scripts to migrate.

In T353076#9425326, @SD0001 wrote:

For mw.messages and mw.user.{options,tokens} which are indeed stable interfaces, we should be better off temporarily monkey-patching them so that each supports .has. In terms of JS cost, it should be a net positive as the three shims are together smaller than the mw.Map implementation.

Yeah, that seems like a sounder solution.

Change 983518 abandoned by Chlod Alejandro:

[mediawiki/core@master] [WIP] Add .has alias for mw.Map#exists, migrate uses

Reason:

per task

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

In T353076#9425326, @SD0001 wrote:

I thought mw.Map probably isn't covered by the frontend stable interface policy since it's a private class, but I realized that mw.messages, mw.user.options, and mw.user.tokens all use mw.Map. When the time comes that we shift those to using native JavaScript Maps, we will break a lot of gadgets which call .exists. We should tread carefully when executing this deprecation.

mw.Map itself is private – or at least it used to be private until the JSDoc migration in https://gerrit.wikimedia.org/r/q/Ia709de48cbc0f00ff832eb4666e6abcffdb6f710 where it seems to have (inadvertently?) lost its @private annotation, cc @Jdlrobson.

There was some ambiguity here. My read of the documentation and usage (https://codesearch.wmcloud.org/deployed/?q=new+mw.Map) was Map itself was private but it was exposed publicly as mw.Map (with no tag specifying it to be private or note explaining for what purpose/access). The frontend policy says "Code that is defined on the public mw object, that has not been documented as @private, regardless of how the code is added (extension, skin or wiki-based code), even if the code has not been marked as @stable." so in this case you could make arguments either way.

I leaned towards it being considered @stable as it is used by quite a few modules outside the class (https://codesearch.wmcloud.org/deployed/?q=mw.Map) and indirectly via mw.messages, mw.user.options, and mw.user.tokens. I think it would make sense to cover it under the frontend stable policy with this in mind.

I have no issues with reverting mw.Map itself back to @private with a deprecation notice if anyone has a strong opinion but it sounds like we'd need to go through the deprecation process either way.

Regardless I support the deprecation of mw.Map in favor of a native ES6 solution :-) and would suggest that as the impetus of any change here!

In T353076#9433650, @Jdlrobson wrote:

In T353076#9425326, @SD0001 wrote:

mw.Map itself is private – or at least it used to be private until the JSDoc migration in https://gerrit.wikimedia.org/r/q/Ia709de48cbc0f00ff832eb4666e6abcffdb6f710 where it seems to have (inadvertently?) lost its @private annotation, cc @Jdlrobson.

There was some ambiguity here. My read of the documentation and usage (https://codesearch.wmcloud.org/deployed/?q=new+mw.Map) was Map itself was private but it was exposed publicly as mw.Map (with no tag specifying it to be private or note explaining for what purpose/access).

I don't think this is the right reading; my understanding was that it was private and an internal-only class that was exposed back when there were no expectations of stability.

The frontend policy says "Code that is defined on the public mw object, that has not been documented as @private, regardless of how the code is added (extension, skin or wiki-based code), even if the code has not been marked as @stable." so in this case you could make arguments either way.

Yes, but that policy is a decade newer than the code. :-)

I have no issues with reverting mw.Map itself back to @private with a deprecation notice if anyone has a strong opinion but it sounds like we'd need to go through the deprecation process either way.

Why would we need to deprecate it, when it was previously marked as private until your documentation update?

Why would we need to deprecate it, when it was previously marked as private until your documentation update?

I guess we don't need to follow the deprecation process provided we update the existing use cases in extensions and mediawiki.language to use Map instead of mw.Map first and send a User-notice (since it's being used in a few gadgets).

I think if we don't intend this to be private we should definitely not be exposing it on the mw object. There's no need and misleads developers into thinking it's a stable API.

Change 998475 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Don't use mw.Map(), use a native Map() instead

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

Change 998475 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Don't use mw.Map(), use a native Map() instead

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

Thanks @simon04 for taking care of MediaViewer code!

(Adjusting the Wikibase tags: MediaWiki-extensions-WikibaseClient should more or less only be used for Wikibase related things on Wikimedia wikis except Wikidata, or for code under the client/ subdirectory in the Wikibase extension.)

mw.Map is an ES3-compatible re-implementation of Map from ES6.

It is not. The naming similarity between mw.Map and ES6 Map is a coincidence and predates our awareness of ES6 by many years. The two are not related to each other.

In the past we did ship inlined ES6 polyfills and we could have (and still can) implement one atop the other. The reason mw.Map is written in a few lines of plain JavaScript, is that mw.Map became larger, slower, and more complicated when I tried to write it another way. We did simplify the code over the years by adopting modern ES5 Object.create( null );, for-in and Array.isArray. If there's ways to simplify the implementation further, patches are welcome! Note that it isn't a general-purpose public API, rather it exists primarily as backend for the public mw.config and mw.user.options APIs. Last I tried, I found trying to put an ES6 Map underneath those, did not help with size, speed, or simplicity. You're welcome to try, but it won't be a full drop-in replacement. It'd be a lower-level patch.

Some widely-used mw.config of features are are lacking in ES6 Map:

• mw.Map#get() returns an object of all values.
• mw.Map#get(key, default) takes a default parameter, widely used, e.g. mw.config.get( 'wgUserGroups', [] ).
• mw.Map#get(keys) takes an array keys parameter for returning a subset of keys, widely used, e.g. mw.config.get( [ 'wgTitle', 'wgNamespaceId' ]).
• mw.Map#set(values) takes an Object values parameter to set multiple key-value pairs in one go, or two separate key-value arguments, widely used.
• mw.Map#exists(key) is a boolean method for checking key existence.

The coincidental overlap with ES6 starts and stops at .get(key) and .set(key, value).

moving from wikidata's active work board to backlog

Change #981602 abandoned by Bartosz Dziewoński:

[mediawiki/core@master] mediawiki.js: Deprecate mw.Map(), move uses to native Map() instead

Reason:

Task was declined

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