(#3885) - Service worker & fetch() fixes

[marten-de-vries]

Changes I needed while writing
https://gist.github.com/marten-de-vries/3bfaf635f9efdbcf5102

[nolanlawson]

Looks good to me; kinda surprised that @KlausTrainer didn't run into this too though?

[marten-de-vries]

@nolanlawson Probably because these changes are only necessary to make PouchDB talk to a remote (http) database using fetch(). IDB backed db worked fine already.

[KlausTrainer]

The comment is misleading, and contradicts itself with regard to the code below. Also, it changes the behaviour so that the fetch fallback is used if XMLHttpRequest is present, but of type 'object' instead of type 'function', which is probably not what we want.

[marten-de-vries]

If XMLHttpRequest is an 'object', you can't do new XMLHttpRequest() (that's a TypeError: object is not a function). That happened in Chromium 41 inside a service worker, so I decided to check for 'function' instead.

The comment was written with the 'old' situation in mind, not the new one. I agree it's confusing. It's probably best to just remove the comment completely. Does that work for you, @KlausTrainer?

[KlausTrainer]

Ah, ok. Well, I agree then that it would be best to just remove the comment :)

[KlausTrainer]

So I guess the actual issue is that in current master there is a declaration of a result variable (see here) that is never initialized, but later used in return result.json(); (see here). There should rather be a return response.json(); instead, and the variable declaration is unnecessary.

[marten-de-vries]

@KlausTrainer That's indeed one issue that's fixed, but it's not the only one. Returning an object with a statusCode instead of a status is also important for request compatibility.

[KlausTrainer]

Ah, thanks. Just saw the equivalent part in the xhr.onreadystatechange function :)

[KlausTrainer]

@marten-de-vries Looks good to me. Also I like the wrappedPromise cleanup.

[nolanlawson]

Failures seem genuine, particularly in PhantomJS and Safari. Might be some old Webkit bug here that we're not accounting for.

[nolanlawson]

Hah, called it. Check out Safari:

[nolanlawson]

Suggestion: can we use typeof fetch === 'function' && !opts.xhr?

[nolanlawson]

UGH if we do that, though, then our idb-blob-support check is going to fail, because Chrome doesn't support fetch()ing blobs at all, so it'll just fall back to base64 everywhere. And checking for window.fetch is not enough, because it was added in Chrome 42, but blobs were still broken until Chrome 43. I hate the web sometimes.

[nolanlawson]

OK how about this: #3890.

[marten-de-vries]

@nolanlawson Looks good, I just merged that into this PR & squashed.

[nolanlawson]

👍, 2914285