Page MenuHomePhabricator

JSDoc of mw.notification.notify is incorrect
Open, Needs TriagePublic

Description

https://doc.wikimedia.org/mediawiki-core/master/js/mw.notification.html#.notify

returns "Notification". This gets linked to the webapi https://developer.mozilla.org/en-US/docs/Web/API/Notification

It is however the internal Notification object: https://doc.wikimedia.org/mediawiki-core/master/js/mediawiki.notification_notification.js.html#line25

which is hidden from jsdoc because "The constructor is not publicly accessible; use [mw.notification.notify]{@link mw.notification} instead."

It's strange that we hide an internal object, that is part of the public API, and link to incorrect unrelated documentation at the same time.

Event Timeline

apaskulin subscribed.

which is hidden from jsdoc because "The constructor is not publicly accessible; use [mw.notification.notify]{@link mw.notification} instead."

It looks like the reason that this is being hidden from JSDoc is because the theme's externals plugin is overriding the creation of the Notification page with the Mozilla link. If I disable the plugin locally, JSDoc generates a Notification page and links to it from mw.notification. See https://github.com/wikimedia/jsdoc-wmf-theme/blob/master/src/defaultMaps.js#L100-L104 and https://github.com/wikimedia/jsdoc-wmf-theme/blob/master/plugins/externals.js.

The quickest fix seems to be to add "Notification": "Notification.html" to the linkMap in the core JSDoc config. This fixes the issue for me locally. Updated: This approach doesn't actually work.

The quickest fix seems to be to change Notification to mw.Notification and remove the @global tag from its doc block. This allows an mw.Notification page to be created and linked to. I assume this would be ok since this was the name of the class in JSDuck. Note that in order to see the type details, we would also need to remove the @hideconstructor tag or create a separate doc block for the type.

A more robust fix would be to modify the externals plugin to treat false values in the linkMap the same way it does for the prefixMap so we could instead fix the issue by adding "Notification": false in the core JSDoc config. I haven't tested this locally yet.

I'm in favor of implementing the quick fix now and opening a task for the more robust fix. Thanks for catching this, @TheDJ!

Change #1127535 had a related patch set uploaded (by Alex Paskulin; author: Alex Paskulin):

[mediawiki/core@master] docs: Fix link to mw.Notification in JSDoc

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

Test wiki created on Patch demo by APaskulin (WMF) using patch(es) linked to this task:
http://patchdemo.wmcloud.org/wikis/7aab404007/w/

Change #1127535 abandoned by Alex Paskulin:

[mediawiki/core@master] docs: Fix link to mw.Notification in JSDoc

Reason:

Thanks for the review, TheDJ. Abandoning since this approach doesn't work

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

Change #1136017 had a related patch set uploaded (by Alex Paskulin; author: Alex Paskulin):

[mediawiki/core@master] docs: Make internal Notification object visible on docs site

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

Test wiki created on Patch demo by APaskulin (WMF) using patch(es) linked to this task:
https://patchdemo.wmcloud.org/wikis/5abcdf2b99/w/

The quickest fix seems to be to change Notification to mw.Notification and remove the @global tag from its doc block.

I experimented with this approach, but it caused the namespace page to move from .../js/mw.notification.html to .../js/mw.notification_.html with an extra underscore. Everything within the site seemed to link to the correct pages still, but I thought this was likely create confusion in some way.

Instead, I submitted an experimental patch that creates a typedef on the namespace page. This fulfills the original issue of the task without creating confusion on the site between a mw.Notification class page and an mw.notification namespace page. Although I'm not sure which approach is better. Thoughts welcome!

Test wiki on Patch demo by APaskulin (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmcloud.org/wikis/5abcdf2b99/w/

Test wiki on Patch demo by APaskulin (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmcloud.org/wikis/7aab404007/w/

Change #1136017 merged by jenkins-bot:

[mediawiki/core@master] docs: Make internal Notification object visible on docs site

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

matmarex assigned this task to apaskulin.
matmarex subscribed.

Looks fixed

The rest of the Notification class docs are still missing (classdesc, start, pause, resume, close) from the web page.

And the search index there is still suggesting and promoting these methods as existant, but linked off to an unrelated MDN page.

Screenshot 2025-06-04 at 14.23.18.png (1×2 px, 486 KB)

The new typedef name is separate from the class, and now has one fixed link pointing at it.

Could we rename the class itself to take that new name (at least from a JSDoc point of view, that is). That should fix it all, I think.

Novem_Linguae subscribed.

Not sure if Krinkle's suggestion should be handled in this task or a new task. Will re-open this one for now to help draw attention.

Change #1167283 had a related patch set uploaded (by Alex Paskulin; author: Alex Paskulin):

[mediawiki/core@master] docs: Surface missing Notification class in JSDoc

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

Test wiki created on Patch demo by APaskulin (WMF) using patch(es) linked to this task:
https://patchdemo.wmcloud.org/wikis/cecc8736f4/w/

I looked into this a bit today and submitted a WIP patch that renames the Notification class to mw.notification~Notification. In order to do this, I needed to remove the @global tag from the Notification class. This creates a new page (patchdemo) for the class nested within the mw.notification module that includes:

  • the classdesc
  • the pause, resume, close methods
  • the typedef, which is now removed from the main mw.notification page and must now be referenced as mw.notification~Notification.Notification to avoid linking out the MDN page

I did not include the start method since it is marked as private.

This removes all incorrect links to the MDN page in the search and the sitemap. Overall, there are a lot of different ways we can present this information in JSDoc. I don't have a strong opinion, so happy to try and work out whatever organization folks think is best.