Skip to content

Commit

Permalink
Revert "http: don't bother making a copy of the options"
Browse files Browse the repository at this point in the history 
This reverts commit 06cfff9.

Reverted because it introduced a regression where (because options were
modified in the later functionality) options.host and options.port would
be overridden with values provided in other, supported ways.

PR-URL: #1467
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
@brendanashworth
brendanashworth committed Apr 18, 2015
1 parent 6870764 commit 7180597
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/_http_client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ function ClientRequest(options, cb) {

if (typeof options === 'string') {
options = url.parse(options);
} else {
options = util._extend({}, options);
}

var agent = options.agent;
Expand Down

5 comments on commit 7180597

@mogadanez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this is return issue with options created by constructor where props defined in prototype
nodejs/node-v0.x-archive#14569

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mogadanez would options = Object.create(options || {}) work?

(as suggested here: #1467 (comment))

@brendanashworth
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mogadanez I think as long as the options aren't modified, we can support getters - would you like to open a PR along the lines of @Fishrock123 's comment / #592?

@mogadanez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not work. Object,create does not make a copy

    console.log(JSON.stringify(options))
    options = Object.create(  options )
    console.log(JSON.stringify(options))

{"host":"node-path-test.s3-website-us-east-1.amazonaws.com","method":"get"}
{}

@brendanashworth
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mogadanez the properties should be on the prototype, util.inspect just doesn't show properties there.

Please sign in to comment.