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

Review request on the ReadableStream API #433

Closed
tyoshino opened this issue Mar 31, 2016 · 10 comments
Closed

Review request on the ReadableStream API #433

tyoshino opened this issue Mar 31, 2016 · 10 comments

Comments

@tyoshino
Copy link
Member

tyoshino commented Mar 31, 2016

/cc @wanderview @hober @othermaciej @jakearchibald @wolenetz @gwicke @annevk @KenjiBaheux @domenic

Hi all, we've finished the last big refactoring around the ReadableStream interface. Only a few editorial and minor naming issues (e.g. #294) (DONE) are remaining.

The up-to-date spec is available at https://streams.spec.whatwg.org/.

You can find usage examples at https://streams.spec.whatwg.org/#rs-intro and https://streams.spec.whatwg.org/#creating-examples.

Briefly speaking, the refactoring made the following changes:

  • (a) addition of byte stream handling API to the existing ReadableStream API
  • (b) the following two minor semantic changes to the ReadableStream API itself
    • 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

Those who are planning to use the Response streaming feature (yutakahirano/fetch-with-streams#63) is affected by (a) (b).

The Media Source Extensions standardization (w3c/media-source#14 (comment)) is affected by (b) (a). They want to get their appendStream() spec updated soon.

So, I'd like you guys to start reviewing the latest ReadableStream API spec and give us your feedback.

Please speak up on this thread.

@tyoshino
Copy link
Member Author

tyoshino commented Apr 8, 2016

... Only a few editorial and minor naming issues (e.g. #294) are remaining.

A PR for this issue is under review: #436

@tyoshino
Copy link
Member Author

/cc @wenbozhu

#294 has been fixed

@tyoshino
Copy link
Member Author

Sorry. The OP contained typos.

  • The Response streaming feature is affected by (b). Not (a).
  • The MSE is affected by (a). Not (b).

I also elaborated (b) by including the list of changes we made on. Please take a look.

@wenbozhu
Copy link

Are controller classes internal to implementations? If so, how do I tell which part of the spec concerns the public API (interfaces)?

@tyoshino
Copy link
Member Author

tyoshino commented Apr 22, 2016

@wenbozhu A ReadableStream can be constructed in scripts by providing their own underlying source and obtaining a ReadableStreamDefaultController/ReadableByteStreamController. I.e. the controllers are exposed to scripts. But for web APIs e.g. the Fetch API, the controller is internal to them. Or even they can omit the controllers and access the ReadableStream's controller-facing [EDITED] interface directly. ReadableStream's public interface is the only interface the users of the web APIs would see.

You can consider the controller stuff as a helper for easier buffering especially for adopting push source.

@tyoshino
Copy link
Member Author

You can check the ReadableStream's controller-facing interface at https://streams.spec.whatwg.org/#rs-abstract-ops-used-by-controllers. These operations are not exposed to web application scripts, but web APIs vending an object implementing the ReadableStream interface may call these internally and directly without letting the object violate the semantics/constraints of the ReadableStream.

@GrumpyMcGrumperson
Copy link

GrumpyMcGrumperson commented Dec 9, 2016

I just stumbled on this Stream spec thing and I'm confused.

Why isn't a byte stream the lowest level building block here? Now it seems "ReadableStream" and "ReadableByteStream" are at the same level of abstraction, even though everything that might go through a stream consists of bytes.

In other words, "byte streams" are actually a lower-level thing than "streams". For example, a TCP connection is a two-way byte stream.

Does this tech involve Node somehow? I hope this is pure web tech. Or at least that's what the world needs - not some sort of hybrid that carries Node's baggage like basically everything in the front-end world these days.

Note also that a "push source" is basically the same thing as an Observable, which itself is conceptually a Stream.

In fact, isn't a "pull source" a Stream too? Pushing and Pulling are just different ways of arranging for stuff to come out of a Stream!

So basically a ReadableStream contains a Stream-that's-not-called-a-Stream? Why wouldn't a ReadableStream just wrap an inner ReadableStream? Streams all the way through. That's the way Java has done it for ages.

I also noticed people talking about reading stuff from a stream, and it looked like you should have just copied what Java does, or come up with something better (if possible).

Check this out: https://docs.oracle.com/javase/7/docs/api/java/io/InputStream.html#read(byte[],%20int,%20int) - it works. It's worked for ages.

This reminds me, why would a GzipStream be a "TransformerStream", with both a Readable- and a WriteableStream? Why would it be necessary?

See what they've done with Java again: https://docs.oracle.com/javase/7/docs/api/java/util/zip/GZIPInputStream.html - it reads gzipped bytes from a wrapped InputStream, and something uses it to read un-gzipped bytes.

Maybe I'm completely missing something here, but I get the feeling that this spec has gone in the wrong direction.

@jakearchibald
Copy link

jakearchibald commented Dec 9, 2016

@GrumpyMcGrumperson

Why isn't a byte stream the lowest level building block here?

A byte stream is a stream that's optimised for chunks of Uint8Arrays, whereas general streams can be of any value (objects, numbers, dates, whatever).

In fact, isn't a "pull source" a Stream too? Pushing and Pulling are just different ways of arranging for stuff to come out of a Stream!

Right! That's why they aren't different types here. See the examples https://streams.spec.whatwg.org/#creating-examples

I also noticed people talking about reading stuff from a stream, and it looked like you should have just copied what Java does

What's bad about the current spec that Java does better?

This reminds me, why would a GzipStream be a "TransformerStream", with both a Readable- and a WriteableStream? Why would it be necessary?

By having a writable endpoint, it can be piped to. Because its a separate property it can be passed to other pieces of code without exposing the whole transform object.

@GrumpyMcGrumperson
Copy link

Alright, carry on! :p

I can't tell if there's a real problem, and I shouldn't use any more time on this thread anyway :)

@ricea
Copy link
Collaborator

ricea commented Dec 4, 2017

I think this issue has served its purpose, so I'm closing it. Feel free to file a new issue for any new concerns.

@ricea ricea closed this as completed Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants