Skip to content
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

fs: implement byob mode for readableWebStream() #46933

Merged
merged 3 commits into from
Apr 10, 2023

Conversation

debadree25
Copy link
Member

@debadree25 debadree25 commented Mar 3, 2023

This is an initial draft trying to implement allowing usage of 'byob' mode for readable web streams created from file handles.

Would like some feedback before I start working on tests :-)

Have added tests and doc updates,
the implementation is similar to how the implementation is here https://nodejs.org/api/webstreams.html#class-readablestreambyobreader

Fixes: #45853

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Mar 3, 2023
@debadree25 debadree25 requested a review from jasnell March 3, 2023 11:09
@debadree25

This comment was marked as outdated.

@debadree25 debadree25 closed this Mar 4, 2023
@debadree25 debadree25 reopened this Mar 4, 2023
@debadree25 debadree25 removed the request for review from jasnell March 4, 2023 19:08
@debadree25 debadree25 marked this pull request as ready for review March 4, 2023 19:10
@debadree25 debadree25 requested a review from jasnell March 4, 2023 19:10
@debadree25 debadree25 added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 5, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 5, 2023
@nodejs-github-bot

This comment was marked as outdated.

@debadree25 debadree25 added the review wanted PRs that need reviews. label Mar 6, 2023
@debadree25
Copy link
Member Author

also cc'ing @nodejs/whatwg-stream

@debadree25
Copy link
Member Author

Ping @nodejs/whatwg-stream , @nodejs/streams ?

@nodejs-github-bot
Copy link
Collaborator

@debadree25
Copy link
Member Author

no one to review? 😕
any direction on whether this direction is appropriate or not would also be helpful!

@benjamingr
Copy link
Member

no one to review? 😕 any direction on whether this direction is appropriate or not would also be helpful!

I suspect many people (at least me) are in spring break holidays Mar/Apr (easter/passover/holi/nowruz/songkram etc)

Also, web streams typically don't get a ton of love (that makes your PRs more welcome not less! And it's good that you're tackling them) but most of the node/streams people (at least matteo, ronag and myself) aren't web-stream experts.

@debadree25
Copy link
Member Author

Hey @benjamingr yeah i also thought people are probably busy rn! no problem at all I will leave the PR open for the time being no hurries 😄😄!

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Ah this just implements the ByoB stuff for fs on top of web streams - that should be fine and much smaller to review

@debadree25 debadree25 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 9, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 9, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

danielleadams added a commit that referenced this pull request Jul 12, 2023
Notable changes:

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
deps:
  * update ada to 2.0.0 (Node.js GitHub Bot) #47339
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * drop ICU requirement for parsing hostnames (Yagiz Nizipli) #47339
  * use ada::url_aggregator for parsing urls (Yagiz Nizipli) #47339
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
danielleadams added a commit that referenced this pull request Jul 12, 2023
Notable changes:

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
deps:
  * update ada to 2.0.0 (Node.js GitHub Bot) #47339
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * drop ICU requirement for parsing hostnames (Yagiz Nizipli) #47339
  * use ada::url_aggregator for parsing urls (Yagiz Nizipli) #47339
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
danielleadams added a commit that referenced this pull request Jul 13, 2023
Notable changes:

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
deps:
  * update ada to 2.0.0 (Node.js GitHub Bot) #47339
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * drop ICU requirement for parsing hostnames (Yagiz Nizipli) #47339
  * use ada::url_aggregator for parsing urls (Yagiz Nizipli) #47339
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
danielleadams added a commit that referenced this pull request Jul 17, 2023
Notable changes:

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
  * use WebIDL converters in WebCryptoAPI (Filip Skokan) #46067
deps:
  * update ada to 2.0.0 (Node.js GitHub Bot) #47339
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) #47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) #46190
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * drop ICU requirement for parsing hostnames (Yagiz Nizipli) #47339
  * use ada::url_aggregator for parsing urls (Yagiz Nizipli) #47339
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
danielleadams added a commit that referenced this pull request Jul 17, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in #47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in #46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) #47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) #46190
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
danielleadams added a commit that referenced this pull request Jul 17, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in #47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in #46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) #47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) #46190
  * (SEMVER-MINOR) implement AbortSignal.any() (Chemi Atlow) #47821
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
danielleadams added a commit that referenced this pull request Jul 18, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in #47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in #46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) #47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) #46190
  * (SEMVER-MINOR) implement AbortSignal.any() (Chemi Atlow) #47821
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
danielleadams added a commit that referenced this pull request Jul 18, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in #47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in #46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) #47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) #46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) #47844
  * add KhafraDev to collaborators (Matthew Aitken) #47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) #47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) #41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) #47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) #46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) #47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) #47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) #47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) #46190
  * (SEMVER-MINOR) implement AbortSignal.any() (Chemi Atlow) #47821
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) #47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) #48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) #47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (#46929) (Robert Nagy) #46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) #48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) #47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) #47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) #47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) #48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #48694
illusi0n7 added a commit to illusi0n7/node that referenced this pull request Jul 20, 2023
JSDoc update was omitted during code implementation

Refs: nodejs#46933
aduh95 pushed a commit that referenced this pull request Jul 23, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: #46933
PR-URL: #48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Jul 27, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs#46933
PR-URL: nodejs#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
pluris pushed a commit to pluris/node that referenced this pull request Aug 6, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in nodejs#47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in nodejs#46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) nodejs#47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) nodejs#46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) nodejs#47844
  * add KhafraDev to collaborators (Matthew Aitken) nodejs#47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) nodejs#47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) nodejs#41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) nodejs#46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) nodejs#47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) nodejs#47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) nodejs#47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) nodejs#46190
  * (SEMVER-MINOR) implement AbortSignal.any() (Chemi Atlow) nodejs#47821
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) nodejs#47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) nodejs#48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) nodejs#46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) nodejs#47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (nodejs#46929) (Robert Nagy) nodejs#46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) nodejs#48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) nodejs#47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) nodejs#47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) nodejs#47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) nodejs#47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) nodejs#48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) nodejs#47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) nodejs#47286

PR-URL: nodejs#48694
pluris pushed a commit to pluris/node that referenced this pull request Aug 6, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs#46933
PR-URL: nodejs#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
pluris pushed a commit to pluris/node that referenced this pull request Aug 7, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in nodejs#47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in nodejs#46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) nodejs#47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) nodejs#46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) nodejs#47844
  * add KhafraDev to collaborators (Matthew Aitken) nodejs#47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) nodejs#47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) nodejs#41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) nodejs#46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) nodejs#47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) nodejs#47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) nodejs#47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) nodejs#46190
  * (SEMVER-MINOR) implement AbortSignal.any() (Chemi Atlow) nodejs#47821
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) nodejs#47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) nodejs#48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) nodejs#46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) nodejs#47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (nodejs#46929) (Robert Nagy) nodejs#46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) nodejs#48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) nodejs#47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) nodejs#47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) nodejs#47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) nodejs#47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) nodejs#48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) nodejs#47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) nodejs#47286

PR-URL: nodejs#48694
pluris pushed a commit to pluris/node that referenced this pull request Aug 7, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs#46933
PR-URL: nodejs#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in nodejs#47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in nodejs#46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) nodejs#47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) nodejs#46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) nodejs#47844
  * add KhafraDev to collaborators (Matthew Aitken) nodejs#47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) nodejs#47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) nodejs#41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) nodejs#46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) nodejs#47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) nodejs#47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) nodejs#47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) nodejs#46190
  * (SEMVER-MINOR) implement AbortSignal.any() (Chemi Atlow) nodejs#47821
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) nodejs#47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) nodejs#48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) nodejs#46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) nodejs#47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (nodejs#46929) (Robert Nagy) nodejs#46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) nodejs#48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) nodejs#47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) nodejs#47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) nodejs#47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) nodejs#47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) nodejs#48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) nodejs#47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) nodejs#47286

PR-URL: nodejs#48694
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs#46933
PR-URL: nodejs#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Notable changes:

Ada 2.0
Node.js v18.17.0 comes with the latest version of the URL parser, Ada. This
update brings significant performance improvements to URL parsing, including
enhancements to the url.domainToASCII and url.domainToUnicode functions
in node:url.

Ada 2.0 has been integrated into the Node.js codebase, ensuring that all
parts of the application can benefit from the improved performance. Additionally,
Ada 2.0 features a significant performance boost over its predecessor, Ada 1.0.4,
while also eliminating the need for the ICU requirement for URL hostname parsing.

Contributed by Yagiz Nizipli and Daniel Lemire in nodejs#47339

Web Crypto API
Web Crypto API functions' arguments are now coerced and validated as per
their WebIDL definitions like in other Web Crypto API implementations. This
further improves interoperability with other implementations of Web Crypto API.

Contributed by Filip Skokan in nodejs#46067

crypto:
  * update root certificates to NSS 3.89 (Node.js GitHub Bot) nodejs#47659
dns:
  * (SEMVER-MINOR) expose getDefaultResultOrder (btea) nodejs#46973
doc:
  * add ovflowd to collaborators (Claudio Wunder) nodejs#47844
  * add KhafraDev to collaborators (Matthew Aitken) nodejs#47510
* events:
  * (SEMVER-MINOR) add getMaxListeners method (Matthew Aitken) nodejs#47039
fs:
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) add recursive option to readdir and opendir (Ethan Arrowood) nodejs#41439
  * (SEMVER-MINOR) add support for mode flag to specify the copy behavior (Tetsuharu Ohzeki) nodejs#47084
  * (SEMVER-MINOR) implement byob mode for readableWebStream() (Debadree Chatterjee) nodejs#46933
http:
  * (SEMVER-MINOR) prevent writing to the body when not allowed by HTTP spec (Gerrard Lindsay) nodejs#47732
  * (SEMVER-MINOR) remove internal error in assignSocket (Matteo Collina) nodejs#47723
  * (SEMVER-MINOR) add highWaterMark opt in http.createServer (HinataKah0) nodejs#47405
lib:
  * (SEMVER-MINOR) add webstreams to Duplex.from() (Debadree Chatterjee) nodejs#46190
  * (SEMVER-MINOR) implement AbortSignal.any() (Chemi Atlow) nodejs#47821
module:
  * change default resolver to not throw on unknown scheme (Gil Tayar) nodejs#47824
node-api:
  * (SEMVER-MINOR) define version 9 (Chengzhong Wu) nodejs#48151
  * (SEMVER-MINOR) deprecate napi_module_register (Vladimir Morozov) nodejs#46319
stream:
  * (SEMVER-MINOR) preserve object mode in compose (Raz Luvaton) nodejs#47413
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (nodejs#46929) (Robert Nagy) nodejs#46929
test:
  * unflake test-vm-timeout-escape-nexttick (Santiago Gimeno) nodejs#48078
test_runner:
  * (SEMVER-MINOR) add shorthands to `test` (Chemi Atlow) nodejs#47909
  * (SEMVER-MINOR) support combining coverage reports (Colin Ihrig) nodejs#47686
  * (SEMVER-MINOR) execute before hook on test (Chemi Atlow) nodejs#47586
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) nodejs#47238
tools:
  * update LICENSE and license-builder.sh (Santiago Gimeno) nodejs#48078
url:
  * (SEMVER-MINOR) implement URL.canParse (Matthew Aitken) nodejs#47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) nodejs#47286

PR-URL: nodejs#48694
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs#46933
PR-URL: nodejs#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs#46933
PR-URL: nodejs#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: #46933
PR-URL: #48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
targos pushed a commit that referenced this pull request Nov 27, 2023
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: #46933
PR-URL: #48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs/node#46933
PR-URL: nodejs/node#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
JSDoc comment did not match the actual implementation of
`FileHandle#readableWebStream`.

Refs: nodejs/node#46933
PR-URL: nodejs/node#48853
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
isker added a commit to isker/node that referenced this pull request Oct 19, 2024
The original implementation of the experimental
`FileHandle.readableWebStream` API created non-`type: 'bytes'` streams,
which prevented callers from creating `mode: 'byob'` readers from the
returned stream, which means they could not achieve the associated
"zero-copy" performance characteristics.

Then, nodejs#46933 added a parameter allowing callers to pass the `type`
parameter down to the ReadableStream constructor, exposing the same
semantics to callers of `FileHandle.readableWebStream`.

But there is no point to giving callers this choice: FileHandle-derived
streams are by their very nature byte streams. We should not require
callers to explicitly opt in to having byte stream semantics. Moreover,
I do not see a situation in which callers would ever want to have a
non-bytes stream: bytes-streams only do anything differently than normal
ones if `mode: 'byob'` is passed to `getReader`.

So, remove the `options` parameter and always create a ReadableStream
with `type: 'bytes'`.

Fixes nodejs#54041.
isker added a commit to isker/node that referenced this pull request Nov 7, 2024
The original implementation of the experimental
`FileHandle.readableWebStream` API created non-`type: 'bytes'` streams,
which prevented callers from creating `mode: 'byob'` readers from the
returned stream, which means they could not achieve the associated
"zero-copy" performance characteristics.

Then, nodejs#46933 added a parameter allowing callers to pass the `type`
parameter down to the ReadableStream constructor, exposing the same
semantics to callers of `FileHandle.readableWebStream`.

But there is no point to giving callers this choice: FileHandle-derived
streams are by their very nature byte streams. We should not require
callers to explicitly opt in to having byte stream semantics. Moreover,
I do not see a situation in which callers would ever want to have a
non-bytes stream: bytes-streams only do anything differently than normal
ones if `mode: 'byob'` is passed to `getReader`.

So, remove the `options` parameter and always create a ReadableStream
with `type: 'bytes'`.

Fixes nodejs#54041.
isker added a commit to isker/node that referenced this pull request Nov 7, 2024
The original implementation of the experimental
`FileHandle.readableWebStream` API created non-`type: 'bytes'` streams,
which prevented callers from creating `mode: 'byob'` readers from the
returned stream, which means they could not achieve the associated
"zero-copy" performance characteristics.

Then, nodejs#46933 added a parameter allowing callers to pass the `type`
parameter down to the ReadableStream constructor, exposing the same
semantics to callers of `FileHandle.readableWebStream`.

But there is no point to giving callers this choice: FileHandle-derived
streams are by their very nature byte streams. We should not require
callers to explicitly opt in to having byte stream semantics. Moreover,
I do not see a situation in which callers would ever want to have a
non-bytes stream: bytes-streams only do anything differently than normal
ones if `mode: 'byob'` is passed to `getReader`.

So, remove the `options` parameter and always create a ReadableStream
with `type: 'bytes'`.

Fixes nodejs#54041.
isker added a commit to isker/node that referenced this pull request Nov 7, 2024
The original implementation of the experimental
`FileHandle.readableWebStream` API created non-`type: 'bytes'` streams,
which prevented callers from creating `mode: 'byob'` readers from the
returned stream, which means they could not achieve the associated
"zero-copy" performance characteristics.

Then, nodejs#46933 added a parameter allowing callers to pass the `type`
parameter down to the ReadableStream constructor, exposing the same
semantics to callers of `FileHandle.readableWebStream`.

But there is no point to giving callers this choice: FileHandle-derived
streams are by their very nature byte streams. We should not require
callers to explicitly opt in to having byte stream semantics. Moreover,
I do not see a situation in which callers would ever want to have a
non-bytes stream: bytes-streams only do anything differently than normal
ones if `mode: 'byob'` is passed to `getReader`.

So, remove the `options` parameter and always create a ReadableStream
with `type: 'bytes'`.

Fixes nodejs#54041.
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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. fs Issues and PRs related to the fs subsystem / file system. review wanted PRs that need reviews. semver-minor PRs that contain new features and should be released in the next minor version. web streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Byte stream support for FileHandle::readableWebStream
5 participants