doc: clearify that http does chunked encoding itself.

[micromaomao]

I don't know if I'm putting it in the right place in the doc (or if there is a better way to present this information), but I think stating this is necessary, and I couldn't find any mention of the fact that chunked encoding is handled by node itself in the existing http doc.

Checklist

• documentation is changed or added
• commit message follows commit guidelines

[micromaomao]

Right… The commit guidelines didn't say no punctuation.

I guess anyone merging this can simply do a --amend.

[Trott]

Welcome @micromaomao and thanks for the pull request! As it currently stands, I think this addition to the docs probably creates more confusion than it clarifies. From the text, it's not clear if chunked is the default for the optional encoding argument, or if chunked encoding is a thing that it always does but that's separate/different from the encoding argument supplied to the function.

[Trott]

Suggested change

	Note that this module automatically encode data into chunked encoding.
	This module automatically encodes data in chunked encoding.

(That's a grammar fix and a style fix, but the content is still problematic, I think.)

[micromaomao]

I see what you're saying.

I also noticed another issue, the doc already says:

By calling this method many times, a request body can be sent to a server — in that case it is suggested to use the ['Transfer-Encoding', 'chunked'] header line when creating the request.

What does suggested to use mean? According to my very quick scan of the source code in lib/_http_outgoing.js:360 (this is my first time reading node source, so I might have looked in the wrong place), the Transfer-Encoding header is automatically added if Content-Length is not provided, and node will automatically encode data with chunked encoding. So I think it isn't clear what is the difference if the user add the Transfer-Encoding: chunked header or not.

That aside, I think maybe the sentence should be rephrased as something like:

This method can be called multiple times. If no Content-Length is set, data will automatically be encoded in HTTP Chunked transfer encoding, so that server knows when data ends. The Transfer-Encoding: chunked header will be added. Call end() when finish sending data so that node would send the trailer.

Please correct me if my understanding is incorrect.

[Trott]

What does suggested to use mean?

In general, I'd be inclined to simply replace any occurrences of it is suggested to use or it is recommended to use to the more succinct use.

In this particular case, if the suggestion is unnecessary, I'd remove it entirely.

That aside, I think maybe the sentence should be rephrased as something like:

This method can be called multiple times. If no Content-Length is set, data will automatically be encoded in HTTP Chunked transfer encoding, so that server knows when data ends. The Transfer-Encoding: chunked header will be added. Call end() when finish sending data so that node would send the trailer.

/ping @nodejs/documentation @nodejs/http Does that change seem correct to you? (There are a few minor style changes I'd make to that, but I'm mostly concerned about getting the content and meaning correct first.)

[BridgeAR]

@nodejs/http @nodejs/http2 PTAL

[mcollina]

I'm good with an edited version of the text in #28379 (comment).

Would you mind adding it to the PR?

[micromaomao]

@mcollina Updated.

@Trott wanted some style change. Also, looking at it now, I can see how the change may make the part that says

The encoding argument is optional and only applies when chunk is a string. Defaults to 'utf8'.

a little bit confusing. Maybe we should make it clear that the encoding argument has nothing to do with the chunked transfer encoding, by e.g. saying that it is about encoding the string to Buffer?

style changes applied

[ronag]

If no Content-Length is set, data will automatically be encoded in HTTP
Chunked transfer encoding, so that server knows when data ends. The Transfer-Encoding: chunked header will be automatically added.

Would be simpler maybe?

[ronag]

With a reference to https://nodejs.org/api/stream.html#stream_writable_write_chunk_encoding_callback? Even though it is not a Writable the intention is to act as similar as possible. @mcollina, what do you think? Would such a reference be harmful?

[HarshithaKP]

@micromaomao, can you follow up with review comments ? and this needs a rebase.

[aduh95]

@nodejs/http This needs reviews.

[mcollina]

I would retain the first sentence

Sends a chunk of the body.

[aduh95]

@micromaomao Can you address the last comment please?

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

[micromaomao]

I think it's alright. Do you want me to amend the commit adding the "Sends a chunk of the body." sentence to the beginning?

[micromaomao]

amended, squashed and rebase'd. Also it's been one year so someone probably needs to verify that this is still the behavior of the function.

[micromaomao]

@aduh95 done

[aduh95]

@nodejs/http Can we land this?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/38616/

[mcollina]

lgtm

[dnlup]

Landed in 67d4a3f