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

stream: standardize async initialization #29314

Closed
ronag opened this issue Aug 25, 2019 · 3 comments
Closed

stream: standardize async initialization #29314

ronag opened this issue Aug 25, 2019 · 3 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@ronag
Copy link
Member

ronag commented Aug 25, 2019 •
edited
Loading

Some streams (e.g.fs and net) need to first asynchronously create resources before they can perform any work. Currently this is implemented in the different stream implementations which both makes things more difficult and error prone (e.g. 'open' and 'ready' are emitted after 'close'). I suggest we provide a standardized way of achieving this.

This would allow further simplification and improved consistency for various stream implementations such as fs and net stream.

I propose the following API for this:

  • a stream._construct(cb) endpoint (similar to _destroy(cb)).
  • a stream.ready property indicating whether the stream is ready (currently this is called pending in some implementations). Including readableReady and writableReady accessors.
  • a 'ready' event (currently this is called 'open' in some implementations).

See, nxtedition#2 for proof of concept.
@ronag ronag changed the title stream: provide a _construct end-point for streams with async initialization. stream: standardize async initialization. Aug 25, 2019
@ronag ronag changed the title stream: standardize async initialization. stream: standardize async initialization Aug 25, 2019
@ronag
Copy link
Member Author

ronag commented Aug 25, 2019 •
edited
Loading

I suggest in order to land this we first update either net or fs to as far as possible use "standardized" streams (e.g. #29048) and then try to apply this suggestion there to prove the concept.

@Fishrock123 Fishrock123 added the stream Issues and PRs related to the stream subsystem. label Aug 27, 2019
@ronag
Copy link
Member Author

ronag commented Sep 5, 2019

Here is a proposal implementation. Don't think I can get much further without some feedback.

nxtedition#2

@ronag ronag mentioned this issue Sep 5, 2019
Closed
4 tasks
@ronag ronag mentioned this issue Sep 22, 2019
Closed
4 tasks
@Fishrock123 Fishrock123 mentioned this issue Sep 23, 2019
Open
ronag added a commit to nxtedition/node that referenced this issue May 25, 2020
@ronag
stream: construct
2fe1044 
Provide a standardized way of asynchronously creating and
initializing resources before performing any work.

Refs: nodejs#29314
ronag added a commit that referenced this issue May 27, 2020
@ronag
stream: construct
fb8cc72 
Provide a standardized way of asynchronously creating and
initializing resources before performing any work.

Refs: #29314

PR-URL: #29656
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@ronag ronag closed this as completed May 27, 2020
@ronag
Copy link
Member Author

ronag commented May 27, 2020

This has landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Fishrock123 @ronag