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.

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

Callback for async _read #113

Closed
calvinmetcalf opened this issue Feb 13, 2015 · 6 comments
Closed

Callback for async _read #113

calvinmetcalf opened this issue Feb 13, 2015 · 6 comments

Comments

@calvinmetcalf
Copy link
Contributor

Idea: pass a second argument to _read that signals to stream that the function is done producing data and should be called again if more data is needed. In other words instead of having to look at the response from this push.

For certain types of data this is a much better setup (for instance finding all the files in a directory and it's children) and is in line with how whatwg streams work iirc.

Doing it backwards compatibly there are a couple paths:

  • use function length and make it async if the length is 2
  • give it a new name (asyncRead? pull?)
  • make it an option (async:true)
@chrisdickinson
Copy link
Contributor

The problem I see with giving _read a callback is that it complicates the logic in the synchronous case: every read that results in a _read would have to create a function + closure and manually track the synchronicity of the function call to ensure that we don't blow out the stack on flow.

@kanongil
Copy link

This is not a new idea, and for a long time this was the way to return data. Eventually it was scrapped, replaced with push. See nodejs/node-v0.x-archive#4878 for some reference.

@calvinmetcalf
Copy link
Contributor Author

calvinmetcalf commented Feb 24, 2015 via email

@kanongil
Copy link

It seems my memory fails me. push was already there when it was removed. The callback was removed to simplify the interface & implementation .

@calvinmetcalf
Copy link
Contributor Author

Was there some discussion somewhere?

On Tue, Feb 24, 2015, 5:48 PM Gil Pedersen [email protected] wrote:

It seems my memory fails me. push was already there when it was removed.
The callback was removed to simplify the interface & implementation .


Reply to this email directly or view it on GitHub
#113 (comment)
.

@mcollina
Copy link
Member

Closing for no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants