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

Optional parameter strategy for stream.Readable.toWeb()? #43487

Closed
rauschma opened this issue Jun 19, 2022 · 5 comments
Closed

Optional parameter strategy for stream.Readable.toWeb()? #43487

rauschma opened this issue Jun 19, 2022 · 5 comments
Labels
feature request Issues that request new features to be added to Node.js. stale

Comments

@rauschma
Copy link

What is the problem this feature will solve?

Currently, it is not possibly to customize the queueing strategy of the ReadableStream returned by stream.Readable.toWeb().

What is the feature you are proposing to solve the problem?

Change the signature from:

stream.Readable.toWeb(streamReadable: Readable): ReadableStream
stream.Readable.toWeb(streamReadable: Readable, strategy?: QueuingStrategy): ReadableStream

What alternatives have you considered?

I’m not aware of any alternatives: As far as I know, there is no way to change the queuing strategy of a ReadableStream after its constructor was invoked.

@rauschma rauschma added the feature request Issues that request new features to be added to Node.js. label Jun 19, 2022
@Trott
Copy link
Member

Trott commented Jun 19, 2022

@nodejs/streams

@benjamingr
Copy link
Member

@rauschma hey Axel, sure this is a pretty easy fix, the code is in /lib/internal/webstreams/adapters.js line 389, the code currently just does:

  const strategy =
    objectMode ?
      new CountQueuingStrategy({ highWaterMark }) :
      { highWaterMark };

Would you be interested in contributing a fix? (Happy to guide you through)

@Warkanlock
Copy link
Contributor

Warkanlock commented Jun 21, 2022

I took it upon myself to make a change to include a strategy parameter in the .toWeb() method inside the Readable.js stream object.

This would be my first PR against the Node code base, please don't be too hard on me lol.

The tests are pending but I would love to hear how I can approach them.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Dec 19, 2022
@benjamingr
Copy link
Member

This already landed 🎉

@targos targos moved this from Pending Triage to Stale in Node.js feature requests Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. stale
Projects
None yet
Development

No branches or pull requests

4 participants