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.tee() using the public API #98

Merged
merged 7 commits into from
Oct 26, 2021
Merged

Conversation

MattiasBuelens
Copy link
Owner

@MattiasBuelens MattiasBuelens commented Oct 26, 2021

We want to make the polyfill compatible with native ReadableStream objects (#20), e.g. from new Response().body in the browser or from Readable.toWeb() in Node.js. In order to get there, the polyfill must implement all of its utility methods using the public API of ReadableStream (e.g. reader.read()) rather than using its own internal abstract operations (e.g. ReadableStreamDefaultReaderRead()).

This PR handles ReadableStream.tee(). We'll want to do the same with pipeTo(), pipeThrough(), [Symbol.asyncIterator]() etc in follow-up PRs.

Since we now use reader.read() internally, we no longer pass the "patched global" tests. For example, the { done, value } object returned by reader.read() can now have a .then() method if the user defines Object.prototype.then(). I think this is fine though. These tests are intended for browser implementations to ensure user-land code cannot affect the internals of pipeTo() and tee(), but I think it's unreasonable to expect a user-land polyfill to try and isolate itself from other user-land code messing with the global Object.prototype.

This PR also sets up some infrastructure to skip specific tests within a WPT test file. We already have logic to skip an entire test file, but often there's just one single problematic test that we can't run (e.g. because it causes the test to hang). Now, we can replace these individual tests with a dummy that fails immediately, and still run all the other tests in the file. We'll need this when we rewrite e.g. pipeTo().

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.

1 participant