Skip to content

Conversation

@Renegade334
Copy link
Member

@Renegade334 Renegade334 commented Nov 30, 2025

readable.compose() was intended to return the Duplex constructed by stream.compose(), and is documented as such.

However, because it was added as a "stream-returning operator", its output is being passed via Readable.from(), which constructs a new object-mode Readable by wrapping the async iterator of the composed Duplex. This is inefficient in the best case, but causes breakage in a load of others:

  • if the destination stream is writable-only, then the composed stream would be a non-readable Duplex, but gets returned as a readable Readable.
  • if the source stream is a readable/writable Duplex, then the composed stream gets returned as a non-writable Readable, which misses the point of stream composition – the composed stream should be able to write to the head of the pipeline if it's writable, and read from the end if it's readable.
  • the returned stream is always in object mode, clobbering the object mode detection of stream.compose().

This change gets rid of the "operator" semantics for readable.compose(), and makes it a standalone method which returns the unaltered composed Duplex stream from stream.compose().

Fixes: #55203

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@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 Nov 30, 2025
-->

* `stream` {Stream|Iterable|AsyncIterable|Function}
* `stream` {Writable|Duplex|WritableStream|TransformStream|Function}
Copy link
Member Author

Choose a reason for hiding this comment

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

There's no behaviour change here. Technically iterables are valid as "transformers" in stream.compose(), but since they ignore the source stream output entirely, it seems pointless to be documenting them here.

@codecov
Copy link

codecov bot commented Nov 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.52%. Comparing base (83ba6b1) to head (9cc1c88).
⚠️ Report is 73 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60907      +/-   ##
==========================================
+ Coverage   88.51%   88.52%   +0.01%     
==========================================
  Files         703      703              
  Lines      208432   208428       -4     
  Branches    40200    40200              
==========================================
+ Hits       184502   184520      +18     
+ Misses      15957    15909      -48     
- Partials     7973     7999      +26     
Files with missing lines Coverage Δ
lib/internal/streams/operators.js 96.14% <ø> (-0.30%) ⬇️
lib/internal/streams/readable.js 96.26% <100.00%> (+0.05%) ⬆️

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

@Renegade334 Renegade334 force-pushed the stream-compose-no-readablify branch from 47933e0 to 2aa0aa9 Compare December 9, 2025 22:26
@Renegade334 Renegade334 force-pushed the stream-compose-no-readablify branch from 2aa0aa9 to 9cc1c88 Compare December 9, 2025 22:33
@Renegade334 Renegade334 requested review from rluvaton and ronag December 9, 2025 22:36
@Renegade334 Renegade334 added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 10, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 10, 2025
@nodejs-github-bot
Copy link
Collaborator

@Renegade334 Renegade334 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 22, 2025
@Renegade334
Copy link
Member Author

cc @nodejs/streams – believe this is good to go, but would be grateful for some more eyes.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added semver-minor PRs that contain new features and should be released in the next minor version. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Dec 23, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 23, 2025
@nodejs-github-bot nodejs-github-bot merged commit 5e677d6 into nodejs:main Dec 23, 2025
74 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 5e677d6

@Renegade334 Renegade334 deleted the stream-compose-no-readablify branch December 23, 2025 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. semver-minor PRs that contain new features and should be released in the next minor version. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream: readable.compose fails to return a Duplex

4 participants