Investigate merging RS and RBS #379
Assignees
Labels
Comments
|
Added #373. More work can be made after it. |
|
Discussed at TPAC2015, the group there felt that there would need to be very good reasons if these were to remain separate. The extension to support writing into an |
|
We need to decide this: ReadableByteStream has a new pull controlling mechanism. Should we port this to ReadableStream?:
|
|
Currently the merging work is happening at PR #418. |
I don't quite understand the proposal. Can you give a bit more detail? Would it be additive, or replace the existing mechanism? How does some example code differ? |
|
Sorry for lack of words. Branched into #423 :) |
|
There's one more thing to decide regarding return type of the controllers. See #424. |
domenic
pushed a commit
that referenced
this issue
Mar 25, 2016
## Background We originally designed ReadableByteStream, which included extended features to handle bytes, as a separate class from the ReadableStream, which handles a stream of general objects. While designing the details of ReadableByteStream (see #361), we noticed that we could simplify ReadableStream and ReadableByteStream by moving variables and logic for handling queuing into their controller class (see #379). This turned out to also clarify which part of the logic represents semantic requirements of readable streams, and which part of it is implementing helper logic for easier development of underlying sources. After the above refactoring, we also noticed that ReadableStream and ReadableByteStream share most of their code. So, we merged the ReadableByteStream class into ReadableStream. This has many benefits for developers who don't have to deal with two similar-but-different classes. Instead, the same class is used, with the behavior customized by the underlying source. ## Change summary The new ReadableStream class has two reader acquisition methods, getReader() and getBYOBReader(), with the latter working when BYOB reading is available. Availability of BYOB reading is determined by whether or not the underlying source passed to the ReadableStream had BYOB pulling functionality. This is indicated by a truthy `byob` property of the underlying source. The two readers are named the default reader and BYOB reader. The default reader's read() method takes no argument. The resulting promise of the read() method will be fulfilled with a newly-allocated chunk. The BYOB reader's read() method takes one argument; it must be an ArrayBuffer view. The ReadableStream will fill the passed ArrayBuffer view, and fulfill the returned promise with it. The ArrayBuffer view might be transferred several times, but the same backing memory is always written to. When the byob option is falsy, the underlying source is given a ReadableStreamDefaultController. This class provides methods to enqueue a new chunk and know the status of the queue. Its queuing strategy is configured by the parameters passed to the ReadableStream's constructor. The underlying source can subscribe to know when chunks are drained from the queue by implementing the "pull" method. Only the getReader() method will be functional on the ReadableStream. When the byob option is truthy, the underlying source is given a ReadableStreamBYOBController. In addition to the functionalities of the ReadableStreamDefaultController, this controller provides a getter named `byobRequest` which exposes the oldest outstanding BYOB reading request into which the underlying source can put bytes directly (see #423). Both the getReader() and the getBYOBReader() method will be functional on the ReadableStream. The ReadableStreamBYOBController can be configured to convert read requests from a default reader into BYOB read requests, by automatically allocating a buffer and exposing it via the byobRequest getter. This eases implementation of a reactive underlying source, as shown in one of the new examples. ## Changes included in this commit In addition to the major changes as described above, this commit includes bunch of design /logic/aesthetic changes as follows: ### Changes to existing observable features Although ReadableStream's internals were refactored immensely, its external behavior (when not providing a BYOB source) is almost identical to before, as verified by our extensive unit tests. However, we did make a few changes which are observable: - Changes to the semantics of the controller methods (see #424): - Make controller.close() and controller.enqueue() fail when the stream is not in the readable state - Make controller.enqueue() throw a predefined TypeError, not the stored error - (As a result of these changes, the tests test/pipe-to-options.js, test/readable-streams/general.js, and test/readable-stream-templated.js have been updated.) - Rename ReadableStreamController to ReadableStreamDefaultController - Rename ReadableStreamReader to ReadableStreamDefaultReader ### Changes to byte streams As explained above, byte streams were changed in fairly extensive ways to merge them into the base ReadableStream class. Here we call out a few notable changes from the previous specification text: - Remove auto release feature from the ReadableByteStream - Rename Byob to BYOB - Make the default highWaterMark of the byte source version to 0 - Port the functionality that the start method can delay pulling by returning a pending promise to the ReadableStreamBYOBController - Port the highWaterMark mechanism to ReadableByteStreamController - Rename ReadableByteStreamController to ReadableStreamBYOBController - Correctly update the [[disturbed]] slot in the byte handling logic - read(view) now checks view.byteLength before setting [[disturbed]]
tyoshino
added a commit
that referenced
this issue
Mar 28, 2016
## Background We originally designed ReadableByteStream, which included extended features to handle bytes, as a separate class from the ReadableStream, which handles a stream of general objects. While designing the details of ReadableByteStream (see #361), we noticed that we could simplify ReadableStream and ReadableByteStream by moving variables and logic for handling queuing into their controller class (see #379). This turned out to also clarify which part of the logic represents semantic requirements of readable streams, and which part of it is implementing helper logic for easier development of underlying sources. After the above refactoring, we also noticed that ReadableStream and ReadableByteStream share most of their code. So, we merged the ReadableByteStream class into ReadableStream. This has many benefits for developers who don't have to deal with two similar-but-different classes. Instead, the same class is used, with the behavior customized by the underlying source. ## Change summary The new ReadableStream class has two reader acquisition methods, getReader() and getBYOBReader(), with the latter working when BYOB reading is available. Availability of BYOB reading is determined by whether or not the underlying source passed to the ReadableStream had BYOB pulling functionality. This is indicated by a truthy `byob` property of the underlying source. The two readers are named the default reader and BYOB reader. The default reader's read() method takes no argument. The resulting promise of the read() method will be fulfilled with a newly-allocated chunk. The BYOB reader's read() method takes one argument; it must be an ArrayBuffer view. The ReadableStream will fill the passed ArrayBuffer view, and fulfill the returned promise with it. The ArrayBuffer view might be transferred several times, but the same backing memory is always written to. When the byob option is falsy, the underlying source is given a ReadableStreamDefaultController. This class provides methods to enqueue a new chunk and know the status of the queue. Its queuing strategy is configured by the parameters passed to the ReadableStream's constructor. The underlying source can subscribe to know when chunks are drained from the queue by implementing the "pull" method. Only the getReader() method will be functional on the ReadableStream. When the byob option is truthy, the underlying source is given a ReadableStreamBYOBController. In addition to the functionalities of the ReadableStreamDefaultController, this controller provides a getter named `byobRequest` which exposes the oldest outstanding BYOB reading request into which the underlying source can put bytes directly (see #423). Both the getReader() and the getBYOBReader() method will be functional on the ReadableStream. The ReadableStreamBYOBController can be configured to convert read requests from a default reader into BYOB read requests, by automatically allocating a buffer and exposing it via the byobRequest getter. This eases implementation of a reactive underlying source, as shown in one of the new examples. ## Changes included in this commit In addition to the major changes as described above, this commit includes bunch of design /logic/aesthetic changes as follows: ### Changes to existing observable features Although ReadableStream's internals were refactored immensely, its external behavior (when not providing a BYOB source) is almost identical to before, as verified by our extensive unit tests. However, we did make a few changes which are observable: - Changes to the semantics of the controller methods (see #424): - Make controller.close() and controller.enqueue() fail when the stream is not in the readable state - Make controller.enqueue() throw a predefined TypeError, not the stored error - (As a result of these changes, the tests test/pipe-to-options.js, test/readable-streams/general.js, and test/readable-stream-templated.js have been updated.) - Rename ReadableStreamController to ReadableStreamDefaultController - Rename ReadableStreamReader to ReadableStreamDefaultReader ### Changes to byte streams As explained above, byte streams were changed in fairly extensive ways to merge them into the base ReadableStream class. Here we call out a few notable changes from the previous specification text: - Remove auto release feature from the ReadableByteStream - Rename Byob to BYOB - Make the default highWaterMark of the byte source version to 0 - Port the functionality that the start method can delay pulling by returning a pending promise to the ReadableStreamBYOBController - Port the highWaterMark mechanism to ReadableByteStreamController - Rename ReadableByteStreamController to ReadableStreamBYOBController - Correctly update the [[disturbed]] slot in the byte handling logic - read(view) now checks view.byteLength before setting [[disturbed]]
|
Done e601d69 |
|
Very exciting, congratulations on finishing this @tyoshino! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The work in #361 moves a lot of the interesting work into the controller class, making the outer RBS class fairly simple. RBS becomes a shell that delegates to RBSC.
One possibility this opens up is that we could remove RBS, making RS just a shell which delegates to either one of RSC or RBSC. This could have good ecosystem benefits by not splitting the ecosystem into two different types of streams.
To do this I think we need to:
Related: #312.
The text was updated successfully, but these errors were encountered: