Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

url.format prefers host over hostname, but http.request does the opposite #2277

Closed
nfriedly opened this issue Jul 30, 2015 · 3 comments
Closed
Labels
http Issues or PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stalled Issues and PRs that are stalled. url Issues and PRs related to the legacy built-in url module.

Comments

@nfriedly
Copy link
Contributor

I noticed this when working on #2271 - regardless of which one you think should "win", you'd expect the last two lines here to end up with the same url:

var myUrl = url.parse('http://foo.com/');
myUrl.host = 'bar.com';
url.format(myUrl); // => 'http://bar.com/'
http.get(myUrl); // makes a request to http://foo.com/

To node/io.js's credit, they are both documented correctly, but that only goes so far.

I don't really care which one wins (I suppose I'd pick hostname if I had to choose), but I think we should consider making a breaking change at some point in order to make those two APIs more consistent.

(Getters and setters on Url objects would help, but I could imagine someone doing this to a regular Object as well, so I still think the APIs should be consistent.)

@Fishrock123 Fishrock123 added http Issues or PRs related to the http subsystem. url Issues and PRs related to the legacy built-in url module. labels Jul 30, 2015
@Fishrock123
Copy link
Contributor

(Getters and setters on Url objects would help, but I could imagine someone doing this to a regular Object as well, so I still think the APIs should be consistent.)

See #1591 and related (reverts of changes originally introduced in #1561) for why this will take an extended period of time to change to.

@nfriedly
Copy link
Contributor Author

Yea, I looked at #1591 and decided that this was an different enough issue to merit it's own ticket.

@Trott Trott added the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 11, 2016
@jasnell jasnell added the stalled Issues and PRs that are stalled. label Apr 9, 2016
@jasnell
Copy link
Member

jasnell commented Mar 24, 2017

this is unlikely to change in the current url module implementation due to backwards compat concerns. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stalled Issues and PRs that are stalled. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

No branches or pull requests

4 participants