Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Streams3 - First Pass #5865

Merged
merged 1 commit into from
Jul 22, 2013
Merged

Streams3 - First Pass #5865

merged 1 commit into from
Jul 22, 2013

Conversation

isaacs
Copy link

@isaacs isaacs commented Jul 18, 2013

This addresses #5860.

Performance-wise it seems to be on-par with master, and I haven't gone through and worked out any trouble spots with the V8 profiling tools.

The commits are a mess, and will be squashed of course, but the best way to see the high level view of what this enables is to check out https://github.com/isaacs/node/blob/streams3/test/simple/test-stream3-pause-then-read.js. That test would switch a streams2 stream into old-mode, but with streams3, you hit s.pause() and it's right back where it was.

The other semantic change is that adding a readable event handler will not cause readable to be emitted until nextTick. Otherwise, stuff like this is problematic:

function MyReadable() {
  Readable.call(this);
  this.on('readable', function() {
    this.doSomeStuff();
  });
}

var r = new MyReadable();
// never gets called!  Already emitted in ctor
r.on('readable', function() {
  r.read();
});

All other semantics should be identical to master. API should be identical.

Still todo: docs.

Review/feedback please: @jclulow @bnoordhuis @indutny @Raynos @tjfontaine

@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit isaacs/node@e4b98bd has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@a03e638 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@8f196d3 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@a7f9678 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@1e4e3c9 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@d2fbfb1 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@29954af has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@b2de7b4 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit isaacs/node@fe96fc8 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information


* Adding a [`'data'` event][] handler to listen for data.
* Calling the [`resume()`][] method to explicitly open the flow.
* Calling the [`pipe()`][] method to send the data to a [Writable][].

Choose a reason for hiding this comment

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

Looks like so in the doc output:

Calling the [pipe()][] method to send the...

Copy link
Author

Choose a reason for hiding this comment

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

fixed.

Closes nodejs#5860

In streams2, there is an "old mode" for compatibility.  Once switched
into this mode, there is no going back.

With this change, there is a "flowing mode" and a "paused mode".  If you
add a data listener, then this will start the flow of data.  However,
hitting the `pause()` method will switch *back* into a non-flowing mode,
where the `read()` method will pull data out.

Every time `read()` returns a data chunk, it also emits a `data` event.
In this way, a passive data listener can be added, and the stream passed
off to some other reader, for use with progress bars and the like.

There is no API change beyond this added flexibility.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants