Page MenuHomePhabricator
Create Task
Maniphest T323542

Forbid new modern code from not targeting mobile
Closed, DeclinedPublic
Actions

Description

The targets system was initially added to reassess code before loading on mobile. It has stayed much longer than expected.

As we embrace Vue and ES6 code I was surprised to see certain code is unintentionally (and intentionally) not being shipped to mobile clients.

We should stop this before it gets out of hand.

To avoid this I recommend:

  • we should change the targets behaviour so that it defaults to [ 'desktop', 'mobile' ] for any code that uses ES6
  • or packageFiles.
  • we will log a warning for any code that intentionally tries to avoid targeting mobile in these circumstances

Details

Other Assignee
Krinkle
SubjectRepoBranchLines +/-
resourceloader: Modern ES6 code should be forced to target mobilemediawiki/coremaster+28 -0
Allow jquery.tipsy on mobilemediawiki/coremaster+1 -0
Revert "resourceloader: Modern ES6 code should be forced to target mobile"mediawiki/coremaster+0 -28
Revert "resourceloader: Modern ES6 code should be forced to target mobile"mediawiki/corewmf/1.40.0-wmf.13+0 -28
resourceloader: Modern ES6 code should be forced to target mobilemediawiki/coremaster+28 -0
resourceloader: Code using packageFiles runs on mobile targets by defaultmediawiki/coremaster+2 -1
Customize query in gerrit

Related Objects
Search...

Event Timeline

Jdlrobson created this task.Nov 21 2022, 11:18 PM
Jdlrobson updated the task description. (Show Details)Nov 21 2022, 11:23 PM
gerritbot added a comment.Nov 21 2022, 11:26 PM

Change 859149 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Modern code should be forced to target mobile

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

gerritbot added a project: Patch-For-Review.Nov 21 2022, 11:26 PM
Jdlrobson updated the task description. (Show Details)Nov 21 2022, 11:28 PM

@Krinkle I'd recommend this as it provides more momentum for eventually disabling the targets system (anyone migrating to ES6 or packageFiles is paying off the technical debt in T127268. There is no practical reason IMO why we should be using the targets system with ES6 / packageFiles code.

Jdlrobson renamed this task from Forbid new code from not targeting mobile to Forbid new modern code from not targeting mobile.Nov 22 2022, 8:11 PM
gerritbot added a comment.Nov 22 2022, 11:46 PM

Change 859633 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] All code using packageFiles runs on mobile and desktop targets

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

gerritbot added a comment.Nov 23 2022, 3:28 AM

Change 859633 abandoned by Jdlrobson:

[mediawiki/core@master] resourceloader: Code using packageFiles runs on mobile targets by default

Reason:

Will come back to this later.

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

larissagaulia updated Other Assignee, added: Krinkle.Nov 28 2022, 8:03 PM
larissagaulia moved this task from Inbox, needs triage to Backlog: Maintenance, non-prioritized on the Performance-Team board.
gerritbot added a comment.Dec 5 2022, 7:43 PM

Change 859149 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Modern ES6 code should be forced to target mobile

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.13; 2022-12-05).Dec 5 2022, 8:01 PM
Jdlrobson updated the task description. (Show Details)Dec 5 2022, 8:14 PM
Maintenance_bot removed a project: Patch-For-Review.Dec 5 2022, 8:30 PM
kostajh mentioned this in T324525: The module 'wikibase.lexeme.lexemeview' must not have target 'mobile' because its dependency 'jquery.ui.languagesuggester' does not have it.Dec 6 2022, 7:43 AM
gerritbot added a comment.Dec 6 2022, 8:19 AM

Change 864907 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] Revert "resourceloader: Modern ES6 code should be forced to target mobile"

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

gerritbot added a project: Patch-For-Review.Dec 6 2022, 8:19 AM
gerritbot added a comment.Dec 6 2022, 9:18 AM

Change 864912 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@wmf/1.40.0-wmf.13] Revert "resourceloader: Modern ES6 code should be forced to target mobile"

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

gerritbot added a comment.Dec 6 2022, 9:35 AM

Change 864907 merged by jenkins-bot:

[mediawiki/core@master] Revert "resourceloader: Modern ES6 code should be forced to target mobile"

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

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.14; 2022-12-12); removed MW-1.40-notes (1.40.0-wmf.13; 2022-12-05).Dec 6 2022, 10:00 AM
Lucas_Werkmeister_WMDE subscribed.Dec 6 2022, 12:31 PM
gerritbot added a comment.Dec 6 2022, 12:41 PM

Change 864912 merged by jenkins-bot:

[mediawiki/core@wmf/1.40.0-wmf.13] Revert "resourceloader: Modern ES6 code should be forced to target mobile"

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

Stashbot added a comment.Dec 6 2022, 12:52 PM

Mentioned in SAL (#wikimedia-operations) [2022-12-06T12:52:32Z] <kharlan@deploy1002> Started scap: Backport for [[gerrit:864912|Revert "resourceloader: Modern ES6 code should be forced to target mobile" (T323542)]]

Stashbot added a comment.Dec 6 2022, 12:54 PM

Mentioned in SAL (#wikimedia-operations) [2022-12-06T12:54:20Z] <kharlan@deploy1002> kharlan and kharlan: Backport for [[gerrit:864912|Revert "resourceloader: Modern ES6 code should be forced to target mobile" (T323542)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet

Stashbot added a comment.Dec 6 2022, 1:00 PM

Mentioned in SAL (#wikimedia-operations) [2022-12-06T13:00:29Z] <kharlan@deploy1002> Finished scap: Backport for [[gerrit:864912|Revert "resourceloader: Modern ES6 code should be forced to target mobile" (T323542)]] (duration: 07m 57s)

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.13; 2022-12-05); removed MW-1.40-notes (1.40.0-wmf.14; 2022-12-12).Dec 6 2022, 1:00 PM
kostajh mentioned this in T320518: 1.40.0-wmf.13 deployment blockers.Dec 6 2022, 1:02 PM
Maintenance_bot removed a project: Patch-For-Review.Dec 6 2022, 1:30 PM
gerritbot added a comment.Dec 6 2022, 3:51 PM

Change 864923 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] resourceloader: Modern ES6 code should be forced to target mobile

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

gerritbot added a project: Patch-For-Review.Dec 6 2022, 3:51 PM
Jdlrobson updated the task description. (Show Details)Dec 6 2022, 4:00 PM
gerritbot added a comment.Dec 6 2022, 10:47 PM

Change 865179 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Allow jquery.tipsy on mobile

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

gerritbot added a comment.Dec 6 2022, 11:14 PM

Change 865179 merged by jenkins-bot:

[mediawiki/core@master] Allow jquery.tipsy on mobile

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

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.14; 2022-12-12); removed MW-1.40-notes (1.40.0-wmf.13; 2022-12-05).Dec 7 2022, 12:01 AM
gerritbot added a comment.Dec 7 2022, 3:25 AM

Change 864923 abandoned by Jdlrobson:

[mediawiki/core@master] resourceloader: Modern ES6 code should be forced to target mobile

Reason:

I've underestimated the work involved here. Will leave an update on ticket with next steps.

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

Jdlrobson closed this task as Declined.Dec 7 2022, 3:28 AM

Hey @Catrope I underestimated Wikidata here. Wikidata is heavily using the targets system, and I've exhausted my timebox trying to port over all the modules there. I think this would have been a good thing to do when we added es6 but I think its too late now.

I think the best strategy is still T235712 (making sure modules don't get added to the page, and then when that hits zero or near zero, ignoring targets on module definitions. I'm going to decline this one.

@Krinkle @Catrope My concern is people keep adding new modules which are making this a moving target and making it even harder to undo. Is there a way we might use CI going forward to prevent new modules setting targets to only mobile or only desktop going forward that you can think of? Thanks in advance for any ideas you might have here!

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits