Issue 3515 - Added polyfill for the fetch() API

Issue 3515 - Added polyfill for the fetch() API

[kzar, 2016-01-15 19:35:19]

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch.js
File lib/polyfill/fetch.js (right):

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:18: "use strict";
Like in the other review, shouldn't this be inside the IIFE?

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:29: Response.prototype = {
Seems like you've omitted quite a few of the Response methods?

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:40: global.fetch = function(url)
Shouldn't this also take a second argument for the request's init options?

[Sebastian Noack, 2016-01-19 13:09:32]

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch.js
File lib/polyfill/fetch.js (right):

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:18: "use strict";
On 2016/01/15 19:35:18, kzar wrote:
> Like in the other review, shouldn't this be inside the IIFE?

Like in the other review, it shouldn't. ;)

Putting "use strict" on the top-level will turn on strict mode for the whole
file, but not for other files even if they share the same context.

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:29: Response.prototype = {
On 2016/01/15 19:35:18, kzar wrote:
> Seems like you've omitted quite a few of the Response methods?

As further explained in the other review, lets keep things simple and only
implement the APIs we actually need.

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:40: global.fetch = function(url)
On 2016/01/15 19:35:18, kzar wrote:
> Shouldn't this also take a second argument for the request's init options?

See above.

[kzar, 2016-01-19 15:03:24]

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch.js
File lib/polyfill/fetch.js (right):

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:18: "use strict";
On 2016/01/19 13:09:31, Sebastian Noack wrote:
> On 2016/01/15 19:35:18, kzar wrote:
> > Like in the other review, shouldn't this be inside the IIFE?
> 
> Like in the other review, it shouldn't. ;)
> 
> Putting "use strict" on the top-level will turn on strict mode for the whole
> file, but not for other files even if they share the same context.

Acknowledged.

https://codereview.adblockplus.org/29333518/diff/29333623/lib/polyfill/fetch....
lib/polyfill/fetch.js:29: Response.prototype = {
On 2016/01/19 13:09:31, Sebastian Noack wrote:
> On 2016/01/15 19:35:18, kzar wrote:
> > Seems like you've omitted quite a few of the Response methods?
> 
> As further explained in the other review, lets keep things simple and only
> implement the APIs we actually need.

Acknowledged.

https://codereview.adblockplus.org/29333518/diff/29333958/lib/polyfill/fetch.js
File lib/polyfill/fetch.js (right):

https://codereview.adblockplus.org/29333518/diff/29333958/lib/polyfill/fetch....
lib/polyfill/fetch.js:56: xhr.overrideMimeType("text/plain");
I'm curious, why do this?

[Sebastian Noack, 2016-01-19 15:08:29]

https://codereview.adblockplus.org/29333518/diff/29333958/lib/polyfill/fetch.js
File lib/polyfill/fetch.js (right):

https://codereview.adblockplus.org/29333518/diff/29333958/lib/polyfill/fetch....
lib/polyfill/fetch.js:56: xhr.overrideMimeType("text/plain");
On 2016/01/19 15:03:24, kzar wrote:
> I'm curious, why do this?

Otherwise, XMLHttpRequest would try to parse the response if it is XML, HTML of
JSON. However, fetch() doesn't do any automatic parsing. So the parsed result is
ignored anyway, and if necessary the document would be parsed twice.

[kzar, 2016-01-19 15:41:02]

LGTM

https://codereview.adblockplus.org/29333518/diff/29333958/lib/polyfill/fetch.js
File lib/polyfill/fetch.js (right):

https://codereview.adblockplus.org/29333518/diff/29333958/lib/polyfill/fetch....
lib/polyfill/fetch.js:56: xhr.overrideMimeType("text/plain");
On 2016/01/19 15:08:29, Sebastian Noack wrote:
> On 2016/01/19 15:03:24, kzar wrote:
> > I'm curious, why do this?
> 
> Otherwise, XMLHttpRequest would try to parse the response if it is XML, HTML
of
> JSON. However, fetch() doesn't do any automatic parsing. So the parsed result
is
> ignored anyway, and if necessary the document would be parsed twice.

Acknowledged.