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.

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

Jump to bottom

doc: deprecate finished #28679

Closed
wants to merge 12 commits into from
+37 −0
Closed

doc: deprecate finished #28679

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ff98ad5
doc: deprecate http finished
ronag Jul 14, 2019
e1408d3
fixup
ronag Sep 19, 2019
fffc596
fixup
ronag Sep 19, 2019
53add50
fixup
ronag Sep 19, 2019
4d8bfdf
Update doc/api/deprecations.md
ronag Nov 19, 2019
7b70641
Update doc/api/deprecations.md
ronag Nov 19, 2019
39be20a
Update doc/api/deprecations.md
ronag Nov 19, 2019
2fa2ec6
fixup
ronag Nov 19, 2019
11337b6
Update doc/api/deprecations.md
ronag Nov 20, 2019
dcd3724
fixup: ref
ronag Nov 20, 2019
476d612
Update doc/api/deprecations.md
ronag Dec 7, 2019
60c6119
Update doc/api/deprecations.md
ronag Dec 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions doc/api/deprecations.md
View file
Open in desktop
Expand Up @@ -2518,6 +2518,27 @@ Type: Documentation-only
Prefer [`response.socket`][] over [`response.connection`] and
[`request.socket`][] over [`request.connection`].

<a id="DEP0XXX"></a>
### DEP0XXX: http finished
ronag marked this conversation as resolved.
Show resolved Hide resolved
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/28679
description: Documentation-only deprecation.
-->

Type: Documentation-only

[`response.finished`][] indicates whether [`response.end()`] has been
ronag marked this conversation as resolved.
Show resolved Hide resolved
called and [`response.writableEnded`][] is `true`, not whether the underlying
ronag marked this conversation as resolved.
Show resolved Hide resolved
data has been flushed, `'finish'` has been emitted and
[`response.writableFinished`][] is `true`.
Copy link
Member

Choose a reason for hiding this comment

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

This description is somewhat difficult to read for me. I might not have enough knowledge about the exact functionality but without further context, it seems quite hard to follow.

I understand the first part of the sentence but the second part is not absolute clear to me. Especially due to the and. What about:

[`response.finished`][] indicates whether [`response.end()`] has been
called and [`response.writableEnded`][] is `true`. It does not indicate
whether the underlying data has been flushed or if `'finish'` has been
emitted.

Copy link
Member Author

@ronag ronag Nov 19, 2019 •
edited

Choose a reason for hiding this comment

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

response.writableEnded should be true after respone.end().

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but that does not seem immediately clear for me from the current description.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, any suggestion on improvement?

Copy link
Member

Choose a reason for hiding this comment

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

I am sorry but I do not know the underlying behavior well enough to give a recommendation in this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about now?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds better to me! Just the and still confuses me. Should it not be: ... has been emitted _or_ response.writeableFinished ...?

Copy link
Member Author

Choose a reason for hiding this comment

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

writeableFinished is set to true at the same time as 'finish' is emitted. I can remove one of them to make it more ambiguous and clearer at the same time :D.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about now?


Use [`response.writableFinished`][] or [`response.writableEnded`][]
accordingly instead to avoid the ambigiuty.

To maintain existing functionality use [`response.writableEnded`][].
ronag marked this conversation as resolved.
Show resolved Hide resolved
ronag marked this conversation as resolved.
Show resolved Hide resolved

[`--pending-deprecation`]: cli.html#cli_pending_deprecation
[`--throw-deprecation`]: cli.html#cli_throw_deprecation
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
Expand Down Expand Up @@ -2576,6 +2597,9 @@ Prefer [`response.socket`][] over [`response.connection`] and
[`request.connection`]: http.html#http_request_connection
[`response.socket`]: http.html#http_response_socket
[`response.connection`]: http.html#http_response_connection
[`response.finished`]: #http_response_finished
[`response.writableFinished`]: #http_response_writablefinished
[`response.writableEnded`]: #http_response_writableended
[`script.createCachedData()`]: vm.html#vm_script_createcacheddata
[`setInterval()`]: timers.html#timers_setinterval_callback_delay_args
[`setTimeout()`]: timers.html#timers_settimeout_callback_delay_args
Expand Down
8 changes: 8 additions & 0 deletions doc/api/http.md
View file
Open in desktop
Expand Up @@ -603,8 +603,11 @@ is finished.
### request.finished
<!-- YAML
added: v0.0.1
deprecated: REPLACEME
-->

> Stability: 0 - Deprecated. Use [`request.writableEnded`][].

* {boolean}

The `request.finished` property will be `true` if [`request.end()`][]
Expand Down Expand Up @@ -1202,8 +1205,11 @@ is finished.
### response.finished
<!-- YAML
added: v0.0.2
deprecated: REPLACEME
-->

> Stability: 0 - Deprecated. Use [`response.writableEnded`][].

* {boolean}

The `response.finished` property will be `true` if [`response.end()`][]
Expand Down Expand Up @@ -2258,12 +2264,14 @@ not abort the request or do anything besides add a `'timeout'` event.
[`request.socket.getPeerCertificate()`]: tls.html#tls_tlssocket_getpeercertificate_detailed
[`request.socket`]: #http_request_socket
[`request.writableFinished`]: #http_request_writablefinished
[`request.writableEnded`]: #http_request_writableended
[`request.write(data, encoding)`]: #http_request_write_chunk_encoding_callback
[`response.end()`]: #http_response_end_data_encoding_callback
[`response.getHeader()`]: #http_response_getheader_name
[`response.setHeader()`]: #http_response_setheader_name_value
[`response.socket`]: #http_response_socket
[`response.writableFinished`]: #http_response_writablefinished
[`response.writableEnded`]: #http_response_writableended
[`response.write()`]: #http_response_write_chunk_encoding_callback
[`response.write(data, encoding)`]: #http_response_write_chunk_encoding_callback
[`response.writeContinue()`]: #http_response_writecontinue
Expand Down
4 changes: 4 additions & 0 deletions doc/api/http2.md
View file
Open in desktop
Expand Up @@ -3033,8 +3033,11 @@ is finished.
#### response.finished
<!-- YAML
added: v8.4.0
deprecated: REPLACEME
-->

> Stability: 0 - Deprecated. Use [`response.writableEnded`][].
ronag marked this conversation as resolved.
Show resolved Hide resolved

* {boolean}

Boolean value that indicates whether the response has completed. Starts
Expand Down Expand Up @@ -3517,6 +3520,7 @@ following additional properties:
[`response.end()`]: #http2_response_end_data_encoding_callback
[`response.setHeader()`]: #http2_response_setheader_name_value
[`response.socket`]: #http2_response_socket
[`response.writableEnded`]: #http2_response_writableended
[`response.write()`]: #http2_response_write_chunk_encoding_callback
[`response.write(data, encoding)`]: http.html#http_response_write_chunk_encoding_callback
[`response.writeContinue()`]: #http2_response_writecontinue
Expand Down