-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: Web Streams Support #61
Conversation
Adds Web Streams API to worker context, and switches to the titelmedia fork of node-fetch which uses Web Streams instead of NodeJS ones. Also closes issue gja#39, as @titelmedia/node-fetch implements Response.redirect.
It looks like FormData is no longer supported. Trying to use it with `wrangler dev` throws an error.
Hi! I can see you've switched to You may be better maintaining your own fork / npm package. |
@ThisIsMissEm thanks for the warning. @titelmedia/node-fetch looks like it's pretty awesome! @mrbbot I see in the other PR that the project is still maintained. However, it's massively divergent from the default node-fetch, so I'm not super keen on making this the default. Perhaps you could update this PR so that the version of fetch is passed in as an argument or something? We can update the documentation to demonstrate how to do this (into create-cloudflare-worker as well) |
@gja the problem with that is then you don't actually conform to the Cloudflare Workers API, which is why that fork of node-fetch was created; node-fetch doesn't conform to the fetch API, and they don't intend to, so really they shouldn't be called "node-fetch" but more node-kinda-fetch. If you're trying to build a cloudflare worker-like environment, you'll definitely want to start from the @titelmedia/node-fetch fork. It sounds like @matchs is actually maintaining the fork, I didn't know it's status, as it'd been years since I initially worked on it. |
@gja Just like @ThisIsMissEm said. The reason we forked and changed node-fetch was because, when we were internally implementing our own version of a local cloudflare-worker, we noticed that we couldn't actually reproduce how cloudflare-worker behaved because node-fetch wasn't compliant to the fetch API, specially on the type of I'm now curious to see how you're making it work with the default node-fetch (and would be glad to share a bit about our solution too - honestly, we should have made that open source years ago). Just in case it's interesting for you, there's already other local cloudflare-workers implementation beyond our own that uses our fork as a starting point: /~https://github.com/dollarshaveclub/cloudworker. |
Ooops! Didn't mean to push that to this branch. |
This PR adds Web Streams (provided by
web-streams-polyfill
) to workers' scope and addsReadableStream
support to KVget
,getWithMetadata
andput
functions.It also switches to using the
@titelmedia/node-fetch
fork ofnode-fetch
which uses Web Streams instead of NodeJS ones, allowing Web Streams to be used as response bodies. This also closes #39, asResponse.redirect
is implemented by this fork (without a default redirect status though, I've opened a PR).