Skip to content

streams: implement min option for ReadableStreamBYOBReader.read(view)#50888

Merged
nodejs-github-bot merged 11 commits into
nodejs:mainfrom
MattiasBuelens:rs-byob-read-min
Jan 4, 2024
Merged

streams: implement min option for ReadableStreamBYOBReader.read(view)#50888
nodejs-github-bot merged 11 commits into
nodejs:mainfrom
MattiasBuelens:rs-byob-read-min

Conversation

@MattiasBuelens

@MattiasBuelens MattiasBuelens commented Nov 23, 2023

Copy link
Copy Markdown
Contributor

This adds support for ReadableStreamBYOBReader.read(view, { min }). When the min option is given, the read will only be fulfilled as soon as min number of elements are available in the readable byte stream.

Specification: https://streams.spec.whatwg.org/#byob-reader-read
Tests: web-platform-tests/wpt#29723

This is ported pretty much directly from the reference implementation, see whatwg/streams#1145.

Fixes #50706

Todo

  • Add documentation

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Nov 23, 2023
@MattiasBuelens MattiasBuelens changed the title streams: implement min option for ReadableStreamBYOBReader.read(view) streams: implement min option for ReadableStreamBYOBReader.read(view) Nov 23, 2023
Comment thread lib/internal/webstreams/readablestream.js Outdated
Comment thread test/wpt/status/streams.json Outdated
Comment thread test/wpt/status/streams.json Outdated
@MattiasBuelens MattiasBuelens marked this pull request as draft November 23, 2023 22:09
@debadree25

Copy link
Copy Markdown
Contributor

I guess this will need a rebase now that .cancel implementation has landed

@MattiasBuelens MattiasBuelens marked this pull request as ready for review December 31, 2023 13:29
Comment thread doc/api/webstreams.md
Comment thread lib/internal/webstreams/readablestream.js Outdated
Comment thread lib/internal/webstreams/transformstream.js Outdated
Comment thread lib/internal/webstreams/writablestream.js Outdated

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina

mcollina commented Jan 2, 2024

Copy link
Copy Markdown
Member

There seems to be a lot of CI failures

@MattiasBuelens

MattiasBuelens commented Jan 2, 2024

Copy link
Copy Markdown
Contributor Author

There seems to be a lot of CI failures

Yes, I think I accidentally broke some more tests with the stricter validation rules. 😅 Working on it!

@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 2, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 2, 2024
@richardlau

Copy link
Copy Markdown
Member

This doesn't land cleanly on v20.x-staging and will need a manual backport.

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web Streams: support ReadableStreamBYOBReader.prototype.read(view, { min })

9 participants