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

Dep: update node-fetch #507

Closed
wants to merge 2 commits into from

Conversation

MattBidewell
Copy link

@MattBidewell MattBidewell commented Sep 12, 2022

Fixes #506

Minor fix for CVE-2022-2596

Also added a temp fix for a type issue brought in by the latest update in node-fetch.
See: node-fetch/node-fetch#1617

First contribution, so any suggestions or things I've missed are greatly welcomed.

  • Tests pass (locally)
  • Appropriate changes to README are included in PR
  • Types updated

@MattBidewell MattBidewell marked this pull request as ready for review September 12, 2022 11:07
@@ -1,7 +1,7 @@
{
"compilerOptions": {
"target": "ES2021",
"lib": ["ES2021"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the reason, I don't what to update node-fetch to the newest version.

Probably gonna looks for some other implementation of fetch API.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use Axios or wait for Node's core fetch implementation as alternatives. Happy to help if needed 😊

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antongolub should we add dom? And make zx v8 with node's fetch?

@antonmedv
Copy link
Collaborator

@antongolub what are your thoughts?

@antongolub
Copy link
Collaborator

antongolub commented Sep 12, 2022

It's time to drop fetch polyfill, IMO. If it is still needed, it will be possible to use #498 to install

@MattBidewell
Copy link
Author

MattBidewell commented Sep 12, 2022

So remove node-fetch completely and allow users to use #498 to install a fetch package on a need-to-use basis?

@MattBidewell
Copy link
Author

I'm going to close the PR. I'll keep an eye on issues and try help when/where I can :)

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 this pull request may close these issues.

There is a vulnerability in node-fetch 3.2.8,upgrade recommended
3 participants