Skip to content

stream: corrections to source normalization#63416

Open
Renegade334 wants to merge 1 commit into
nodejs:mainfrom
Renegade334:stream-iter-source-normalization
Open

stream: corrections to source normalization#63416
Renegade334 wants to merge 1 commit into
nodejs:mainfrom
Renegade334:stream-iter-source-normalization

Conversation

@Renegade334
Copy link
Copy Markdown
Member

  • Arrays, as special cases of sync iterables, have lower precedence than async iterables (and protocol objects) in async normalization operations.
  • Sync iterables should be normalized synchronously in async normalization operations, ie. they may not contain nested async iterables (continuation of stream: minor stream/iter implementation edits #63132).
  • Similarly, sync toStreamables should not be allowed to return async sources.
  • No explicit rejection is required for async patterns in fromSync(); doing so may exclude objects with both sync and async functionality (eg. objects with both [Symbol.iterator] and [Symbol.asyncIterator] properties). The spec text seems to suggest that objects with a valid sync pattern should be accepted here even if they also have an async pattern, but could do with some clarity.
  • In from(), normalise the object returned by toAsyncStreamable directly, not the return value of its Symbol.asyncIterator method: firstly, the async iterator returned by the Symbol.asyncIterator method is not itself obliged to have a Symbol.asyncIterator method, and secondly, the result could in theory be any valid from() source, not just an async iterable.
  • Use isUint8ArrayBatch for the from()/fromSync() fast paths.

Note that as things stand, some of the from() cases throw validation errors synchronously (eg. fromSync({ [toStreamable]() {} })) whereas the majority only throw validation errors at the point of iteration. AFAICT, the latter behaviour is what is compliant with the spec, but I haven't touched these for now.

Signed-off-by: Renegade334 <contact.9a5d6388@renegade334.me.uk>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@Renegade334 Renegade334 requested a review from jasnell May 18, 2026 17:45
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels May 18, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

❌ Patch coverage is 81.19658% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.03%. Comparing base (3b19867) to head (5da7742).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/from.js 81.19% 22 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63416      +/-   ##
==========================================
- Coverage   91.68%   90.03%   -1.65%     
==========================================
  Files         361      714     +353     
  Lines      156230   225741   +69511     
  Branches    24021    42710   +18689     
==========================================
+ Hits       143232   203242   +60010     
- Misses      12729    14298    +1569     
- Partials      269     8201    +7932     
Files with missing lines Coverage Δ
lib/internal/streams/iter/from.js 81.47% <81.19%> (-5.36%) ⬇️

... and 476 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. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants