doc: clarify http.request supports headers as array #58769
Conversation
This PR updates the documentation for `http.request()` to clarify that the `headers` option can also be an array of key-value pairs, similar to `response.writeHead`. Fixes: nodejs#57986
|
Review requested:
|
nodejs-github-bot
added
doc
There was a problem hiding this comment.
Thanks for contributing @Dinesh0007000! This is indeed officially supported and it's great to get this documented.
There's a few issues here to tidy up to get this ready to merge. I've added a comment on the markdown, and the commit message check is also failing due to line length. The guidelines for commit messages are here: https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines
doc/api/http.md
Outdated
| * `headers` {Object} An object containing request headers. | ||
| * `headers` {Object | Array} Request headers to send with the request. This can be: | ||
| * An object like `{ 'Content-Type': 'application/json' }`, or | ||
| * An array of key-value pairs like `[ 'Content-Type', 'text/plain', 'X-Custom', 'yes' ]`, similar to how headers are passed to `response.writeHead()`. |
There was a problem hiding this comment.
There's a series of extra spaces here (are passed), and this line is very long, which will be why the markdown linting is failing here. Can you fix that? You can run the markdown linting locally by running make lint-md.
"An array of key-value pairs" is not clear I think - that will make people think of [["key1", "value1"], ["key2", "value2"]] which is not what's happening. We should be specific that this is a flat array of values, not tuples, since this can be confusing.
I think it would be good to link to response.writeHead here, and request.rawHeaders as well (which uses the same format).
It might be better to keep the text inline here simple (a very short explanation of "headers object or flat array of raw headers" or similar) and then include the larger explanation & examples as a paragraph in the text below. What do you think? See what looks best - you can test the docs locally by running make docserve and then looking at http://localhost:8000.
Added a detailed explanation for how the 'headers' option in http.request can also be passed as a flat array of alternating header names and values. This provides clarity for beginners and aligns with the format used in response.writeHead() and request.rawHeaders.
There was a problem hiding this comment.
There's a couple of tweaks required here, and the linting is still failing for the commit message & markdown. You'll need to look at the failure details there and fix this. For the markdown, you can test this locally with make lint-md.
For the commit message, the issue is that you've added a new commit, but the linting always runs against the first commit in the PR (because that's the commit that typically later appears in the changelog). You'll need to edit that previous commit instead and force push your change to overwrite it.
doc/api/http.md
Outdated
| @@ -4329,3 +4341,6 @@ A browser-compatible implementation of [`WebSocket`][]. | |||
| [`writable.uncork()`]: stream.md#writableuncork | |||
| [`writable.write()`]: stream.md#writablewritechunk-encoding-callback | |||
| [initial delay]: net.md#socketsetkeepaliveenable-initialdelay | |||
| [`response.writeHead()`]: https://nodejs.org/api/http.html#responsewriteheadstatuscode-statusmessage-headers | |||
There was a problem hiding this comment.
I think you can skip this one - it's already defined above (line 4323)
doc/api/http.md
Outdated
| @@ -4329,3 +4341,6 @@ A browser-compatible implementation of [`WebSocket`][]. | |||
| [`writable.uncork()`]: stream.md#writableuncork | |||
| [`writable.write()`]: stream.md#writablewritechunk-encoding-callback | |||
| [initial delay]: net.md#socketsetkeepaliveenable-initialdelay | |||
| [`response.writeHead()`]: https://nodejs.org/api/http.html#responsewriteheadstatuscode-statusmessage-headers | |||
| [`request.rawHeaders`]: https://nodejs.org/api/http.html#requestrawheaders | |||
There was a problem hiding this comment.
Can you update this so it's not absolute? It should be #requestrawheaders like the other links above. That way it'll work properly locally, or if we ever change the URLs.
- Removed duplicate definition of [`response.writeHead()`] as it already exists earlier in the file. - Updated [`request.rawHeaders`] link to be relative (#requestrawheaders) instead of using an absolute URL. These fixes resolve markdown linting issues flagged by the CI and ensure consistency with documentation standards.
This PR updates the documentation for
http.request()to clarify that theheadersoption can also be provided as an array of key-value pairs, like['Content-Type', 'text/plain', 'X-Custom', 'yes'].This behavior aligns with
response.writeHead(), but was previously undocumented in thehttp.request()API docs.Fixes: #57986