GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fs: implement byob mode for readableWebStream() by debadree25 · Pull Request #46933 · nodejs/node

Conversation

debadree25 commented Mar 3, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

~~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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss and KhafraDev reacted with eyes emoji

All reactions

👀 2 reactions


          fs: implement byob mode for readableWebStream()

42d6d8d

Fixes: nodejs#45853

nodejs-github-bot added fs

Issues and PRs related to file-system APIs and the fs module.

needs-ci PRs that need a full CI run. labels

Mar 3, 2023

debadree25 requested a review from jasnell

March 3, 2023 11:09

This comment was marked as outdated.

debadree25 closed this

Mar 4, 2023

debadree25 reopened this

Mar 4, 2023

debadree25 removed the request for review from jasnell

March 4, 2023 19:08


          fixup! update logic, add tests and doc

698ab09

debadree25 marked this pull request as ready for review

March 4, 2023 19:10

debadree25 requested a review from jasnell

March 4, 2023 19:10

debadree25 added the web streams Issues and PRs related to the Web Streams API. label

Mar 4, 2023


          fixup! doc

4e951e7

debadree25 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Mar 5, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Mar 5, 2023

This comment was marked as outdated.

debadree25 added the review wanted PRs that need review. label

Mar 6, 2023

debadree25 commented Mar 7, 2023

Copy link

Copy Markdown

Contributor Author

also cc'ing @nodejs/whatwg-stream

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

debadree25 commented Mar 16, 2023

Copy link

Copy Markdown

Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Mar 16, 2023

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

debadree25 requested review from mcollina and ronag

March 21, 2023 17:31

debadree25 commented Mar 29, 2023

Copy link

Copy Markdown

Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr commented Apr 8, 2023

Copy link

Copy Markdown

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.

lin72h and debadree25 reacted with heart emoji

All reactions

❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

debadree25 commented Apr 8, 2023

Copy link

Copy Markdown

Contributor 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 😄😄!

lin72h reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr approved these changes

Apr 9, 2023

View reviewed changes

benjamingr left a comment

Copy link

Copy Markdown

Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

debadree25 and lin72h reacted with hooray emoji

debadree25 and lin72h reacted with rocket emoji

All reactions

🎉 2 reactions
🚀 2 reactions

debadree25 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Apr 9, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Apr 9, 2023

This comment was marked as outdated.

danielleadams added a commit that referenced this pull request

Jul 17, 2023


          2023-07-17, Version 18.17.0 'Hydrogen' (LTS)

5c1428a

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


          2023-07-17, Version 18.17.0 'Hydrogen' (LTS)

ba8e95b

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


          2023-07-18, Version 18.17.0 'Hydrogen' (LTS)

a39b8a2

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


          2023-07-18, Version 18.17.0 'Hydrogen' (LTS)

51513b2

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


          fs: sync JSDoc with the actual implementation

eddf5cc

JSDoc update was omitted during code implementation

Refs: nodejs#46933

illusi0n7 mentioned this pull request

Jul 20, 2023

fs: sync JSDoc with the actual implementation #48853

Merged

aduh95 pushed a commit that referenced this pull request

Jul 23, 2023


          typings: sync JSDoc with the actual implementation

9c5e272

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


          typings: sync JSDoc with the actual implementation

f7ed4bd

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


          2023-07-18, Version 18.17.0 'Hydrogen' (LTS)

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


          typings: sync JSDoc with the actual implementation

e0dc180

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


          2023-07-18, Version 18.17.0 'Hydrogen' (LTS)

9cae6c1

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


          typings: sync JSDoc with the actual implementation

0bb33de

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


          typings: sync JSDoc with the actual implementation

094523b

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


          typings: sync JSDoc with the actual implementation

be7b511

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


          typings: sync JSDoc with the actual implementation

4d70a2c

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


          typings: sync JSDoc with the actual implementation

2164ac0

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


          typings: sync JSDoc with the actual implementation

3b9317d

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 mentioned this pull request

Oct 19, 2024

fs: make FileHandle.readableWebStream always create byte streams #55461

Merged

isker added a commit to isker/node that referenced this pull request

Nov 17, 2024


          fs: make FileHandle.readableWebStream always create byte streams

41c448c

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.

nodejs-github-bot pushed a commit that referenced this pull request

Feb 10, 2025


          fs: make FileHandle.readableWebStream always create byte streams

1781f63

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, #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 #54041.

PR-URL: #55461
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>

coderabbitai Bot mentioned this pull request

Sep 19, 2026

Add smoke tests for the built CLI primocms/primo-cli#25

Merged

Reviewers

mcollina

mcollina approved these changes

benjamingr

benjamingr approved these changes

jasnell

Awaiting requested review from jasnell

ronag

Awaiting requested review from ronag

+1 more reviewer

saschanaz

saschanaz left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

commit-queue-squash

PRs the Commit Queue should land as one squashed commit.

Issues and PRs related to file-system APIs and the fs module.

review wanted

PRs that need review.

semver-minor

PRs that contain new features and should be released in the next minor version.

web streams

Issues and PRs related to the Web Streams API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Byte stream support for FileHandle::readableWebStream

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

debadree25 commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

debadree25 commented Mar 7, 2023

Uh oh!

debadree25 commented Mar 16, 2023

Uh oh!

nodejs-github-bot commented Mar 16, 2023

Uh oh!

debadree25 commented Mar 29, 2023

Uh oh!

benjamingr commented Apr 8, 2023

Uh oh!

debadree25 commented Apr 8, 2023

Uh oh!

benjamingr left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

debadree25 commented Mar 3, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.