search

MattiasBuelens / web-streams-polyfill

Web Streams, based on the WHATWG spec reference implementation
MIT License
285 stars 29 forks source link

Rewrite TransformStream using the public API #105

Closed MattiasBuelens closed 2 years ago

MattiasBuelens commented 2 years ago

This re-implements TransformStream to use the public API of ReadableStream and WritableStream, rather than using internal abstract ops. The idea is that, in the future, we could construct a polyfill TransformStream on top of a native ReadableStream and/or WritableStream.

This also adds partial support for WritableStreamDefaultController.signal.reason, which we use to detect when writable.abort(reason) is called. We do have to "cheat" a bit here, since AbortSignal.reason may not yet be supported everywhere and web-streams-polyfill does not attempt to polyfill this API by itself.

jimmywarting commented 2 years ago

I'm seeing all this _ prefix everywhere all over the pr and in all the files... why don't you just use # instead?

MattiasBuelens commented 2 years ago

I still ship a build for ES5, so I have to be very careful with what syntax features I can or cannot use. The down-level emit for private fields uses WeakMap, and I'm not 100% sure if that's safe to use everywhere... 😬

I could move everything into a semi-private Symbol("state") or something (like in Node), and replace Symbol with String on platforms that don't support it. Not sure yet. 😛

jimmywarting commented 2 years ago

I'm not sure if typescript can handle that down leveling for you, don't use typescript... but i prefer the god old _ prefix over symbol cuz it's cleaner code.

MattiasBuelens commented 2 years ago

I could throw Babel into the mix and use the loose option instead. That changes the emit to generate a unique property name for the private field instead, which would work on ES5. See demo.

It would make the brand checks nicer though, since you can do #privateField in this instead and be guaranteed that this was constructed by your class. See docs. This ties in with https://github.com/MattiasBuelens/web-streams-polyfill/issues/70#issuecomment-851058462.

Anyway, I'll leave that for another time. 😛