-
Notifications
You must be signed in to change notification settings - Fork 640
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
suggestion: deprecate readableStreamFromReader
and point to better alternatives
#3018
Comments
I agree. The GitHub search serves as evidence adequate to support the suggestion, IMO. Would deprecating |
Don't know how I missed that, nice catch. I agree that |
readableStreamFromReader & its opposite, + the equivalents for writable streams should be deprecated imo |
I had a similar thought, but wanted to keep this proposal pretty tame. Good to know I'm not completely out of my mind. Will open a PR for this soon, would like a few more takes from the community. |
@kt3k, WDYT? |
I'm in favor of deprecation in the future, but I'm not sure we're completely ready to do this. Do all I/O APIs in Deno namespace provide Web Stream interfaces now? |
Deprecated ones like Deno.Buffer do not, but just checked the non-deprecated ones and it looks like they do |
Curious on your reasoning for why we aren't ready to do this yet? I feel like all of the apis that would be deprecated in this proposal should really never be used in """modern""" or """correct""" Deno code anyways. If viable alternatives exist, which they do, we should be encouraging their use by deprecating these anti-patterns. Again, all of this is personal opinion and I am fully open to the fact that I may be missing something. |
|
There also is #1986 |
closed by #3640 |
Would it make sense to keep |
@jespertheend Does this cause trouble to you? |
Yeah, it would be a good workaround for when the day comes. And of course |
Is your feature request related to a problem? Please describe.
Before I begin my rambling: I created this issue to start a discussion, not sure if this makes any sense at all.
Deno has been continuously moving away from using
Deno.reader
and (afaik) it is essentially replaced/supplemented in every part of the deno api with aReadableStream<Uint8Array>
. We should continue to push people away from usingDeno.reader
by deprecating and eventually removingreadableStreamFromReader
from the std.To make sure I wasn't completely insane, I did a quick GitHub search and found that almost every use of
readableStreamFromReader
was for reading files (which supportsReadableStream<Uint8Array>
) and reading stdin/stderr (which also supportsReadableStream<Uint8Array>
). We should be pushing people away from using these for those use cases.The only thing that makes me hesitate a little bit is found in the optional options (
autoClose
, etc)Describe the solution you'd like
We deprecate and eventually remove
readableStreamFromReader
Describe alternatives you've considered
Because of the options, there may be an argument to keep it.
The text was updated successfully, but these errors were encountered: