Repository navigation
Simpler stream creation #102
Description
Activity
I like it. Gets us closer to through2 streams as well.
We'd discussed this back in the 0.10 days, but never implemented it.
It's also similar to how WHATWG streams work, from what I can see from the examples.
Obviously adding to the prototype would be desirable for optimisation.
Anyway thanks for the feedback, I wanted to get some opinions before implementing anything 😃
This is quite elegant.
On Sun Jan 25 2015 at 12:32:08 PM Sam Newman notifications@github.com
wrote:It's also similar to how WHATWG streams work, from what I can see from the
examples.Obviously adding to the prototype would be desirable for optimisation.
Anyway thanks for the feedback, I wanted to get some opinions before
implementing anything [image: 😃]—
Reply to this email directly or view it on GitHub
#102 (comment)
.transform likely needs an optional flush
var transform = new stream.Transform({ transform: function (data, enc, next) { }, flush: function (done) { } })duplex and writeable need an optional writev
var writable = new stream.Writable({ write: function (data, enc, next) { }, writev: function (chunks, next) { } }) var duplex = new stream.Duplex({ read: function (n) { }, write: function (data, enc, next) { }, writev: function (chunks, next) { } })but yes I like this idea
@calvinmetcalf yes this is very true, thanks for the reminder 😄
If we go the route of simpler stream instantiation, I'd like to support the revealing-constructor approach from WHATWG streams:
var readable = new Readable({ start: (enqueue, ready) => {}, pull: (enqueue, n, end) => {} // ... });
Adding
enqueue(separate fromend) would also give us a lever with which to coax new features into streams without affecting.push()– this could be a good mechanism to introduce things like in-alphabetnull/undefined,objectModedefault, or dropping thenbyte requirement from._read.Oh yeah – almost forgot! /cc @iojs/streams
@chrisdickinson this is was what I was hoping to allude too 😄
This would give us a way to introduce these things into streams, without breaking backwards compatibility.
I'm not sure exactly on semantics of
startandpullbut could they be triggered inside of_readmethod semtantics for instance?For node / io.js internals it is going to be more advantageous to inherit from prototypes since the memory reuse. Does WHATWG streams utilise the
prototypein the same way?For node / io.js internals it is going to be more advantageous to inherit from prototypes since the memory reuse. Does WHATWG streams utilise the prototype in the same way?
I don't really understand this question, but here is a how things typically work:
Given the code at https://streams.spec.whatwg.org/#example-both, plus:
const source = new WebSocketSource(ws); const readable = new ReadableStream(source);
you will have:
readable.read === ReadableStream.prototype.read, etc. for all public API methodsreadable._underlyingSource === sourcereadable._underlyingSource.start === WebSocketSource.prototype.start, etc.
so in no cases will there be methods that are not on some prototype.
I'm not sure exactly on semantics of start and pull but could they be triggered inside of _read method semtantics for instance?
WRT to
start, that's mostly for implementers wrapping a flowing underlying resource, so that they can set up listeners on the resource andenqueueany incoming data (as I understand it.)pullis analogous to_read, in that it gets called when a stream needs to fetch data from an underlying resource in order to fulfill a.read()request.For node / io.js internals it is going to be more advantageous to inherit from prototypes since the memory reuse. Does WHATWG streams utilise the prototype in the same way?
They do not use the prototype, though I think the overhead from adding a new function per-stream is relatively small, and for folks implementing subclasses I'm fairly certain there are ways to reuse the
pushandstartfunction definitions separate from attaching them to the.prototype.+1 from me, I'd like to see a PR for this!
While we're at it, how about we add
new-less constructors to get even closer to idiomatic Node code (if (!(this instanceof ReadableStream)) return new ReadableStream(whatever)). Basically making through2 redundant.@rvagg we already to that
https://github.com/iojs/readable-stream/blob/master/lib/_stream_transform.js#L95-L96
https://github.com/iojs/readable-stream/blob/master/lib/_stream_readable.js#L116-L117
https://github.com/iojs/readable-stream/blob/master/lib/_stream_writable.js#L154-L155
https://github.com/iojs/readable-stream/blob/master/lib/_stream_duplex.js#L37-L38doh! I did a quick sanity check before posting that but obviously was too quick about it
They do not use the prototype, though I think the overhead from adding a new function per-stream is relatively small, and for folks implementing subclasses I'm fairly certain there are ways to reuse the push and start function definitions separate from attaching them to the .prototype.
Do we have a stream benchmark? We probably wanna make sure that (whatever) approach is taken is easy optimizable by v8
@domenic thanks, I think you answered my question, it could have definitely been worded better. I think I really need to start using WHATWG streams from the reference implementation to understand properly how they work.
@chrisdickinson ahh so
pullalmost the equivalent to_read.@mafintosh I think allowing this functionality would be OK for high-level users (i.e. application developers) without too much of a need for speed. But in terms of core internals that's obviously a different matter.
This is an extremely noddy test http://jsperf.com/proto-vs-constructor-set-methods.
9 remaining items
IMO, providing both
readandpullshould throw an exception.someone could also potentially set the methods on a constructed stream too:
var readable = new Readable() readable._read = n => readable.push(/* data */) readable._pull = enqueue => enqueue(/* data */)
(sorry i'm just over analysing this from all angles)
I guess what i'm thinking is that if
pullwas in someway an implementation on top of_readthat would not be an issue. But if it wasn't there might be a conflict of behaviour.@mafintosh you are absolutely right, it is not directly.
I guess I am just playing devils advocate and thought I'd share my thoughts since these questions hadn't really occurred to me before.
@sonewman So, I suppose what I'd want to do – assuming we can do so without unduly breaking browserify – is: when we receive a
pullfunction, we poison the stream's._readproperty such that setting it throws an exception.@chrisdickinson that's a great answer! 😈
No further questions.
- added a commit that references this issue
on Feb 11, 2015 This passed, and was merged into io.js core.
- added 2 commits that reference this issue
on Jul 20, 2015
I have mentioned this in a couple of other threads before, I just wanted to get some thoughts and opinions really.
My suggestion is that we allow the
_methods, which need to be set on a stream, be passed in as part of the options on instantiation. e.g.edit - functions in ES6 for readability
What do you think?