Fix TypeError on Edge 18

[belgattitude]

Fixes TypeError with edge 18 and probably a bunch of older browsers. I took a different approach from #261, while we are tackling the same problem.

IMO the fix should not be made in mergeHeaders, but in the Ky constructor. The this._input is string|Url|Request, headers won't be present if a string is passed (url)... Modern browsers won't care so much and merge with 'undefined', but the error seems legit to me.

Regarding testing, it's almost impossible to test this. At least I've tested on browserstack with success (edge 18 on Window 10).

Will close

• Edge <= 18 Invalid argument exception #260

Notes:

As reported on sentry

[belgattitude]

@sindresorhus if this fix looks right to you, do you think it can be released in a short time... Just to know, otherwise I'll use a fork meanwhile.

Thanks for your work.

[belgattitude]

For info I also had publish an hotfix in this package: https://www.npmjs.com/package/belgattitude-ky.

yarn add belgattitude-ky@0.20.1-canary.0

Webpack/Nextjs users can alias it to ky.

config.resolve.alias['ky'] = 'belgattitude-ky';

Just a rush solution...

[sholladay]

The constructor is not the only place that we use mergeHeaders(), though, and thus it is not the only place that is prone to the error. I believe your approach would fail when undefined headers are received via ky.extend().

It's for precisely these kinds of reasons that we cannot accept this without a test. I disagree that it's hard to test. You should be able to make a test that monkey patches the Headers constructor to match the offending browsers.

Something along the lines of...

const OriginalHeaders = Headers;
global.Headers = class Headers {
    constructor(init) {
        // Do what the offending browsers do here...
        // e.g. if (init === undefined) { throw new Error() }
    }
}
// ... test Ky ...
global.Headers = OriginalHeaders;

[belgattitude]

Agree haven't thought about extend... and was not sure about how throwing an error would be received in relation to possible BC.

I'll update tonight. Thanks @sholladay

[belgattitude]

@sholladay The approach taken by @JosNun #261 is equivalent, but tackles the issue in the mergeHeaders method.

Both valid, what's your feeling about the road to take. Either way I'm fine

[sholladay]

I prefer the approach in #261 because it should handle .extend() properly. But whatever works. If you are able to write a test, we can just update the code in this PR.

[belgattitude]

@sholladay @JosNun It's actually a better idea to do it like #261 ... I'm about to close this one if you agree

[JosNun]

Fine with me