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

Rewrite ReadableStream.pipeTo() using the public API #99

Merged
merged 20 commits into from
Oct 27, 2021

Conversation

MattiasBuelens
Copy link
Owner

@MattiasBuelens MattiasBuelens commented Oct 26, 2021

Following up on #98, this re-implements ReadableStream.pipeTo() (and by extension .pipeThrough()) to use the public API of ReadableStream and WritableStream, rather than using internal abstract ops.

This opens the possibility to call readable.pipeTo(writable) with a native WritableStream. Because we no longer use abstract ops to interact with the WritableStream, any implementation with a compliant getWriter() method should do. Note that this isn't possible yet, because we still need to loosen the brand checks (e.g. IsWritableStream()).

Unfortunately we have to accept a few more WPT test failures:

  • This test "cheats". Internally, pipeTo() releases its reader lock while there is still a pending read() request. The public API (reader.releaseLock()) guards against this and throws an error, but the internal ReadableStreamGenericReaderRelease abstract op doesn't actually check this. See Reject pending reads when releasing reader whatwg/streams#1168 for the proposed fix in the specification. For now, I've added a modified unit test that also enqueues a chunk in order to unblock the read().
  • This test and this one deal with a race between an errored readable stream and an erroring/closing writable stream. I'm still investigating what's going on here, and if we can fix the polyfill or add a modified test with slightly tweaked expectations. I've added modified tests.

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

Successfully merging this pull request may close these issues.

None yet

1 participant