Skip to content

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.

3 participants