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

Align input sanitization with WebIDL #57

Merged
merged 36 commits into from
Jul 19, 2020
Merged

Align input sanitization with WebIDL #57

merged 36 commits into from
Jul 19, 2020

Conversation

MattiasBuelens
Copy link
Owner

In #52, I updated the polyfill just enough to get all tests passing again. However, there were still a lot of differences between how the polyfill does its input sanitization and how the reference implementation does it.

The reference implementation uses webidl2js to generate wrapper classes which around the actual implementation class, which handle all input sanitization based on the WebIDL. For the polyfill, I decided to write these checks manually, utilizing TypeScript's type system (with custom type guards) where possible.

By writing the type checks and conversions manually, we can also avoid some weirdness from the reference implementation. For example, the spec needs to pass both the original underlyingSource as well as the converted underlyingSourceDict to SetUpReadableStreamDefaultControllerFromUnderlyingSource, so that it can call the source's methods with the correct this context. For the polyfill, I make sure that the converted underlying source already has its methods bound to the original object, so I don't need to worry about this in the rest of the implementation. 🙂

I've also restructured the code to better match the new structure of the reference implementation. It's not a perfect match, for example the reference implementation puts all of the abstract operations for readable streams in a single file, whereas the polyfill puts them in the files that use them.

@MattiasBuelens MattiasBuelens added the domain: compliance Issues related to compliance with the streams standard label Jul 19, 2020
@MattiasBuelens MattiasBuelens added this to the v3.0.0 milestone Jul 19, 2020
@MattiasBuelens MattiasBuelens self-assigned this Jul 19, 2020
@MattiasBuelens MattiasBuelens merged commit 14cbf38 into master Jul 19, 2020
@MattiasBuelens MattiasBuelens deleted the webidl branch July 19, 2020 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: compliance Issues related to compliance with the streams standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant