elbywan / wretch

A tiny wrapper built around fetch with an intuitive syntax. :candy:
MIT License
4.83k stars 98 forks source link

Should no longer recommend form-data #179

Closed jimmywarting closed 1 year ago

jimmywarting commented 1 year ago

I just so happen to find this project when i read a reddit post

// w is a reusable wretch instance
const w = wretch().polyfills({
  fetch: require("node-fetch"),
  FormData: require("form-data"),
  URLSearchParams: require("url").URLSearchParams,
});

i saw this snipped and node-fetch@3 no longer supports cjs and it have ditched support for the abnormal form-data package for not follow the standard and not being iterable and working with Blobs instead. instead you should recommend a spec'ed variant of formdata such as formdata-polyfill which node-fetch now also depends internally on and re-exports it... just b/c it needs handle decoding payloads back to formdata using response.formData().then(fd => { ... }) so it should actually be:

import fetch, { FormData } from 'node-fetch'
const w = wretch().polyfills({ fetch, FormData })
jimmywarting commented 1 year ago

ofc, NodeJS v20 just now got released so i think ppl should begin targeting v16 as LTS now. so then they can begin using undici instead directly

node-fetch have now kind of become a older-supported-targeted solution

elbywan commented 1 year ago

Hey @jimmywarting,

i saw this snipped and node-fetch@3 no longer supports cjs and it have ditched support for the abnormal form-data package for not follow the standard and not being iterable and working with Blobs instead.

Thanks for pointing this out 👍 ! I updated the outdated polyfill part of the readme.

ofc, NodeJS v20 just now got released so i think ppl should begin targeting v16 as LTS now. so then they can begin using undici instead directly

Indeed, wretch has been initially written a long time ago when fetch support was a long way from being a part of the stdlib (node lts version was 8 if I recall correctly). The node-fetch part of the readme is only for node < 18 which is kinda deprecated already. I will drop it completely at some point.

jimmywarting commented 1 year ago
// Either mutate the global object…
global.fetch = fetch;
global.FormData = FormData;

global should not be used any longer either. globalThis is the new name everybody should be using from now on. it's universal for all env/workers

global is a node specific thing.

elbywan commented 1 year ago

global is a node specific thing.

Yes, this code snippet comes from the nodejs specific section.

Closing the issue, feel free to reopen if I am missing something related to the original problem.

jimmywarting commented 1 year ago

global is a node specific thing.

Yes, this code snippet comes from the nodejs specific section.

even so i think globalThis should be used instead 😉

Closing the issue, feel free to reopen if I am missing something related to the original problem.

that's okey with me 👍