Skip to content

lib: narrow ReadableStreamBYOBRequest.view return type to Uint8Array - #62958

Closed
Jah-yee wants to merge 1 commit into
nodejs:mainfrom
Jah-yee:fix/readablestreambyobrequeest-view-type
Closed

lib: narrow ReadableStreamBYOBRequest.view return type to Uint8Array#62958
Jah-yee wants to merge 1 commit into
nodejs:mainfrom
Jah-yee:fix/readablestreambyobrequeest-view-type

Conversation

@Jah-yee

Copy link
Copy Markdown
Contributor

Description

Follow WHATWG streams spec update: whatwg/streams#1367

ReadableStreamBYOBRequest.view is always constructed as a Uint8Array, and the specification will be updated to enforce this. This changes the documented return type from ArrayBufferView to Uint8Array.

Fix

Changed the JSDoc type annotation for view getter from @type {ArrayBufferView} to @type {Uint8Array} in lib/internal/webstreams/readablestream.js.

Fixes#62952

Follow WHATWG streams spec update:
whatwg/streams#1367
ReadableStreamBYOBRequest.view is always constructed as a Uint8Array.
This changes the documented return type from ArrayBufferView to Uint8Array.
Fixes: nodejs#62952
@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Apr 25, 2026
@Jah-yee

Copy link
Copy Markdown
ContributorAuthor

👋 Checking in — PR is a clean 1-line JSDoc type fix per WHATWG streams spec update. Happy to address any feedback or add tests. Thanks! 🙏

@Jah-yee

Copy link
Copy Markdown
ContributorAuthor

PR still active and ready for review. 🙏

@gurgunday

Copy link
Copy Markdown
Member

Hi, thanks for the PR

Please stop spamming, the maintainers will review it when they are free

@codecov

codecovBot commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (42a154b) to head (fb1859c).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #62958 +/- ##
==========================================
+ Coverage 89.64% 89.66% +0.01% 
==========================================
Files 706 706 Lines 219394 219394 Branches 42066 42066 ==========================================
+ Hits 196674 196710 +36 + Misses 14629 14584 -45 - Partials 8091 8100 +9 
Files with missing linesCoverage Δ
lib/internal/webstreams/readablestream.js98.54% <100.00%> (ø)

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MattiasBuelensMattiasBuelens added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Apr 28, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 28, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Jah-yee

Copy link
Copy Markdown
ContributorAuthor

Hi @MattiasBuelens, thank you for the approval! I noticed there are 2 failing CI checks:

  1. lint-commit-message - FAILURE
  2. test-macOS - FAILURE

The lint failure may be blocking the merge. Please let me know if you'd like me to address anything. I'm happy to help rebase if needed.

@MattiasBuelens

MattiasBuelens commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Indeed, the linter flagged this problem with the commit message:

not ok 4 line-length: Line should be <= 72 columns.

It looks like you'll need to slightly tweak the wording and/or word wrapping in the message. You'll want to use git commit --amend to do that. Have a look at our commit message guidelines for more details.

@Jah-yee

Jah-yee commented Apr 28, 2026

Copy link
Copy Markdown
ContributorAuthor

Replaced by updated PR with corrected commit message (lint commit message fix per maintainer feedback). New PR: #63017

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

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

webstreams: narrow ReadableStreamBYOBRequest.view to Uint8Array

5 participants

@Jah-yee@gurgunday@nodejs-github-bot@MattiasBuelens@xy020311