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

http: Using URL with http.request (feature suggestion) #20795

Closed
ronkorving opened this issue May 17, 2018 · 7 comments
Closed

http: Using URL with http.request (feature suggestion) #20795

ronkorving opened this issue May 17, 2018 · 7 comments
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@ronkorving
Copy link
Contributor

  • Version: v10.1.0
  • Platform: all
  • Subsystem: http

Using URL objects is clearly the way to go when it comes to dealing with URLs, and http.request() can take a URL object as input. This is great. But if I want to also send custom headers, I'm stuck. I would like to suggest an option for the options object called url that can be set to URL object and maybe even a string. I don't really want to make an already complex API more complex, but I'm not sure how else we can make it easier to use URL objects with HTTP requests.

Feedback, suggestions, thoughts from @nodejs/collaborators very welcome.

@Trott
Copy link
Member

Trott commented May 17, 2018

Can you provide some sample code showing the way you would like to be able to write calls to http.request() using this feature? Perhaps with either additional code showing how it has to be done currently?

@Trott Trott modified the milestone: http May 17, 2018
@Trott Trott added http Issues or PRs related to the http subsystem. feature request Issues that request new features to be added to Node.js. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels May 17, 2018
@bnoordhuis
Copy link
Member

Seems like a reasonable feature to me but what takes precedence when both .path and .url are present? Why?

Alternative: overload http.request() with a 3-arg http.request(url[, options, callback]).

@mcollina
Copy link
Member

I'm 👍 for a 3-arg signature.

@ronkorving
Copy link
Contributor Author

@Trott I was suggesting:

const options = { url: new URL('...'), headers: { foo: 'bar' } };
http.request(options, cb);

@bnoordhuis Good question, and I think your suggestion for 3-arg is a good alternative that makes the precedence very clear (the URL wins, right? :)).

@joyeecheung
Copy link
Member

Fun fact: in fs.createReadStream(path[, options]), when options.fd is specified, path gets ignored.

@benjamingr
Copy link
Member

Slightly related whatwg/fetch#452

@ronkorving
Copy link
Contributor Author

ronkorving commented May 18, 2018

@benjamingr Yeah, I think the takeaway there is that the URL is basically expected to be (or is cast to) a string, and URL.prototype.toString will do the trick for URL objects. I think that behavior (strings and URL objects) makes sense for us too.

targos pushed a commit to targos/node that referenced this issue Aug 2, 2018
Fixes: nodejs#20795
PR-URL: nodejs#21616
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit that referenced this issue Aug 7, 2018
Fixes: #20795
PR-URL: #21616
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Backport-PR-URL: #21880
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants