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

Incremental read: Why only Uint8Array instead of BufferSource? #1724

Open
saschanaz opened this issue Nov 16, 2023 · 12 comments
Open

Incremental read: Why only Uint8Array instead of BufferSource? #1724

saschanaz opened this issue Nov 16, 2023 · 12 comments

Comments

@saschanaz
Copy link
Member

saschanaz commented Nov 16, 2023

What is the issue with the Fetch Standard?

https://fetch.spec.whatwg.org/#incrementally-read-loop

chunk steps, given chunk

  1. Let continueAlgorithm be null.
  2. If chunk is not a Uint8Array object, then set continueAlgorithm to this step: run processBodyError given a TypeError.

But maybe it can accept BufferSource instead?

(See also whatwg/streams#1299)

@annevk
Copy link
Member

annevk commented Nov 16, 2023

I don't really understand why Web Transport does that. That seems like a bug. We decided that we'd use Uint8Array generally for byte buffer representation.

@saschanaz
Copy link
Member Author

Hmm, it makes sense to return Uint8Array but I don't see the point to accept only Uint8Array?

@saschanaz
Copy link
Member Author

The Encoding APIs also accept (AllowShared)BufferSource via TextDecoder[Stream] and emits Uint8Array via TextEncoder[Stream], which makes sense to me. Would be nice to do some IDL crawl and see how many APIs are currently accepting Uint8Array or BufferSource.

@asutherland
Copy link

Would be nice to do some IDL crawl and see how many APIs are currently accepting Uint8Array or BufferSource.

Do the pages https://dontcallmedom.github.io/webidlpedia/names/BufferSource.html and https://dontcallmedom.github.io/webidlpedia/names/Uint8Array.html from https://dontcallmedom.github.io/webidlpedia/names/ accomplish what you want?

@saschanaz
Copy link
Member Author

Thanks Andrew! That clearly shows BufferSource is winning 🥇

@annevk
Copy link
Member

annevk commented Nov 20, 2023

Someone will have to figure out why we made this decision this way as there was a reason and I'd rather not flip on it based on popularity. I suspect you can find the rationale either in this repository, Streams, or the "fetch with streams" repository (not sure what the link is, but it should be linked from issues in this repository).

@saschanaz
Copy link
Member Author

saschanaz commented Nov 21, 2023

The first addition happened in #449, Anne in 2016 said allowing only Uint8Array is the goal in #441 (comment), I don't immediately see the background behind that but I assume maybe that was what implementations did?

@annevk
Copy link
Member

annevk commented Nov 22, 2023

No, it was aspirational. Initially we designed it such that (byte) streams would only emit Uint8Array to consumers. And later we adopted that for the other direction as well. Mainly for consistency I think, but also with some hope that would allow optimizations.

Most things in the platform that accept buffers should indeed accept BufferSource, but they're not dealing with streams.

@saschanaz
Copy link
Member Author

Mainly for consistency I think

Emit Uint8Array, so accept only Uint8Array too, you mean? Kinda make sense but still not consistent with other APIs at this point.

but also with some hope that would allow optimizations.

Do you remember in what way it can allow optimizations? Getting bytes from typed arrays should be straightforward both in JS and C++, so I don't really follow here.

@annevk
Copy link
Member

annevk commented Nov 22, 2023

I don't think byte streams have to be consistent with other APIs necessarily. They're somewhat unique. Also, consistency within seems more important.

@saschanaz
Copy link
Member Author

saschanaz commented Nov 22, 2023

I mean, ReadableByteStreamController accepts ArrayBufferView instead of just Uint8Array: https://streams.spec.whatwg.org/#ref-for-rbs-controller-enqueue (Not BufferSource, hmm.)

Edit: Based on that I still think "Emit Uint8Array, receive more broad types" pattern is used across many specs and thus gives consistency.

@saschanaz
Copy link
Member Author

saschanaz commented Nov 22, 2023

Here is an inconsistency:

// One can enqueue Uint16Array with type: bytes
await new Response(new ReadableStream({
  start(c) {
    c.enqueue(new Uint16Array([0xD55C, 0xAE00]));
    c.close();
  },
  type: "bytes"
})).blob()

// But this fails
await new Response(new ReadableStream({
  start(c) {
    c.enqueue(new Uint16Array([0xD55C, 0xAE00]));
    c.close();
  }
})).blob()

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

No branches or pull requests

3 participants