Polyfill does not properly iterate options.headers, if they are an instance of window.Headers

[joseph-galindo]

Hey,

I am liking this polyfill, but I noticed a minor issue when trying to polyfill fetch calls that use an options.headers that are an instance of window.Headers instead of a simple object literal.

In this case, the loop here: https://github.com/developit/unfetch/blob/master/src/index.mjs#L41-L43

Tries to loop it as a normal object literal, giving output like this:

> let foo = new Headers()
< undefined
> foo
< Headers {}
> foo.set('Content-Type', 'application/json')
< undefined
> foo
< Headers {}
> for (const i in foo) { console.log(i); console.log(foo[i]); }
> VM860:1 append
> VM860:1 ƒ append() { [native code] }
> VM860:1 delete
> VM860:1 ƒ delete() { [native code] }
> VM860:1 get
> VM860:1 ƒ () { [native code] }
> VM860:1 has
> VM860:1 ƒ has() { [native code] }
> VM860:1 set
> VM860:1 ƒ () { [native code] }
> VM860:1 keys
> VM860:1 ƒ keys() { [native code] }
> VM860:1 values
> VM860:1 ƒ values() { [native code] }
> VM860:1 forEach
> VM860:1 ƒ forEach() { [native code] }
> VM860:1 entries
> VM860:1 ƒ entries() { [native code] }
> undefined

I think this loop needs to be updated to check if:

• window.Headers exists
• if it does exist, if options.headers instanceof window.Headers

If both are true, they can be iterated on using https://developer.mozilla.org/en-US/docs/Web/API/Headers/entries, instead of a simple for...in loop

For the sample case above:

> for (const pair of foo.entries()) { console.log(pair[0]); console.log(pair[1]); }
< VM1247:1 content-type
< VM1247:1 application/json

I plan to open a PR for this today, but wanted to write the issue first

[joseph-galindo]

On second thought, it looks like support for Request and Headers has been intentionally avoided?

#90 (comment)

I will still create the PR, but may need to fork if this codebase would prefer to not have support for these APIs.

[joseph-galindo]

I have made the changes to add the support I needed, but unfortunately could not add them without notable size increases (up to 550b).

With that in mind I plan to limit these changes to my fork, and will close this issue.

[dmitryt]

Hey, guys. When are you going to release this fix?

[joseph-galindo]

@dmitryt I ended up closing the PR against developit/unfetch since the proposed changes would exceed the size constraints.

I have a fork with the changes hosted on my personal github, but I did not have plans to publish it to the public npm registry. I currently only use my fork as a code mirror within my employer's closed-source codebase.

If you want to use the fork, I think your package.json could link to it directly using the git url. If you want to see these changes make it into the mainline developit/fetch, I could reopen this issue and the corresponding PR, but I am hard-pressed to fit this work into the size constraints.