stream: allow pass stream class to `stream.compose` #50187

himself65 · 2023-10-14T22:05:57Z

class A extends Transform {
  // ...
}

stream.compose(A) // old behavior: A will be called as a normal function and nothing will happen

Fixes: #50176

nodejs-github-bot · 2023-10-14T22:06:02Z

Review requested:

@nodejs/streams

nodejs-github-bot · 2023-10-15T06:11:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54804/

nodejs-github-bot · 2023-10-15T19:22:34Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54834/

koshic · 2023-10-16T14:48:53Z

lib/internal/streams/duplexify.js

@@ -85,6 +85,11 @@ module.exports = function duplexify(body, name) {
  if (typeof body === 'function') {
    const { value, write, final, destroy } = fromAsyncGen(body);

+    // Body might be a constructor function instead of an async generator function.
+    if (isDuplexNodeStream(value)) {


should we reflect this in the docs?

Hard to say, since this is for internal use and the way of init is not decently

It's used for public API Duplex.from, whose behavior will also be slightly changed

node/lib/internal/streams/duplex.js

Lines 142 to 147 in 356b4a2

Duplex.from = function(body) {

if (!duplexify) {

duplexify = require('internal/streams/duplexify');

}

return duplexify(body, 'body');

};

Duplex.from is still an experimental API. I will put it into doc later

Stable pipeline also affected

node/lib/internal/streams/pipeline.js

Lines 288 to 298 in 4032ad5

if (typeof stream === 'function') {

ret = stream({ signal });

if (!isIterable(ret)) {

throw new ERR_INVALID_RETURN_VALUE(

'Iterable, AsyncIterable or Stream', 'source', ret);

}

} else if (isIterable(stream) || isReadableNodeStream(stream) || isTransformStream(stream)) {

ret = stream;

} else {

ret = Duplex.from(stream);

}

I will update doc later

nodejs-github-bot · 2023-10-16T22:16:59Z

Commit Queue failed

- Loading data for nodejs/node/pull/50187
✔  Done loading data for nodejs/node/pull/50187
----------------------------------- PR info ------------------------------------
Title      stream: allow pass stream class to `stream.compose` (#50187)
Author     Alex Yang  (@himself65)
Branch     himself65:himself65/1014/spec -> nodejs:main
Labels     author ready, needs-ci
Commits    2
 - stream: allow pass stream class to `stream.compose`
 - fixup! fix: lint
Committers 1
 - Alex Yang 
PR-URL: https://github.com/nodejs/node/pull/50187
Fixes: https://github.com/nodejs/node/issues/50176
Reviewed-By: Moshe Atlow 
Reviewed-By: Robert Nagy 
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50187
Fixes: https://github.com/nodejs/node/issues/50176
Reviewed-By: Moshe Atlow 
Reviewed-By: Robert Nagy 
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 14 Oct 2023 22:05:57 GMT
   ✔  Approvals: 3
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/50187#pullrequestreview-1678677911
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/50187#pullrequestreview-1678678064
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/50187#pullrequestreview-1679839150
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-10-15T19:22:34Z: https://ci.nodejs.org/job/node-test-pull-request/54834/
- Querying data for job/node-test-pull-request/54834/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 50187
From https://github.com/nodejs/node
 * branch                  refs/pull/50187/merge -> FETCH_HEAD
✔  Fetched commits as 4032ad593c0d..83cf4248107a
--------------------------------------------------------------------------------
[main 843d25cb7c] stream: allow pass stream class to `stream.compose`
 Author: Alex Yang 
 Date: Sat Oct 14 17:04:43 2023 -0500
 2 files changed, 15 insertions(+)
[main 52a91a55af] fixup! fix: lint
 Author: Alex Yang 
 Date: Sat Oct 14 17:43:19 2023 -0500
 2 files changed, 2 insertions(+), 2 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

stream: allow pass stream class to stream.compose
PR-URL: #50187

Fixes: #50176

Reviewed-By: Moshe Atlow moshe@atlow.co.il

Reviewed-By: Robert Nagy ronagy@icloud.com

Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
[detached HEAD 92ba1de03d] stream: allow pass stream class to stream.compose

Author: Alex Yang himself65@outlook.com

Date: Sat Oct 14 17:04:43 2023 -0500

2 files changed, 15 insertions(+)

Rebasing (3/4)

Rebasing (4/4)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

fixup! fix: lint
PR-URL: #50187

Fixes: #50176

Reviewed-By: Moshe Atlow moshe@atlow.co.il

Reviewed-By: Robert Nagy ronagy@icloud.com

Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
[detached HEAD 81e52e6e8e] fixup! fix: lint

Author: Alex Yang himself65@outlook.com

Date: Sat Oct 14 17:43:19 2023 -0500

2 files changed, 2 insertions(+), 2 deletions(-)
Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/6539860160

PR-URL: #50187 Fixes: #50176 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

himself65 · 2023-10-16T22:38:41Z

Landed in ef7363e

PR-URL: #50187 Fixes: #50176 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

targos · 2023-10-23T09:22:21Z

Please think about assigning the semver-minor label when you add new features.

Notable changes: doc: * add H4ad to collaborators (Vinícius Lourenço) #50217 esm: * (SEMVER-MINOR) detect ESM syntax in ambiguous JavaScript (Geoffrey Booth) #50096 fs: * (SEMVER-MINOR) add flush option to appendFile() functions (Colin Ihrig) #50095 lib: * (SEMVER-MINOR) add `navigator.userAgent` (Yagiz Nizipli) #50200 stream: * (SEMVER-MINOR) allow pass stream class to `stream.compose` (Alex Yang) #50187 * call helper function from push and unshift (Raz Luvaton) #50173 PR-URL: #50335

PR-URL: nodejs#50187 Fixes: nodejs#50176 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: doc: * add H4ad to collaborators (Vinícius Lourenço) nodejs#50217 esm: * (SEMVER-MINOR) detect ESM syntax in ambiguous JavaScript (Geoffrey Booth) nodejs#50096 fs: * (SEMVER-MINOR) add flush option to appendFile() functions (Colin Ihrig) nodejs#50095 lib: * (SEMVER-MINOR) add `navigator.userAgent` (Yagiz Nizipli) nodejs#50200 stream: * (SEMVER-MINOR) allow pass stream class to `stream.compose` (Alex Yang) nodejs#50187 * call helper function from push and unshift (Raz Luvaton) nodejs#50173 PR-URL: nodejs#50335

PR-URL: #50187 Fixes: #50176 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) #49908 doc: * add H4ad to collaborators (Vinícius Lourenço) #50217 esm: * (SEMVER-MINOR) detect ESM syntax in ambiguous JavaScript (Geoffrey Booth) #50096 * use import attributes instead of import assertions (Antoine du Hamel) #50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) #49869 fs: * (SEMVER-MINOR) add flush option to appendFile() functions (Colin Ihrig) #50095 * add flush option to writeFile() functions (Colin Ihrig) #50009 lib: * (SEMVER-MINOR) add WebSocket client (Matthew Aitken) #49830 stream: * (SEMVER-MINOR) allow pass stream class to `stream.compose` (Alex Yang) #50187 * call helper function from push and unshift (Raz Luvaton) #50173 * optimize Writable (Robert Nagy) #50012 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) #49996 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) #50141 * use default HDO when importModuleDynamically is not set (Joyee Cheung) #49950 wasi: PR-URL: #50682

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) nodejs#49908 doc: * add H4ad to collaborators (Vinícius Lourenço) nodejs#50217 esm: * (SEMVER-MINOR) detect ESM syntax in ambiguous JavaScript (Geoffrey Booth) nodejs#50096 * use import attributes instead of import assertions (Antoine du Hamel) nodejs#50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) nodejs#49869 fs: * (SEMVER-MINOR) add flush option to appendFile() functions (Colin Ihrig) nodejs#50095 * add flush option to writeFile() functions (Colin Ihrig) nodejs#50009 lib: * (SEMVER-MINOR) add WebSocket client (Matthew Aitken) nodejs#49830 stream: * (SEMVER-MINOR) allow pass stream class to `stream.compose` (Alex Yang) nodejs#50187 * call helper function from push and unshift (Raz Luvaton) nodejs#50173 * optimize Writable (Robert Nagy) nodejs#50012 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) nodejs#49996 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) nodejs#50141 * use default HDO when importModuleDynamically is not set (Joyee Cheung) nodejs#49950 wasi: PR-URL: nodejs#50682

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 14, 2023

stream: allow pass stream class to stream.compose

d4cb027

himself65 force-pushed the himself65/1014/spec branch from f0ca58e to d4cb027 Compare October 14, 2023 22:38

himself65 marked this pull request as ready for review October 14, 2023 22:38

himself65 mentioned this pull request Oct 14, 2023

test_runner: refactor spec reporter to generator function #50177

Closed

fixup! fix: lint

83cf424

MoLow approved these changes Oct 15, 2023

View reviewed changes

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2023

ronag approved these changes Oct 15, 2023

View reviewed changes

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2023

himself65 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2023

MoLow added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 16, 2023

aymen94 approved these changes Oct 16, 2023

View reviewed changes

benjamingr approved these changes Oct 16, 2023

View reviewed changes

koshic reviewed Oct 16, 2023

View reviewed changes

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 16, 2023

himself65 closed this Oct 16, 2023

himself65 mentioned this pull request Oct 16, 2023

docs: update api stream.compose #50206

Merged

targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 23, 2023

targos added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 23, 2023

targos mentioned this pull request Oct 23, 2023

v21.1.0 release proposal #50335

Merged

targos mentioned this pull request Nov 12, 2023

v20.10.0 release proposal #50682

Merged

juanarbol mentioned this pull request Dec 6, 2023

2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stream: allow pass stream class to `stream.compose` #50187

stream: allow pass stream class to `stream.compose` #50187

himself65 commented Oct 14, 2023 •

edited

Loading

nodejs-github-bot commented Oct 14, 2023

nodejs-github-bot commented Oct 15, 2023

nodejs-github-bot commented Oct 15, 2023

koshic Oct 16, 2023

himself65 Oct 16, 2023

koshic Oct 16, 2023

himself65 Oct 16, 2023

koshic Oct 16, 2023

himself65 Oct 16, 2023

nodejs-github-bot commented Oct 16, 2023

PR-URL: #50187
Fixes: #50176
Reviewed-By: Moshe Atlow moshe@atlow.co.il
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

PR-URL: #50187
Fixes: #50176
Reviewed-By: Moshe Atlow moshe@atlow.co.il
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

Successfully rebased and updated refs/heads/main.

himself65 commented Oct 16, 2023

targos commented Oct 23, 2023

	Duplex.from = function(body) {
	if (!duplexify) {
	duplexify = require('internal/streams/duplexify');
	}
	return duplexify(body, 'body');
	};

	if (typeof stream === 'function') {
	ret = stream({ signal });
	if (!isIterable(ret)) {
	throw new ERR_INVALID_RETURN_VALUE(
	'Iterable, AsyncIterable or Stream', 'source', ret);
	}
	} else if (isIterable(stream) \|\| isReadableNodeStream(stream) \|\| isTransformStream(stream)) {
	ret = stream;
	} else {
	ret = Duplex.from(stream);
	}

stream: allow pass stream class to stream.compose #50187

stream: allow pass stream class to stream.compose #50187

Conversation

himself65 commented Oct 14, 2023 • edited Loading

nodejs-github-bot commented Oct 14, 2023

nodejs-github-bot commented Oct 15, 2023

nodejs-github-bot commented Oct 15, 2023

koshic Oct 16, 2023

Choose a reason for hiding this comment

himself65 Oct 16, 2023

Choose a reason for hiding this comment

koshic Oct 16, 2023

Choose a reason for hiding this comment

himself65 Oct 16, 2023

Choose a reason for hiding this comment

koshic Oct 16, 2023

Choose a reason for hiding this comment

himself65 Oct 16, 2023

Choose a reason for hiding this comment

nodejs-github-bot commented Oct 16, 2023

PR-URL: #50187 Fixes: #50176 Reviewed-By: Moshe Atlow moshe@atlow.co.il Reviewed-By: Robert Nagy ronagy@icloud.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

PR-URL: #50187 Fixes: #50176 Reviewed-By: Moshe Atlow moshe@atlow.co.il Reviewed-By: Robert Nagy ronagy@icloud.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

Successfully rebased and updated refs/heads/main.

himself65 commented Oct 16, 2023

targos commented Oct 23, 2023

stream: allow pass stream class to `stream.compose` #50187

stream: allow pass stream class to `stream.compose` #50187

himself65 commented Oct 14, 2023 •

edited

Loading

PR-URL: #50187
Fixes: #50176
Reviewed-By: Moshe Atlow moshe@atlow.co.il
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

PR-URL: #50187
Fixes: #50176
Reviewed-By: Moshe Atlow moshe@atlow.co.il
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com