-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
stream: allow pass stream class to stream.compose
#50187
Conversation
Review requested:
|
f0ca58e
to
d4cb027
Compare
@@ -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)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we reflect this in the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard to say, since this is for internal use and the way of init is not decently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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'); | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplex.from is still an experimental API. I will put it into doc later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update doc later
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)https://github.com/nodejs/node/actions/runs/6539860160 |
Landed in ef7363e |
Please think about assigning the |
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
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
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
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
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
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) #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) #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) #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
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
Fixes: #50176