Skip to content

doc: improve http.setHeader and getHeader typeinfo #19902

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.

Sign up for GitHub

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

Closed
Flarna wants to merge 5 commits into from
Closed

doc: improve http.setHeader and getHeader typeinfo #19902

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3699198
doc: improve http.setHeader and getHeader typeinfo
Flarna Apr 9, 2018
32685cf
fix review items
Flarna Apr 10, 2018
1d5c3e2
fix review items
Flarna Apr 10, 2018
5dc2a5d
fix review findings, correct request and response APIs
Flarna Apr 10, 2018
3ab5dc7
setHeader accepts type any
Flarna Apr 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions doc/api/http.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,9 +587,11 @@ added: v1.6.0
-->

* `name` {string}
* Returns: {string}
* Returns: {string, string[]} - type as set via `setHeader()`

@Trott Trott Apr 9, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The type as set via part seems out of place. If the information is critical, it can be included in the text below.

Use a | to separate the two possible return types, not a comma. (This is to be consistent with our documentation elsewhere.)

@Trott Trott Apr 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vsemozhetbyt Yes, thanks, updated!


Reads out a header on the request. Note that the name is case insensitive.
The return value is that one set via `setHeader()` as coercing to string
happens during transmission.

@Trott Trott Apr 9, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This sentence is a bit difficult to understand. How about this instead:

The type of the return value depends on the arguments provided to `setHeader()`.

Also, setHeaders() should be a link to that function in the docs.

@Trott Trott Apr 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vsemozhetbyt Whoops, yes, thanks for the correction!


Example:
```js
Expand Down Expand Up @@ -1210,7 +1212,8 @@ added: v0.4.0

Sets a single header value for implicit headers. If this header already exists
in the to-be-sent headers, its value will be replaced. Use an array of strings
here to send multiple headers with the same name.
here to send multiple headers with the same name. Values are coerced to
strings if needed during transmission.

@Trott Trott Apr 9, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps just this for the last sentence? Non-string values will be coerced to strings.


Example:

Expand Down