Codecov Report

Merging #311 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #311   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines         422    428    +6     
  Branches      133    134    +1     
=====================================
+ Hits          422    428    +6

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7c1ef8...7bafe6b. Read the comment docs.

data URLs are a can of worms as they are notoriously underspecified. I would prefer to use an established parser on npm or something instead of rolling our own. https://www.npmjs.com/package/data-uri-to-buffer looks pretty nice.

Besides that, you have to construct a Response object, and return a promise from fetch that will resolve to that.

You also have to pass the type along to the Response object as a Content-Type header per spec.

Handling non-base64 URLs is needed too. See https://tools.ietf.org/html/rfc2397#section-2 on the format for data URLs in general, though data-uri-to-buffer will do this for you.

Oh, and you need to be careful to wrap all the errors that may occur in the URL-to-data process in a promise instead of throwing synchronously in fetch().

Ok, great, thanks, will do, I'll try to behave like spec/browser

it's not very beautiful, but is it ok now?

I'd like to put this on hold because of whatwg/fetch#579.

Looks like the standard change has landed. @caub, can you update this module to use https://www.npmjs.com/package/data-urls instead?

@TimothyGu yup, done

Is data-urls now a dependency instead of devDependency?

@bitinn done, indeed

it fails on node4 because data-urls contains some destructuring https://github.com/jsdom/data-urls/blob/master/lib/parser.js#L3

might add rollup there so