Page MenuHomePhabricator

MobileFrontend should not use moment.js for a single line
Open, Needs TriagePublic


Moment.js is quite a large library (70K gzipped) and is used only in a single line (though, loaded asynchronously).

Event Timeline

simon04 created this task.May 29 2019, 10:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 29 2019, 10:36 PM

Change 513233 had a related patch set uploaded (by Simon04; owner: Simon04):
[mediawiki/extensions/MobileFrontend@master] Replace moment.js with Date.toLocaleString

Jdlrobson added a subscriber: Jdlrobson.

There is a problem with the patch as written. I think this should be the editing teams call as I'd hope long term that all editor code would live inside VisualEditor or one of their projects and I'm not sure whether moment.js is part of that. I don't think this is Reading-Web's domain, but let me know if it should be.

@Jdlrobson after checking with the team the Editing-team is responsible for this task, however, it isn't high on priority list at this time.

Moment is mostly needed to display "relative" times, as noted on the patch by Jon:

My understanding is that toLocaleString won't work here as moment is generating messages such as "a few seconds". So really this becomes a question of how useful that is, and whether it's worth the bloat that comes with moment.js

Personally I wouldn't mind removing the relative time, and only leaving the absolute expiration date. On desktop only the absolute expiration date is shown.

Historical note: the current code using Moment was added by me in rEMFR7b64322a2fdc: Add fancy block info popups from Minerva to MobileFrontend, replacing earlier implementation by @dbarratt in rSMINf51cf7db7b01: Use a Drawer for Block Notices that formatted dates in PHP code.

We only used them for registered users, but MobileFrontend's EditorOverlay code can use them for anonymous users as well.

(…) In Minerva we did this in PHP code and put it in mw.config(), but we can't do it for anonymous users, because that ends up in page HTML and is cached.

Formatting expiry time and duration is done using Moment.js rather than the Language class methods and custom code. (…)

Also, we could go back to rendering the relative expiry time in PHP – this could happen in ApiBlockInfoTrait (in MW core), which should allow it to be returned by API queries used by both editors (visual and wikitext). This didn't exist when David's original version was written (and VE didn't display block notices back then, now it does).

Change 513233 abandoned by Jdlrobson:
Replace moment.js with Date.toLocaleString

Please restore once we've got consensus on the right approach on the ticket. Thanks!