-
Notifications
You must be signed in to change notification settings - Fork 77
Commit
Add static methods to TextEncoder and TextDecoder allowing them to vend TransformStream instances. These can then interface with other APIs based on the whatwg streams specification. A stream of strings can be converted to a byte stream, or a byte stream can be converted to a stream of strings. The streams spec can be found at https://streams.spec.whatwg.org/ TransformStream is not yet specified, but a reference implementation exists. This change is not up-to-date with the latest version of the reference implementation API. The API is not yet stable.
- Loading branch information
There are no files selected for viewing
7 comments
on commit 9224c4c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exciting! I guess we couldn't just add {writable, readable} to the TextEncoder instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah!
I guess we couldn't just add {writable, readable} to the TextEncoder instance?
If we make the existing encode() and transform stream interface co-exist, we will either have them work independently or work as two different data input paths for a single processing. I feel that former approach should be implemented in the ricea's way. I can't come up with clear and useful behavior of encode() for the latter. It could be either syntax sugar of writer.write() while there's an active writer, but it doesn't match the current definition of encode(). Making it bypass writer lock sounds bad in terms of the streams design philosophy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jakearchibald That's an interesting alternative. stream.pipeThrough(new TextEncoder())
is a very attractive interface.
I have two concerns I'd like more feedback on:
- Spec complexity. The encoding spec would end up with quite a lot of text basically copy-and-pasted from the definition of TransformStream in the streams spec. Maybe this could be mitigated by "exporting" the necessary methods from the streams spec so that they could be reused in the encoding spec?
- Implementation optimisation cost. I expect implementations will want to optimise TransformStream to bypass the cost of flow-control tracking and buffer-juggling where possible. But if TextEncoder doesn't use TransformStream directly, then it could easily fall through the cracks or require duplicate optimisation effort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What @tyoshino said reminded me why I made .stream() a static method rather than an object method. TextEncoder is okay because it's stateless, but using a single TextDecoder object as a stream transform while also calling the .decode() method is a recipe for confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ricea, refactoring would be nice. Even we don't merge them, we can reduce code duplication to some extent, I guess.
Your (2) might be also a good point regarding implementation details. It should be not impossible to have TransformStream and TextEncoder/Decoder on the same instance, but might lead to some challenge...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for all the feedback! I will update this branch next week.
This week I am busy with something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I agree that decoder.decode()
should fail if either the readable or writable is locked. encoder.encode()
should probably do the same for symmetry.
encoding -> encoder