-
Notifications
You must be signed in to change notification settings - Fork 6
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
Should accept any BufferSource as a seed, not just Uint8Array #26
Comments
I think this is the first place (outside of the I am generally in favor of following web platform patterns, and Is this feature of the web platform actually considered good, such that we'd want to carry it forward? Are people intentionally passing Float64Arrays to TextDecoder's |
I can't say it's super common in my experience. It's just been a question of, why make the developer manually convert to Uint8Array if we could instead do it for them? I'm unsure I understand your point about using platform endianness. On the web platform we just extract the bytes using this algorithm: https://webidl.spec.whatwg.org/#dfn-get-buffer-source-copy . It doesn't care about the type of the containing buffer, but instead just looks at the [[ViewedArrayBuffer]], [[ByteOffset]], and [[ByteLength]] properties. I do see that there's an endianness dependence in https://tc39.es/ecma262/multipage/structured-data.html#sec-getvaluefrombuffer but it seems like that dependency would be the same for all wrapping classes. Maybe it's because we're passing Uint8 for the type argument, and you were anticipating that we'd pass a value that differs depending on the typed array? |
Mostly because it's not obvious how to do that in the way the developer was intending. There's the endianness concern as mentioned. Also someone might reasonably expect the conversion to work like Generally speaking, accepting things of the wrong type and trying to guess at the type conversion is a worse experience for developers than making them do the conversion explicitly.
If you construct a BigInt64Array from some 64-bit values, or any other non-single-byte TA kind, the bytes in the underlying buffer will be different on a little-endian vs a big-endian platform. (This falls out of SetValueInBuffer step 6.) So if you just directly extract the resulting bytes from the buffer, they'll be different based on platform endianness. It's hard to demonstrate this without a big-endian device, but you can simulate using the endianness argument of the methods on DataViews, which is not platform dependent: let x = new ArrayBuffer(8);
(new DataView(x)).setBigUint64(0, 0x0123456789ABCDEFn, /* isLittleEndian */ true);
console.log([...new Uint8Array(x)]); // [239, 205, 171, 137, 103, 69, 35, 1]
(new DataView(x)).setBigUint64(0, 0x0123456789ABCDEFn, /* isLittleEndian */ false);
console.log([...new Uint8Array(x)]); // [1, 35, 69, 103, 137, 171, 205, 239]
Endianness doesn't affect Int8Array or Uint8Array (or Uint8ClampedArray), since those are just a single byte per element. But it does affect all the other TA kinds. |
So it sounds like taking an |
Int8Array and Uint8ClampedArray don't really come up in practice, but I guess we might as well. There's also DataView, which is basically just a wrapper for an ArrayBuffer (with an offset/length, like a TypedArray). It is extremely weird to me that the web platform allows you to use a DataView as a BufferSource, but it doesn't suffer from endianness issues either, so... I mean I guess? |
I think following web platform precedent and accepting any views, regardless of endianness issues, would be better. But since it's not likely to be a big deal in practice, and we can always loosen it later, this isn't a huge deal... |
The explainer doesn't currently explain the input types in much detail, but I wanted to flag that we should follow the usual web platform pattern of accepting any typed array,
DataView
, orArrayBuffer
, i.e. aBufferSource
.TBD whether
SharedArrayBuffer
-backed buffers should be accepted.The bytes can then be extracted and exposed (modulo #25) as a
Uint8Array
.The text was updated successfully, but these errors were encountered: