Type of `request` property in `RequestEvent` is incorrect

[aberndtiteratec]

Describe the bug

When I try to call getReader() on event.request.body in a handle function, I receive an error: response.body.getReader is not a function, even though autocompletion suggests that it should be possible.

Reproduction

See the log outputs after clicking on Test Request
https://stackblitz.com/edit/svelte-kit-p42fqc?file=src/hooks.ts

Logs

No response

System Info

System:
    OS: Windows 10 10.0.19044
    CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz  
    Memory: 18.11 GB / 31.72 GB
  Binaries:
    Node: 16.11.1 - C:\Program Files\nodejs\node.EXE        
    npm: 8.0.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (97.0.1072.76)
    Internet Explorer: 11.0.19041.1202
  npmPackages:
    @sveltejs/adapter-node: ^1.0.0-next.67 => 1.0.0-next.67 
    @sveltejs/kit: ^1.0.0-next.251 => 1.0.0-next.251        
    svelte: ^3.46.3 => 3.46.3

Severity

annoyance

Additional Information

No response

[Rich-Harris]

Yep, this is an unfortunate quirk of node-fetch. Handling streaming bodies is a high priority TODO, but we haven't quite figured out how to make the ReadableStream interface available on node-fetch Request bodies #3419

[aberndtiteratec]

If this is a problem with node-fetch, I think this type is supposed to come from node-fetch rather than from the fetch api

[Rich-Harris]

We don't want to use node-fetch types because while node-fetch is used in dev, preview and the netlify/node/vercel adapters, it's not used universally (e.g. Deno and Cloudflare Workers expose a more standards-compliant fetch interface). Node itself now has fetch behind a flag. Instead, we need to make interfaces like getReader available somehow

[jeremybradbury]

FWIW node-fetch is garbage and often the cause of issues (and often type related since they reuse type/interface names that exist in core node) with libraries like this one, whose maintainers often argue "axios is too large".

Well for those humans (like me), there's Request which works fantastically with a simple JS wrapper, running in a compiled C++ binary bundled with node, it's not only not a dependency, it's not even really JS (just the simple wrapper part).

Edit: I added a gist of an old CJS wrapper function I keep using that could replace what you use from the node-fetch library in a few lines of JS (or TS). https://gist.github.com/jeremybradbury/cf049bf7aa6447e284bf07c923601c0d

Then I made some edits to address goals and concerns discussed here: #3384

Some say don't reinvent the wheel... I say build custom rims when you can make them better (code that works and can be maintained without a PR, waiting for updates, workarounds or "just dealing with it") & cheaper (less operating costs, time spent, lines of code).

Other libraries can make maintaining a library really hard. So in this case, when one isn't even needed, it seems to be doing more harm than good.

[Rich-Harris]

The value of using node-fetch isn't the implementation, it's the standard, which allows developers to use the same code in Node, Deno, Cloudflare Workers, browsers, and so on. I'm sure your wrapper is great, but it can't do that.

We'll continue to use node-fetch until fetch is available natively in Node without a flag.

[jeremybradbury]

I'm sure your wrapper is great, but it can't do that.

no it cannot, not without some modifications and planning. but in about 50 lines it handles any get/post binary/files/json/text streams or files of any kind.

anyway you'll likely need your own tooling for processing & rendering streaming (binary) results from a firehose, which I know is a highly anticipated feature.

I personally don't think node-fetch is close enough to the browser fetch APIs to be of much benefit, even then, all fetch patterns have much to be desired without using some sort of wrapping, even what's in node.

anyway, i only offered an idea that solves 2 (or more) problems.

starting with a possible new feature & later considering it for replacement after some testing and API work, seems like a logical path.

We'll continue to use node-fetch until fetch is available natively in Node without a flag.

for sure, use what you think is best. i'm just some dude on the internet.

i wasn't expecting you to use my code, just explain what I meant by pointing at it like a document.

sorry for the tangent which was mostly unrelated to this particular thread, but also a possible solution here.

[Rich-Harris]

Closing this in favour of #3419