-
Notifications
You must be signed in to change notification settings - Fork 48
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
New principle: vend byte arrays as Uint8Arrays #480
Conversation
index.bs
Outdated
@@ -1856,6 +1856,19 @@ as the [Promise integration](https://github.com/WebAssembly/js-promise-integrati | |||
is still under development as of today. | |||
</div> | |||
|
|||
<h3 id="uint8array">Vend byte arrays as Uint8Arrays</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good advice, but I'd like to see the reciprocal as well, which is that APIs should take ArrayBufferView
as input, not any specific type.
Nit:
<h3 id="uint8array">Vend byte arrays as Uint8Arrays</h3> | |
<h3 id="uint8array">Output an array of bytes with Uint8Array</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good advice, but I'd like to see the reciprocal as well, which is that APIs should take
ArrayBufferView
as input, not any specific type.
At least one API (encodeInto) takes a Uint8Array specifically, and I don't know that I agree it would be better for it to take an ArrayBufferView
. It seems like a confusion of types for a user to pass a Uint16Array
there, for example. Maybe that only applies when taking an input as a write target rather than just a source.
Also, wouldn't BufferSource be more appropriate? It is more common than ArrayBufferView, I'm pretty sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake, BufferSource was the one I was looking for.
encodeInto has an in-out argument, so perhaps it is a poor example of an input argument.
I'm interested in whether you think that people should be permitted to supply data in a form that best suits them. it seems like ArrayBufferView
endianness issues might cause you to prefer something less likely to tempt that sort of concern, but I see that things like Web Crypto use it happily (noting that I'm quite receptive to arguments of the form "Web Crypto is not an API you should model behaviour on").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were I designing web APIs from scratch, I would take only the narrowest appropriate TypedArray type, and in the case of byte-taking APIs specifically I'd take Uint8Arrays and ArrayBuffers (and no other types), since those both unambiguously represent a sequence of bytes, whereas (e.g.) a Uint16Array does not unambiguously represent a sequence of bytes.
Even leaving aside the issue of endianness, one can store byte values in a Uint16Array, and passing such a thing to a byte-taking API is not going to do what the user presumably expected. Requiring the user to explicitly do either new Uint8Array(uint16ArrayInstance)
or new Uint8Array(uint16ArrayInstance.buffer, uint16ArrayInstance.byteOffset, uint16ArrayInstance.byteLength)
(depending on which of those two behaviors they actually wanted) would be better. More generally, I think we'd all be better off if web platform APIs were much stricter about their input types (and I'm trying to move JS, at least, in that direction).
But I'm not designing web APIs from scratch, and in the world we actually live in, BufferSource is pretty widely used. So I'm neutral on this. I don't really want to be the one to codify "accept BufferSource" as a principle, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess there is a question as to whether we want to tackle this incrementally or offer advice for a variety of things. Shared memory also comes to mind as something that might need some advice.
And yeah, I think accepting BufferSource is probably the way to go, unless there's a very compelling reason not to have parity with the vast majority of existing APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resizable buffers too, come to that.
It's a bit hard to offer advice on those right now because it seems like web platform hasn't entirely figured out whether APIs should take BufferSources backed by shared / resizable buffers. Personally I think almost all places should take them, but [AllowResizable]
and [AllowShared]
are still default-off right now (IIUC just because that was the safe option) and few (if any?) places have been updated to support them.
I suppose starting with a principle that shared/resizable buffers should be accepted could be a way to start moving in that direction. But I think it would be best to do that in a different issue/PR, rather than holding up this one until that's done.
Co-authored-by: Martin Thomson <[email protected]>
Co-authored-by: Martin Thomson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Fixes #463.
Preview | Diff