Page MenuHomePhabricator
Create Task
Maniphest T260198

Add fetch polyfill to mediawiki core
Closed, ResolvedPublic
Actions

Description

As we move towards Vue and adopt Vue components, we are keen to create reusable libraries that can be used outside MediaWiki for rendering Wikimedia-specced Vue components. One implication of this is that we cannot make assumptions about the existence of jQuery and/or the existence of mw.Api if the library is to be used by teams such as analytics.

Cross-checking caniuse [1] with our browser compatibility table [2] fetch is supported with the exceptions of:

  • IE11
  • Android browser
  • iOS 6.1 => 10.3

Note we only support Edge current and latest versions (currently 83 and 84)

I think it's likely a mistake that we support Android browser - the traffic there is low and will hopefully be confirmed by T248907
but iOS 9 accounts for 0.6% of our traffic and IE11 2%.

An optionally loaded polyfill [3] added to MediaWiki core using a skipFunction would support these 2 browsers and allow us to use fetch with a few documented caveats [4] that do not seem like deal-breakers to me. Note we also already have Promise support thanks to T237688.

Thoughts?

[1] https://caniuse.com/#search=fetch
[2] https://www.mediawiki.org/wiki/Compatibility#Modern_(Grade_A)
[3] https://github.com/github/fetch#installation
[4] https://github.github.io/fetch/, https://www.npmjs.com/package/whatwg-fetch

Details

SubjectRepoBranchLines +/-
resources: Add a fetch polyfill with skip functionmediawiki/coremaster+1 K -0
Customize query in gerrit

Related Objects

Event Timeline

Jdlrobson created this task.Aug 11 2020, 9:51 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 11 2020, 9:51 PM
Jdlrobson added projects: MediaWiki-User-Interface, MediaWiki-ResourceLoader.Aug 11 2020, 9:52 PM
Restricted Application added a project: Performance-Team. · View Herald TranscriptAug 11 2020, 9:52 PM
Jdlrobson added projects: Browser-Support-Apple-Safari, Browser-Support-Internet-Explorer.Aug 11 2020, 9:52 PM
Krinkle triaged this task as Low priority.Aug 11 2020, 10:33 PM

I think this is fine overall, but I'd like to take some time to do an inventory of the options available here and also think ahead about where this would fit in with other API abstraction layers we have and whether it would make sense as its own module bundle or that it'd be better to ship with a larger bundle (e.g. with other polyfills, or with other API-related stuff and take the hit on a the unused code).

For now, I don't think we should fast-track this. For the Vue case, there's a couple ways this could be done orthogonally so that we can do this later with more attention to the detail, and temporarily take a small amount of debt and/or robustness. Some ideas, in no particular order:

  1. Inject a simple Promise-returning get-request function, to call with either fetch()text() or $.ajax().
  2. Inject a simple Promise-returning get-json function, to call with either fetch()json() or $.ajax(…"json").
  3. Recognise that loading jquery (or slim-jquery) doesn't matter much inside apps on the short-term and just use it anyway.
  4. Use XHR directly with a 5-line wrapper function, supported in all browsers (http://youmightnotneedjquery.com/#request).
Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.Aug 11 2020, 10:33 PM
Krinkle moved this task from Inbox, needs triage to To-do: Goals, prioritized next 4 Quarters on the Performance-Team board.
• bearND subscribed.Aug 12 2020, 8:25 PM
Jdlrobson moved this task from Backlog to General requests on the MediaWiki-User-Interface board.Aug 27 2020, 4:48 PM
Jdlrobson added a subtask: T237688: Raise MW JS requirement to include ES6 Promise (drop IE11, Safari 5-6, and Android 4.1-4.3).Oct 21 2020, 4:56 PM
Krinkle added a comment.Oct 21 2020, 9:27 PM

@Jdlrobson Did you mean to depend on Provide es6-promise polyfill? (That is already resolved.) Or is there something about a particular fetch polyfill you have in mind that would require browser support to be raised first?

Krinkle removed a subtask: T237688: Raise MW JS requirement to include ES6 Promise (drop IE11, Safari 5-6, and Android 4.1-4.3).Nov 19 2020, 3:59 AM
Krinkle moved this task from To-do: Goals, prioritized next 4 Quarters to Abstract Wikipedia matrixing on the Performance-Team board.Mar 22 2021, 7:48 PM
Krinkle raised the priority of this task from Low to Medium.Mar 22 2021, 7:51 PM
gengh claimed this task.Apr 14 2021, 1:25 PM
gengh added a comment.Apr 14 2021, 3:18 PM

Some options, as suggested:

Seems the most commonly used and well maintained. Supports Safari 6.1+ and IE 10+. The most important problem (which seems to share with other polyfills) is that it will not reject HTTP error status, but that doesn't seem to be a problem considering mw.api error returns. Supports more options than other polyfills, but still not all, redirect and cache are not supported. Promise-based.

Polyfill to the bare minimum, but only support for IE 8+ and only takes the options method, headers, credentials and `body. Same issue with HTTP error responses as whatwg-fetch. Also depends on Promise being present.

Other well-maintained options such as[[ https://github.com/lquixada/cross-fetch | cross-fetch ]] use whatwg for browsers and other polyfills for other platforms (node, react native). For the moment every other option looks limited or not maintained.

gerritbot added a comment.Apr 15 2021, 11:56 AM

Change 679775 had a related patch set uploaded (by Genoveva Galarza; author: Genoveva Galarza):

[mediawiki/core@master] Add fetch polyfill with skip function

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

gerritbot added a project: Patch-For-Review.Apr 15 2021, 11:56 AM
• Gilles moved this task from Abstract Wikipedia matrixing to Doing: Prio Interrupt on the Performance-Team board.May 20 2021, 2:07 PM
gerritbot added a comment.May 25 2021, 10:06 PM

Change 679775 merged by jenkins-bot:

[mediawiki/core@master] resources: Add a fetch polyfill with skip function

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

Maintenance_bot removed a project: Patch-For-Review.May 25 2021, 10:10 PM
Krinkle added a comment.Jun 8 2021, 11:43 PM

@Jdlrobson I've done basic testing on IE11 (see patch review comments) so I'll close this one for now, but you may want to file a follow-up for your team to start use of this in some of your products and verify that it also handles the specific use cases you have for API calls and such.

Krinkle closed this task as Resolved.Jun 8 2021, 11:43 PM
egardner mentioned this in T301489: Create a "web 2017" polyfill RL module.Feb 10 2022, 5:05 PM
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