sindresorhus / got

🌐 Human-friendly and powerful HTTP request library for Node.js
MIT License
14.27k stars 935 forks source link

New options `https` are not merged when extend() #1303

Closed joolfe closed 4 years ago

joolfe commented 4 years ago

Describe the bug

Actual behavior

The new config fields for options.https are not merged correctly when extend a got instance.

Expected behavior

If you have an instance of got that has some config of options.https and provide a new one then the resulted instance should has a merged verison.

Code to reproduce

const { describe, it } = require('mocha')
const Got = require('got')
const { deepEqual } = require('assert').strict

describe('Got https options', function () {
    it('should merge http options', function () {
        const got1 = Got.extend({ https: { certificateAuthority: 'TestCertificate' } })
        const got2 = Got.extend(got1, { https: { key: 'TestKey' } })
        deepEqual(got2.defaults.options.https, {
            certificateAuthority: 'TestCertificate',
            key: 'TestKey' 
        })
    })
})

Checklist

Giotino commented 4 years ago

You are right, currently the entire https object is merged, not the single options inside it.

@szmarczak I really haven't thought about it, if you think that it should merge every single options I can fix it.

sindresorhus commented 4 years ago

We should do deep merge. That’s the expected behavior.

sindresorhus commented 4 years ago

https://github.com/sindresorhus/got/releases/tag/v11.3.0