request.defaults could support headers better. #257

Closed
zoowar opened this Issue May 29, 2012 · 11 comments

Projects

None yet

3 participants

@zoowar
zoowar commented May 29, 2012

request.defaults could support headers better.
version: '2.9.202',

For example
var req = request.defaults({
headers: {
"user-agent": "some user agent",
},
});

req.get(uri, {headers: {"if-modified-since: "Tue, 29 May 2012 19:30:57 GMT"}, ...

Curently, we see the following headers on the wire
{"if-modified-since: "Tue, 29 May 2012 19:30:57 GMT"}

What we would like to see is
{"user-agent": "some user agent", "if-modified-since: "Tue, 29 May 2012 19:30:57 GMT"}

@mikeal
Member
mikeal commented Jul 25, 2012

i thought I did this? i used to do this, it must have broken.

@manuel-woelker manuel-woelker added a commit to manuel-woelker/request that referenced this issue Dec 25, 2012
@manuel-woelker manuel-woelker test case for #257: merging default and per-request headers a5524a8
@manuel-woelker manuel-woelker added a commit to manuel-woelker/request that referenced this issue Dec 25, 2012
@manuel-woelker manuel-woelker defaults: merge default and per-request headers (fixes #257)
added underscore as dependency in package.json
c288ef3
@aj0strow
Contributor

76a96de ???

@mikeal
Member
mikeal commented Jun 25, 2014

i think it was before that, because that was just a revert from the lodash merge.

@aj0strow
Contributor

Lodash does a deep merge tho, which fixes the issue. The headers object is overwritten in the current version.

@mikeal
Member
mikeal commented Jun 25, 2014

we want a deep clone and the

@mikeal mikeal closed this Jun 25, 2014
@mikeal mikeal reopened this Jun 25, 2014
@mikeal
Member
mikeal commented Jun 25, 2014

whoops, what i meant was

we only want a deep clone on a couple properties.

@aj0strow
Contributor

Ah true. Looks like there's already PRs so I'll be patient.

@mikeal
Member
mikeal commented Jun 25, 2014

where?

@aj0strow
Contributor

Nvm, misread this title #890. Shall i submit one?

@mikeal
Member
mikeal commented Jun 25, 2014

go for it :)

@mikeal
Member
mikeal commented Aug 27, 2014

this is fixed.

@mikeal mikeal closed this Aug 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment