Skip to content

stream: support array of streams in promises pipeline - #40193

Closed
Mesteery wants to merge 4 commits into
nodejs:masterfrom
Mesteery:patch-2
Closed

stream: support array of streams in promises pipeline#40193
Mesteery wants to merge 4 commits into
nodejs:masterfrom
Mesteery:patch-2

Conversation

@Mesteery

@MesteeryMesteery commented Sep 23, 2021

Copy link
Copy Markdown
Contributor

Fixes: #40191

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 23, 2021
@lpinca

lpinca commented Sep 23, 2021

Copy link
Copy Markdown
Member

It should be already supported

node/lib/internal/streams/pipeline.js

Lines 172 to 174 in fd86dad

if(ArrayIsArray(streams[0])&&streams.length===1){
streams=streams[0];
}
no?

@MesteeryMesteery added the stream Issues and PRs related to the stream subsystem. label Sep 23, 2021
@Mesteery

Mesteery commented Sep 23, 2021•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Since c04d621, promises.pipeline calls directly pipelineImpl:

node/lib/stream/promises.js

Line 14 in c9ed79f

const{pipelineImpl: pl}=require('internal/streams/pipeline');

@lpinca

lpinca commented Sep 23, 2021

Copy link
Copy Markdown
Member

Ah ok. Can you please add a test?

lpinca
lpinca approved these changes Sep 24, 2021
View reviewed changes
Comment threadtest/parallel/test-stream-pipeline.js
@Ayase-252Ayase-252 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 7, 2021
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 7, 2021
@nodejs-github-bot

nodejs-github-bot commented Oct 7, 2021

Copy link
Copy Markdown
Collaborator

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

This was referenced Oct 8, 2021
Open
Open
@nodejs-github-bot

nodejs-github-bot commented Oct 9, 2021

Copy link
Copy Markdown
Collaborator

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

This was referenced Oct 10, 2021
Open
Open
@MesteeryMesteery removed the needs-ci PRs that need a full CI run. label Oct 12, 2021
@github-actionsgithub-actionsBot mentioned this pull request Oct 13, 2021
Open
22 tasks
@Ayase-252

Ayase-252 commented Oct 20, 2021

Copy link
Copy Markdown
Member

Landed in b920a10

Ayase-252 pushed a commit that referenced this pull request Oct 20, 2021
@Mesteery@Ayase-252
stream: support array of streams in promises pipeline
b920a10
Fixes: #40191
PR-URL: #40193
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Mesteery
Mesteery deleted the patch-2 branch October 20, 2021 11:56
targos pushed a commit that referenced this pull request Oct 20, 2021
@Mesteery@targos
stream: support array of streams in promises pipeline
b84f101
Fixes: #40191
PR-URL: #40193
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit that referenced this pull request Oct 20, 2021
@targos
2021-10-20, Version 17.0.1 (Current)
d68dd62
Notable changes:
Fixed distribution for native addon builds
This release fixes an issue introduced in Node.js v17.0.0, where some V8
headers were missing from the distributed tarball, making it impossible
to build native addons. These headers are now included.
#40526
Fixed stream issues
* Fixed a regression in `stream.promises.pipeline`, which was introduced
in version 16.10.0, is fixed. It is now possible again to pass an
array of streams to the function.
#40193
* Fixed a bug in `stream.Duplex.from`, which didn't work properly when
an async generator function was passed to it.
#40499
PR-URL: #40535
@targostargos mentioned this pull request Oct 20, 2021
Merged
targos added a commit that referenced this pull request Oct 20, 2021
@targos
2021-10-20, Version 17.0.1 (Current)
73b2d3b
Notable changes:
Fixed distribution for native addon builds
This release fixes an issue introduced in Node.js v17.0.0, where some V8
headers were missing from the distributed tarball, making it impossible
to build native addons. These headers are now included.
#40526
Fixed stream issues
* Fixed a regression in `stream.promises.pipeline`, which was introduced
in version 16.10.0, is fixed. It is now possible again to pass an
array of streams to the function.
#40193
* Fixed a bug in `stream.Duplex.from`, which didn't work properly when
an async generator function was passed to it.
#40499
PR-URL: #40535
targos pushed a commit that referenced this pull request Nov 4, 2021
@Mesteery@targos
stream: support array of streams in promises pipeline
36d3b12
Fixes: #40191
PR-URL: #40193
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Nov 26, 2021
Merged
1 task
@yume-chanyume-chan mentioned this pull request Mar 14, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpincalpincalpinca approved these changes
@jasnelljasnelljasnell approved these changes
@ronagronagronag approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.streamIssues and PRs related to the stream subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

stream.promises.pipeline doesn't support arrays of streams since node 16.10

7 participants

@Mesteery@lpinca@nodejs-github-bot@Ayase-252@jasnell@ronag@targos