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

Add bytes() method for reading bytes into a Uint8Array #369

Closed
wants to merge 1 commit into from

Conversation

bakkot
Copy link

@bakkot bakkot commented May 10, 2024

Edit: Replaced by #370.


The Fetch API is getting a Uint8Array-returning bytes() method alongside its existing arrayBuffer() method, following the principle that APIs should generally vend byte buffers as Uint8Arrays.

This PR makes the same change for PushMessageData, which has its own distinct arrayBuffer method.

I'm assuming this is uncontroversial given the support from the three major implementations for doing this on Body, but I can open an issue and solicit explicit support separately if you'd prefer. I'll write tests if I get a signal that this is able to go forward.

It's unfortunate that getKey and applicationServerKey vend ArrayBuffers instead of Uint8Arrays, but it's too late to fix those now.

  • Modified Web platform tests (link to pull request)

Implementation commitment:


Preview | Diff

@bakkot
Copy link
Author

bakkot commented May 13, 2024

I couldn't find any existing tests to extend here, presumably due to #365. So I haven't written any tests for this. Happy to do so if there's a way to do it.

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine. IPR commitments, less so. The IPR bot points to the necessary instructions.

@marcoscaceres
Copy link
Member

Pinging @martinthomson and @beverloo for input and additional implementer support.

This is implemented in WebKit (thanks @annevk)

@bakkot, argh... small blocker on IPR, as this is substantive contribution. I'll need you to "make a non-member patent licensing commitment" unless you somehow join the working group. I'll see how we make that as painless as possible (hi @siusin, can you help us!).

@bakkot
Copy link
Author

bakkot commented May 20, 2024

Signing such a commitment may be problematic since I do work for a web technologies company which is not a W3C member, even though this work was not done on their behalf. Given that this PR is basically trivial and involved no decisions, I'd prefer not to bother the lawyers just for this. I am happy to close it and let a member make an identical PR if it really can't be marked as non-substantive.

@marcoscaceres
Copy link
Member

@bakkot, ok... sounds good.

Coincidently, I had the same idea! 😜
#370

@bakkot
Copy link
Author

bakkot commented May 21, 2024

Sweet, thanks.

Closing in favor of #370.

@bakkot bakkot closed this May 21, 2024
@bakkot bakkot deleted the bytes-method branch May 21, 2024 04:14
@marcoscaceres
Copy link
Member

Thanks for doing the actual work bakkot!

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

Successfully merging this pull request may close these issues.

None yet

3 participants