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

TransformStream byte streams #616

Open
ricea opened this issue Nov 21, 2016 · 6 comments
Open

TransformStream byte streams #616

ricea opened this issue Nov 21, 2016 · 6 comments

Comments

@ricea
Copy link
Collaborator

ricea commented Nov 21, 2016

@isonmad submitted a pull request #601 to support asymmetric default->byte TransformStream objects. Although only default->default and default->byte transforms are possible at the moment, I expect that byte->default and byte->byte will become useful in the future.

@tyoshino made this comment on #601:

This attempt clarified some issues regarding extension of TransformStream to more variants corresponding to the (current and future) RS/WS variants. So, it's really useful. Thanks. E.g. I think we don't want to make the TransformStreamController be an all-in-one class with disabled methods (e.g. the byobRequest getter). OTOH, we also don't want to have a lot of TransformStream variants defined for each combination.

The TransformStream class is basically a helper for implementing stuff following the transform streams concept and explanation of one reasonable backpressure handling. No one is disallowed to directly use the ReadableXXXStream and WritableXXXStream to build a TransformStream. I guess we shouldn't bother ourselves for maintaining a lot of wrappers.

I agree that objects-to-bytes use cases are not uncommon. But I think we should be careful not to inflate the spec. Can we componentize the TransformStream class to avoid the all-in-one controller and also avoid combinatorial explosion? Such attempt could also result in some additional complexity, but my gut feeling is we should explore that.

I think it would be good to split off discussion into this issue.

@ricea
Copy link
Collaborator Author

ricea commented Nov 21, 2016

Strawman proposal: Can we split the TransformStream controller into a readable controller and a writable controller? The readable controller would be the one that is passed into transform() methods, since it provides enqueue(). Either controller could call methods on the transformer object. Does this even make sense?

@tyoshino
Copy link
Member

I think it's worth trying though it might result in making the interface almost the same as direct use of a raw ReadableStream and WritableStream which may look useless.

I think this wouldn't block ship of pipeTo() and pipeThrough(). I hope this could be done in parallel with a relatively less priority.

@ricea
Copy link
Collaborator Author

ricea commented Nov 22, 2016

My idea is that the interface to transformer-authors or transform-consumers doesn't change: only the internal implementation.

So for default->default transform streams it would look exactly the same as it does today. For default->byte streams the controller interface visible to the transformer would have a byobRequest accessor. For byte->default streams the controller would have the same API as today, but the transformer might have additional methods depending on whether we choose RBYOB, KYOB or something else.

The difference is that if, for example, we added a "char" type in future, would we need to add 5 new controller types to TransformStream, or only 2?

@domenic
Copy link
Member

domenic commented Nov 22, 2016

That sounds attractive, but would it in practice save code? It seems like the "glue" code that comprises the majority of the transform stream implementation kind of needs to know about the various controller types in order to perform WS -> transformation -> RS transfer. We could help things by making more abstract operations into polymorphic methods (like [[Cancel]] and [[Pull]] currently are) but it seems like there would need to be some transform stream code that actually uses the BYOB request and such, right? And that code needs to account for the permutations involved.

@ricea
Copy link
Collaborator Author

ricea commented Nov 24, 2016

I thought about it a bit more, and decided that talking about multiple controllers on a single object was just confusing.

Instead, what I think we should do is rename TransformStreamSource to TransformStreamDefaultSource, and similarly with TransformStreamSink. Then for readable byte streams we add have TransformStreamByteSource. The controller is a thin wrapper around the ReadableStream's controller.

I have a rough idea how the Source and Sink should communicate. The Sink calls a polymorphic method on the Source with a promise while will fulfill when the transform completes. That method returns a promise chained from the passed in promise which will fulfill when the Source is ready for more data.

This all falls apart if the name or arguments of the transform() method need to depend on the type of the ReadableStream.

@jimmywarting

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants