lquixada / cross-fetch

Universal WHATWG Fetch API for Node, Browsers and React Native.
MIT License
1.67k stars 104 forks source link

Not working on cloudflare worker #69

Closed tttp closed 1 year ago

tttp commented 4 years ago

Hi,

Cloudflare worker provides a fetch function, however, when I'm webpacking your lib, it does seem to use XMLHttpRequest

ReferenceError: XMLHttpRequest is not defined

I did try to force webpack to use node

module.exports = {
  target: "webworker",
  entry: "./worker.js", // inferred from "main" in package.json
  resolve: {
    mainFields: ['module', 'main']
  }
};

But it generates another error (global not defined)

Is there a way to force cross-fetch to to use the native fetch function?

as a proof of concept/fugly workaround, I replaced

const Fetch = fetch; //require('cross-fetch');

and it seems to work fine.

Munter commented 3 years ago

I'm hitting this as well because https://www.npmjs.com/package/prismic-javascript is using cross-fetch and I am doing things in Cloudflare workers.

In order to work around the lack of support for workers I added the following to my webpack.config.js:

  externals: [
    {
      'cross-fetch': 'fetch'
    }
  ]

That essentially replaces this whole module with the native fetch.

I still think cross-fetch should not try to polyfill what already exists in the environment it runs in. I think experience shows this is not good practice

joshmsamuels commented 2 years ago

~@lquixada This issue has been open for a while with no resolution. It appears the issue is that this repo overrides fetch if global.fetch is undefined. Cloudflare workers do not have a global object defined~

The issue actually looks more complex than I thought. Is it on the roadmap to fix this issue?

joshmsamuels commented 2 years ago

👀 https://github.com/lquixada/cross-fetch/blob/main/dist/browser-ponyfill.js#L546

Uncommenting that line and commenting out the one below fixed my issue

kiwicopple commented 2 years ago

We've been testing the supabase libs with the new Next.js middleware and found that it's also encountering this bug.

I can confirm that the patch @joshmsamuels has made fixes it.

@lquixada - I'm not familiar enough with this repo to know the repercussions of this change. However if you would like me to submit a PR with the change let me know

rattrayalex commented 2 years ago

@joshmsamuels the link to your fix seems to 404 now. Would you be willing to put up a PR?

joshmsamuels commented 2 years ago

The link is 404'ing since the dist folder that I was referencing was deleted along with all the other dist files at the beginning of the year.

It looks like that was also how browser fetch was supported so I am unsure what @lquixada's plan was as the owner and maintainer of the repo.

rattrayalex commented 2 years ago

Got it. Maybe I will make a fork to support this and other features, since it seems this repo isn't so actively maintained anymore…

SokratisVidros commented 2 years ago

Any updates on this?

lquixada commented 1 year ago

I've been working on version 4 of cross-fetch to fix this issue. If anyone's interested, please run npm install cross-fetch@latest-v4.x in your project and give it a try. Let me know if any issues come up.

aroman commented 1 year ago

I've been working on version 4 of cross-fetch to fix this issue. If anyone's interested, please run npm install cross-fetch@latest-v4.x in your project and give it a try. Let me know if any issues come up.

Thanks so much. Thanks to your 4.x branch, I was able to port a dependency I wanted to use (@logtail/node) to Cloudflare Workers — it's working great so far. I'll let you know if I run into any issues.

UPDATE: while the code seems to work fine, it looks like the 4.x branch still has a dependency on node-fetch, which is causing typescript issues: https://github.com/lquixada/cross-fetch/blob/v4.x/package.json#L71

lquixada commented 1 year ago

Version 4 has been officially released with the fix. Please check it out: npm install cross-fetch.