-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
stream: add bytes() method to stream/consumers #60426
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
base: main
Are you sure you want to change the base?
Conversation
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60426 +/- ##
==========================================
- Coverage 88.58% 88.56% -0.02%
==========================================
Files 704 704
Lines 207823 207867 +44
Branches 40049 40052 +3
==========================================
Hits 184101 184101
- Misses 15772 15816 +44
Partials 7950 7950
🚀 New features to boost your workflow:
|
3887f53 to
4117fc0
Compare
- Add bytes() method to get Uint8Array from streams - Add tests for bytes() method in PassThrough and ObjectMode scenarios - Update documentation Fixes: nodejs#59542
4117fc0 to
b6a5a18
Compare
Co-authored-by: René <[email protected]>
Ethan-Arrowood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM but I'm not explicitly approving it. I don't know why we really need this when user's could do new Uint8Array(await arrayBuffer(stream)); themselves (or rather any conversion from the raw ArrayBuffer).
|
Looking at the issue, I see this is meant to align us with the new I'm okay with that then :) nice work! |
| import { bytes } from 'node:stream/consumers'; | ||
| import { Readable } from 'node:stream'; | ||
|
|
||
| const dataBuffer = Buffer.from('hello world from consumers!'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like lint-md is complaining about this line – you'll need to add an explicit Buffer import from node:buffer to your examples to make it happy!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, thanks for catching that! I've added explicit imports
for Buffer from 'node:buffer' in both the ESM and CommonJS examples.
Local lint verification confirms it passes now.
stream: add bytes() method to stream/consumers
Add a
bytes()method to thestream/consumersmodule that returns a Uint8Array, providing a convenient way to consume streams as typed arrays.Fixes: #59542