Skip to content

webstreams: allow shared array buffer sources in writable adapter#62163

Open
Renegade334 wants to merge 1 commit intonodejs:mainfrom
Renegade334:webstream-adapter-writable-buffer-sources
Open

webstreams: allow shared array buffer sources in writable adapter#62163
Renegade334 wants to merge 1 commit intonodejs:mainfrom
Renegade334:webstream-adapter-writable-buffer-sources

Conversation

@Renegade334
Copy link
Member

The webidl buffer source definition includes array buffers as well as array buffer views. Since node streams don't handle the former, #61913 added a transform step to the Writable.toWeb adapter to accept ArrayBuffers.

This PR expands the adapter behaviour to include SharedArrayBuffers – since the adapter already accepts views on shared array buffers, it doesn't make much sense to exclude these asymmetrically from the adapter logic.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Mar 9, 2026
Comment on lines +235 to +237
if (!streamWritable.writableObjectMode && isAnyArrayBuffer(chunk)) {
chunk = new Uint8Array(chunk);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This operation can throw (if chunk is a detached array buffer), so should be within the try/catch block.

@Renegade334 Renegade334 force-pushed the webstream-adapter-writable-buffer-sources branch from ad1a533 to 2d3e275 Compare March 9, 2026 12:18
@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.65%. Comparing base (3725bd2) to head (2d3e275).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62163      +/-   ##
==========================================
- Coverage   89.65%   89.65%   -0.01%     
==========================================
  Files         676      676              
  Lines      206543   206541       -2     
  Branches    39547    39554       +7     
==========================================
- Hits       185184   185175       -9     
- Misses      13480    13499      +19     
+ Partials     7879     7867      -12     
Files with missing lines Coverage Δ
lib/internal/webstreams/adapters.js 86.29% <100.00%> (ø)
lib/internal/webstreams/compression.js 100.00% <100.00%> (ø)

... and 49 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.

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

Labels

needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants