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

start readable stream in paused mode #7

Merged
merged 1 commit into from
Oct 13, 2019

Conversation

achingbrain
Copy link
Contributor

The node docs say:

All Readable streams begin in paused mode but can be switched to flowing mode in one of the following ways:

  • Adding a 'data' event handler.
  • Calling the stream.resume() method.
  • Calling the stream.pipe() method to send the data to a Writable.

This module seems to return a stream that starts emitting data on the next tick and doesn't wait for the 'data' event handler to be added.

This PR:

  1. Starts the stream in paused mode
  2. Overrides the .on method on the stream to start data flowing when a data event is added in the same way that .pipe is overridden
  3. Adds a test for the above

The [node docs say](https://nodejs.org/api/stream.html#stream_two_reading_modes):

> All Readable streams begin in paused mode but can be switched to flowing mode in one of the following ways:
>
> * Adding a 'data' event handler.
> * Calling the stream.resume() method.
> * Calling the stream.pipe() method to send the data to a Writable.

This module seems to return a stream that starts emitting data on the
next tick and doesn't wait for the 'data' event handler to be added.

This PR:

1. Starts the stream in `paused` mode
2. Overrides the `.on` method on the stream to start data flowing when
  a `data` event is added in the same way that `.pipe` is overridden
3. Adds a test for the above
@dominictarr
Copy link
Member

thanks, I'm gonna merge this as a breaking change, I think it probably won't break anything, but this has been out a long time and no one complained. This way, old code will still use the old version of this module, but new users will get the new code.

your test fails by the way

@dominictarr
Copy link
Member

... this was written using an old version of tape that didn't output anything in that error case. updating gives and error, clearing the timeout of the second timeout fixes it.

@dominictarr
Copy link
Member

merged into 2.0.0

@dominictarr dominictarr merged commit 475a494 into master Oct 13, 2019
@achingbrain achingbrain deleted the start-data-flow-when-data-event-is-added branch October 14, 2019 06:41
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

Successfully merging this pull request may close these issues.

2 participants