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

Not working on cloudflare worker #69

Closed
tttp opened this issue Jul 24, 2020 · 11 comments · May be fixed by paaschdigital/utils#2
Closed

Not working on cloudflare worker #69

tttp opened this issue Jul 24, 2020 · 11 comments · May be fixed by paaschdigital/utils#2
Labels

Comments

@tttp
Copy link

tttp commented Jul 24, 2020

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
Copy link

Munter commented Mar 22, 2021

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
Copy link

joshmsamuels commented Oct 3, 2021

@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
Copy link

👀 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
Copy link

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
Copy link

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

@joshmsamuels
Copy link

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
Copy link

rattrayalex commented Mar 2, 2022

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
Copy link

Any updates on this?

ThisIsMissEm added a commit to ThisIsMissEm/cross-fetch that referenced this issue May 26, 2022
I'm not sure why the code was written the way it is, but it appears to be forcing the polyfill to be used even in environments where the Fetch API exists natively, which isn't intentional and causes the bugs described in lquixada#91, lquixada#78, and lquixada#69.

The original commit and PR don't describe why the behaviour was changed: lquixada@123b1a2
bahlo pushed a commit to axiomhq/cross-fetch that referenced this issue Jun 3, 2022
I'm not sure why the code was written the way it is, but it appears to be forcing the polyfill to be used even in environments where the Fetch API exists natively, which isn't intentional and causes the bugs described in lquixada#91, lquixada#78, and lquixada#69.

The original commit and PR don't describe why the behaviour was changed: lquixada@123b1a2
bahlo pushed a commit to axiomhq/cross-fetch that referenced this issue Jun 3, 2022
I'm not sure why the code was written the way it is, but it appears to be forcing the polyfill to be used even in environments where the Fetch API exists natively, which isn't intentional and causes the bugs described in lquixada#91, lquixada#78, and lquixada#69.

The original commit and PR don't describe why the behaviour was changed: lquixada@123b1a2
@lquixada
Copy link
Owner

lquixada commented Jun 9, 2023

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

@aroman
Copy link

aroman commented Jun 17, 2023

I've been working on version 4 of cross-fetch to fix this issue. If anyone's interested, please run npm install [email protected] 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
Copy link
Owner

lquixada commented Jul 3, 2023

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

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

Successfully merging a pull request may close this issue.

8 participants