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

Remove node-fetch and its typings #67

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

Protonull
Copy link
Contributor

@Protonull Protonull commented Aug 12, 2023

No description provided.

@Huskydog9988
Copy link
Contributor

Can't we replace this with the native fetch api node introduced?

@Gjum
Copy link
Member

Gjum commented Aug 12, 2023

Some distros don't make it easy to use a recent nodejs, but if we require a recent nodejs for other features anyway we can switch to builtin fetch. Are there any benefits from using the builtin fetch vs node-fetch 2?

@Huskydog9988
Copy link
Contributor

Huskydog9988 commented Aug 12, 2023

Not really sure to be honest, and personally I just use Docker so node versions don't affect me much. So I'm fine with whatever you wanna do.

@Protonull
Copy link
Contributor Author

Just tested and node 18 has native fetch, so the module shouldn't be necessary anymore

@Protonull Protonull changed the title Update node-fetch to 2.6.12 Remove node-fetch and its typings Aug 12, 2023
No longer necessary given the native fetch function now available in node 18.
@Protonull
Copy link
Contributor Author

All good now, this is now a 'yeet node-fetch' PR

@Huskydog9988
Copy link
Contributor

I assume this is tested?

@Protonull
Copy link
Contributor Author

I assume this is tested?

$ node --version
v18.17.1
// test.js
fetch("https://github.com/CivPlatform/map-sync/pull/67")
    .then((res) => res.text())
    .then((txt) => console.log("SUCCESS!"))
    .catch((err) => console.error("Err!", err))
$ node test.js
SUCCESS!

@Huskydog9988
Copy link
Contributor

Good enough for me!

@Huskydog9988 Huskydog9988 merged commit 815da1e into CivPlatform:stable Aug 12, 2023
2 checks passed
@Protonull Protonull deleted the update-fetch branch August 12, 2023 02:07
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.

3 participants