node
node copied to clipboard
nodejs
lib: add support for readable byte streams to .toWeb()
Add support for the creation of ReadableByteStream to Readable.toWeb() and Duplex.toWeb() This enables the use of .getReader({ mode: "byob" }) on e.g. socket().toWeb()
e.g.:
const { writable, readable } = Duplex.toWeb(duplex, { type: 'bytes' });
const data = new Uint8Array(dataToRead.length);
readable.getReader({ mode: 'byob' }).read(data).then((result) => {
// do something with result
});
Kind regards, Hans
Refs: https://github.com/nodejs/node/issues/56004#issuecomment-2908265316 Refs: https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Using_readable_byte_streams
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 89.95%. Comparing base (09b4c57) to head (55af65e).
:warning: Report is 916 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #58664 +/- ##
==========================================
- Coverage 90.07% 89.95% -0.13%
==========================================
Files 641 667 +26
Lines 188998 196801 +7803
Branches 37069 38420 +1351
==========================================
+ Hits 170246 177023 +6777
- Misses 11462 12205 +743
- Partials 7290 7573 +283
| Files with missing lines | Coverage Ξ | |
|---|---|---|
| lib/internal/streams/duplex.js | 100.00% <100.00%> (ΓΈ) |
|
| lib/internal/webstreams/adapters.js | 85.59% <100.00%> (+0.36%) |
:arrow_up: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
this lgtm - only thing missing would be a test when the user provides an invalid
typeargument value.
Thx, just added the test on the validity of the type option.
Sorry for the noob question, but do I resolve conflicts in doc/api/stream.md or will the process handle this by itself ? If I need to do this, what is the best way to do this without messing up the git branch ?
@seriousme your best option is to mergen main into this branch. Alternatively, you can do a git rebase. Before doing anything anyway, backup the current branch in another.
@jasnell @mcollina are you both ok with the changes applied to this PR or is there anything more left to do?
Could someone please start CI again? I am assuming no other changes are required for now.
Failed to start CI
β Commits were pushed since the last approving review: β - lib: add support for readable byte streams to .toWeb() β - fix: correct link from webstream to streamduplextowebstreamduplex β - rewrite of test-stream-readable-to-web-byob.js β - add validation of 'type' option β - use internal validators to validate options β - Update lib/internal/webstreams/adapters.js β - add item to doc changes β - applied various changes β - fixed test-stream-readable-to-web-byob.js β - applied review changes to adapters.js β - Merge remote-tracking branch 'upstream/main' into lib-add-toweb-readaβ¦ β - remove colon from description as it breaks the YAML β - add single quotes to 'type' β Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/16175195572
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
CI seems to have passed al checks, can someone please decide to merge or let me know what needs to be changed. I'd rather not have to rebase again just because this PR is sitting idle.
I checked and last nights build failures do not seem related to this PR.
- RHEL 8 on s390 (where rhel 9 on s390 works)
- win11-arm64-COMPILED_BY-vs2022_clang-arm64 (where all other windows builds incl other arm64 works)
Btw: I read in the contribution guide:
If you see that requested changes have been made, you can clear another collaborator's
Changes requestedreview.
Maybe someone can verify that the requested changes were applied? (as far as I can see I followed up on all of them)
@Ethan-Arrowood @jasnell @MattiasBuelens @mcollina You all spent time providing valueable feedback, thanks for that! I think it would be sad if you spent that precious time without seeing a NodeJS improvement as result. As far as I can tell all your wishes have been implemented, so we just need someone to approve and for obvious reasons that can't be me!
Kind regards, Hans ps. if anything still needs to be done I'm more than happy to help out!
Given all the work that you already have done, is there anything we can do to land this PR?
@Ethan-Arrowood @jasnell @MattiasBuelens @mcollina
It feels to me like we are almost there and you all put so much time in it that I think it would be sad to let this PR just slip into oblivion. @mcollina can you please review? If not can you please let us know? If you don't have the time to review, would it be ok if someone else signs off that I implemented your requested changes?
Thanks again for all the effort!
Kind regards, Hans
@mcollina @jasnell @Ethan-Arrowood @MattiasBuelens
Fellow NodeJS enthousiasts, I appreciate that you are all very busy improving NodeJS! What can we do to land this PR and thereby land another improvement? (which someone refered to as "Solid work" π )
Kind regards, Hans ps. I don't need credits or my name on any list or whatever, I just would like to fix this use case so that I can cleanup my own code and help others in the process, in the true spirit of Open Source.