webstreams: allow shared array buffer sources in writable adapter#62163
webstreams: allow shared array buffer sources in writable adapter#62163Renegade334 wants to merge 1 commit intonodejs:mainfrom
Conversation
| if (!streamWritable.writableObjectMode && isAnyArrayBuffer(chunk)) { | ||
| chunk = new Uint8Array(chunk); | ||
| } |
There was a problem hiding this comment.
This operation can throw (if chunk is a detached array buffer), so should be within the try/catch block.
ad1a533 to
2d3e275
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #62163 +/- ##
==========================================
- Coverage 89.79% 89.78% -0.02%
==========================================
Files 697 697
Lines 215773 215773
Branches 41297 41305 +8
==========================================
- Hits 193749 193724 -25
- Misses 14117 14157 +40
+ Partials 7907 7892 -15
🚀 New features to boost your workflow:
|
|
@Renegade334 how is this aligned with the spec and #62632? |
|
This doesn't affect CompressionStream, just the Writable.toWeb adapter. We currently have the scenario where the adapter will silently transform ArrayBuffer to Uint8Array before passing to the node stream (since ArrayBuffer is a valid buffer source in the web spec), but SharedArrayBuffers are not transformed and instead rejected – whereas node streams obviously accept views on SharedArrayBuffers, so there's unnecessary asymmetry. CompressionStream correctly rejects both SABs and views on SABs, and this doesn't change. |
|
Ok, can you rebase your PR and |
Refs: nodejs#61913 Signed-off-by: Renegade334 <contact.9a5d6388@renegade334.me.uk>
2d3e275 to
bb853fc
Compare
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.toWebadapter 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.