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

Upgrade to node-fetch 3.0 final #2434

Closed
benmccann opened this issue Sep 15, 2021 · 6 comments · Fixed by #2422
Closed

Upgrade to node-fetch 3.0 final #2434

benmccann opened this issue Sep 15, 2021 · 6 comments · Fixed by #2422
Milestone

Comments

@benmccann
Copy link
Member

benmccann commented Sep 15, 2021

Describe the problem

we're on a beta version

Describe the proposed solution

I tried briefly in #2422, but ran into some issue. I'd love if someone were able to take a closer look and figure out what's going on

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

@benmccann benmccann added this to the 1.0 milestone Sep 15, 2021
@dominikg
Copy link
Member

I tried to look into this today:
node-fetch updated to pure esm with 3.0.0. But their package.json looks a bit strange.
It only has a "main" field pointing at src/index.js no "exports", no "module".
According to node docs, main is for require, so cjs.

I'm not entirely sure if/how that trips up kits build, but its very odd.

@dominikg
Copy link
Member

tried it with a local node-fetch where i added "module" and "exports", no luck. There is a strange import to "streams/web" in kit/dist/chunks/index5.js which is experimental and only available in node16. And what can i say, when i use the edited node-fetch and node16, i can build the hn example ...

If someone has an idea whats causing that import, i'd be glad to hear it.

@benmccann
Copy link
Member Author

It looks like they tried to pollyfill it:
https://github.com/sveltejs/kit/pull/2422/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR4090

I'm not sure why that isn't working

@dominikg
Copy link
Member

the code that tries it is in fetch-blob , streams.cjs

if (!globalThis.ReadableStream) {
  try {
    Object.assign(globalThis, require('stream/web'))
  } catch (error) {
		// TODO: Remove when only supporting node >= 16.5.0
    Object.assign(globalThis, require('web-streams-polyfill/dist/ponyfill.es2018.js'))
  }
}

@dominikg
Copy link
Member

aaaand there is a bug report on fetch-blob already node-fetch/fetch-blob#117

@winston0410
Copy link

tried it with a local node-fetch where i added "module" and "exports", no luck. There is a strange import to "streams/web" in kit/dist/chunks/index5.js which is experimental and only available in node16. And what can i say, when i use the edited node-fetch and node16, i can build the hn example ...

If someone has an idea whats causing that import, i'd be glad to hear it.

I tried building in node16 in local and node14.x in vercel, still the same error for not finding stream.

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

Successfully merging a pull request may close this issue.

3 participants