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

feat: Replace http(s) with node-fetch #93

Conversation

shrink
Copy link
Contributor

@shrink shrink commented Nov 13, 2024

Closes #82

Replaces http(s) with node-fetch. The public-facing API of the library is unchanged but the mechanism for communicating with the API has changed and could manifest in different behaviour, e.g: previously we were handling errors in different places with different logic. There's also an increase in the minimum node version from 10 to 14.

Added a Workflow to test the library with each supported version of node.

  • Release as v4

@ipinfo ipinfo deleted a comment from linear bot Nov 13, 2024
@shrink shrink requested a review from max-ipinfo November 13, 2024 16:15
__tests__/ipinfoWrapper.test.ts Show resolved Hide resolved
@max-ipinfo
Copy link
Contributor

@shrink Feel free to include a semver upgrade to 4.x.x

Here is an example from the last release: 089af58

@shrink
Copy link
Contributor Author

shrink commented Nov 14, 2024

@max-ipinfo added the changelog entry and new version. I've also simplified the documentation and added information about catching errors as a best practice. The documentation includes separate TypeScript and JavaScript examples but as TypeScript can run all JavaScript we do not need separate examples: I have consolidated the examples.

@shrink shrink marked this pull request as ready for review November 14, 2024 07:38
Copy link
Contributor

@max-ipinfo max-ipinfo left a comment

Choose a reason for hiding this comment

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

LGTM!

@shrink shrink merged commit 60d6279 into master Nov 21, 2024
7 checks passed
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.

Use Fetch API instead of node's http(s) package
2 participants