Page MenuHomePhabricator

Switch Hogan for Mustache
Closed, ResolvedPublic5 Story Points

Description

We will remove Hogan from MobileFrontend and make sure of Mustache in core. Mustache is smaller, uses the same syntax and available in MediaWiki. This will make it easier for template users working across mobile and desktop environments.

Acceptance criteria

  • A dependency is added to mediawiki.template.hogan to mediawiki.template.mustache
  • For compatibility with Minerva which cannot use inline templates (right now) , to avoid loading both Mustache and Hogan and to avoid this being a breaking change the Hogan compiler should remain but look like this:
// register hogan compiler with core
mw.template.registerCompiler( 'hogan', mw.template.getCompiler('mustache') );
  • Any webpack dependencies relating to Hogan are removed.
  • A deprecated flag is added to mediawiki.template.hogan to ensure existing users move to mediawiki.template.mustache

QA steps

Switching Hogan to Mustache will have implications on Minerva, which will still be using templates for rendering page issues, main menu, notifications and the back to top feature

  • In beta ensure the back to top feature is still rendering and matches production when opted into BETA (Android only)
  • Make sure the main menu rendering matches production
  • Make sure the page issues overlay rendering matches production. Use https://en.m.wikipedia.beta.wmflabs.org/wiki/Pharmacovigilance for testing this.
  • Make sure the notification badge (top right corner in LTR mode) is still rendering and matches production.

A general QA of the entire site (either by developer and/or test engineer) will also be needed to check different flows are rendering easily. (This would be much more straightforward if we had a Storybook)

Sign off steps

  • Record this on release timeline
  • Open a task to update Minerva to use mediawiki.template.mustache (already done)
  • Open a task to remove the mediawiki.template.hogan module completely (patch merged 3 weeks after, no task necessary)

QA Results

Event Timeline

Jdlrobson triaged this task as High priority.Apr 10 2019, 3:38 PM
Jdlrobson created this task.
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Apr 10 2019, 3:42 PM
Jdlrobson moved this task from Incoming to Upcoming on the Readers-Web-Backlog board.
Jdlrobson updated the task description. (Show Details)Apr 10 2019, 4:24 PM
Krinkle removed a subscriber: Krinkle.Apr 17 2019, 6:26 PM

Change 506337 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Swap Hogan for Mustache

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

Change 506338 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Minerva is Hogan free

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

I downloaded both patches, on pretty clean MediaWiki instance I immediately get warning Use of "compile" is deprecated. Hogan compiler renders templates as Mustache. Use the mustache compiler directly and confirm your template is compatible.

I downloaded both patches, on pretty clean MediaWiki instance I immediately get warning Use of "compile" is deprecated. Hogan compiler renders templates as Mustache. Use the mustache compiler directly and confirm your template is compatible.

Fixed in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/506337/5..6/src/mediawiki.template.hogan/mediawiki.template.hogan.js,unified

As discussed this is dead code (and removed in https://gerrit.wikimedia.org/r/507621). Not blocking this change.

Change 506337 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Swap Hogan for Mustache

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

Change 506338 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Minerva is Hogan free

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

Jdlrobson reassigned this task from Jdlrobson to Edtadros.May 1 2019, 9:51 PM
Jdlrobson added a subscriber: alexhollender.

Please test on https://en.m.wikipedia.beta.wmflabs.org - specifically we're looking for any UI regressions compared with production (cc @alexhollender )

@Jdlrobson I think we've discussed potential issues with such broad QA asks like this in the past. Is there any way to provide some clues as to what might be off as a result of this change?

@Jdlrobson I think we've discussed potential issues with such broad QA asks like this in the past. Is there any way to provide some clues as to what might be off as a result of this change?

I think the risk of any issue is low and if there is an issue it should be obvious and visual (A made-up example might be "when I clicking search and search, the search results are not displaying!"). Templates are used in all our JavaScript based experiences, so we should test all workflows such as editing, viewing languages, talk, searching and checking they still work (which I've done myself). That should be sufficient. The browser tests have also already given us a strong indication that things are working.

Apologies for the delay here, not sure if this is still needed but everything looks good on both iOS and Android.

In beta ensure the back to top feature is still rendering and matches production
Make sure the main menu rendering matches production
Make sure the page issues overlay rendering matches production
Make sure the notification badge (top right corner in LTR mode) is still rendering and matches production.
alexhollender updated the task description. (Show Details)May 6 2019, 7:01 PM
Edtadros reassigned this task from Edtadros to alexhollender.EditedMay 8 2019, 9:36 AM
Edtadros added a subscriber: Edtadros.

Test Result

Status: Not Complete
OS: macOS Mojave
Browser: Chrome
Device: BrowserStack - Galaxy S5

Test Artifact(s):

❓ AC1: In beta ensure the back to top feature is still rendering and matches production (Android only)
@alexhollender I was able to get the back to top overlay in beta but not in production.

✅ AC2: Make sure the main menu rendering matches production


❓ AC3: Make sure the page issues overlay rendering matches production
@alexhollender how do I get the page issues overlay to render?

✅ AC4: Make sure the notification badge (top right corner in LTR mode) is still rendering and matches production.

Jdlrobson updated the task description. (Show Details)

I think the edits to the description in https://phabricator.wikimedia.org/transactions/detail/PHID-XACT-TASK-ccksmfyydwihpcu/ should help with the remaining 2. IM me if not!

Edtadros reassigned this task from Edtadros to ovasileva.EditedMay 8 2019, 11:11 PM

@Jdlrobson, that helped. And neat phab link!

Test Result

Status: ✅ PASS
OS: macOS Mojave
Browser: Chrome
Device: BrowserStack - Galaxy S5

Test Artifact(s):

✅ AC1: In beta ensure the back to top feature is still rendering and matches production (Android only)


✅ AC3: Make sure the page issues overlay rendering matches production

Krinkle removed a subscriber: Krinkle.May 8 2019, 11:13 PM
Edtadros updated the task description. (Show Details)May 8 2019, 11:14 PM
Jdlrobson closed this task as Resolved.May 14 2019, 12:55 PM

Hogan is no more 🦑🦑🦑🐳🎉🎉🎉

Jdlrobson updated the task description. (Show Details)May 14 2019, 2:14 PM