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

Feature Request: support body from ReadableStream #55

Open
cdaringe opened this issue Apr 23, 2024 · 4 comments
Open

Feature Request: support body from ReadableStream #55

cdaringe opened this issue Apr 23, 2024 · 4 comments
Assignees

Comments

@cdaringe
Copy link

Problem

fetchToCurl(new Request(myUrl, { method: 'POST', body: someReadableStream }) doesn't work, and yields --data-binary={}

Discussion

It's easy to get into wanting this case when dealing with Request instances from common node tools. For example, I'm using msw, which... by the time a request is exiting the VM and you're in a proxy handler, body is converted to a stream just from node itself.

Here's a dummy example of what we could do:

async function reqWithStreamToBodyString(req) {
  const reader = req.clone().body.getReader();
  let result = '';
  while (true) {
    const { value, done } = await reader.read();
    if (done) break;
    result += new TextDecoder("utf-8").decode(value);
  }
  return JSON.stringify(result);
}

Then, rather than a naive if (typeof body === 'object') { ... }, we could first do a if (typeof req.body?.getReader === 'function) { ... } and do the needful

@leoek
Copy link
Owner

leoek commented May 7, 2024

Hi @cdaringe

This sounds indeed like a useful addition, however I'd like to keep the fetchToCurl function synchronous for now. And I assume that implementing this would require it to be async right?

However if we hide this feature behind a flag or configuration option it shouldn't break any code which relies on it returning the string immediately. Are you interested in opening a PR?

Best regards
Leo

@cdaringe
Copy link
Author

cdaringe commented May 7, 2024

Yes to all!

@cdaringe
Copy link
Author

cdaringe commented May 13, 2024

So I started working on it, and i hit a few snags. I ended up wanting

  1. TS (RFC: typescript  #56 ) enough,
  2. a single entrypoint (e.g. just fetchToCurl)
  3. support for node and web streams

...that I just forked.

https://github.com/cdaringe/fetch-to-curl-ts

With that said, i'm happy to close the fork iff you're interested in the totality of the changes!

I just kinda got to hackin... and I either was gonna send you a massive PR, a wave a PRs, or this fork, and all were kinda overwhelming feeling (but i wanted stuff now)!

Sorry for the noise. Lemme know what you think.

-CD

@cdaringe cdaringe reopened this May 13, 2024
@leoek
Copy link
Owner

leoek commented Jun 5, 2024

I've not forgotten about this and will check it out :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants