Removing stream.pipe(req)
functionality
#443
jimmywarting
started this conversation in
General
Replies: 3 comments 1 reply
This comment has been hidden.
This comment has been hidden.
-
def down to mirror undici/fetch behavior more (hopefully using undici under the hood server-side instead of xmlhttprequest and fetch client-side) and do a semver major release |
Beta Was this translation helpful? Give feedback.
0 replies
-
@jimmywarting feel free to take the reigns with this effort and use your best judgment, love the thought and mental cycles you're putting in here, very very stoked to see what you come up with |
Beta Was this translation helpful? Give feedback.
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
If the intent is to use fetch / Request instead
...then you "can't" really pipe a stream to request. you instead use a stream as a body.
so it's kind of like piping but yet it isn't in the terminology of NodeJS streams.
so with fetch you would actually have to do something like
The fetch spec do also support async iterator that yield uint8arrays (so that it can accept both web & node streams)
so undici, web and NodeJS do all support generators that yields uint8array as a body...
So with that in mind i would suggest that pipe & write gets removed.
it should be replaced with a
.send()
method instead which acts more or less like a "body" -setterSo i would suggest that this example code (here) becomes more like:
neither
XMLHttpRequest
andfetch
do not have thisstream.pipe(req)
equally functionality. and it's considerable much harder to support it with some kind of own hackyRequestBase.prototype.pipe|write
functionality then it's to just simple handle it the proper way in a.send()
method.Making superagents
Request
extendnode:streams
makes it considerable much harder to extendBaseRequest
cuz it instead depends on mixins to add functionality onto classesSo it makes it hard to refactor the prototype based codespace into classes when i can't use
class X extends Y
.The web it dose not even inherit stream and you can't pipe stuff at all in the web version of superagent. so removing pipe, write and inheriting streams makes more sense.
inherit (or extending)
node:streams
means it would have to polyfill a bunch of things for non-NodeJS env and including buffer, EventEmitter, text_decoder and a bunch of things that are not a good fit for eg web or Deno.Beta Was this translation helpful? Give feedback.
All reactions