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: add customizable fetch function to ApiClientOptions #655

Merged
merged 1 commit into from
Nov 9, 2024

Conversation

IKatsuba
Copy link
Contributor

@IKatsuba IKatsuba commented Nov 9, 2024

This pull request introduces a new feature to the ApiClient class in src/core/client.ts to allow custom fetch functions. This enables more flexibility in making HTTP requests, especially in different environments like Node.js and Deno.

@KnorpelSenf
Copy link
Member

Can you explain a use case for a custom fetch function?

@IKatsuba
Copy link
Contributor Author

IKatsuba commented Nov 9, 2024

This PR gives a possibility to use native fetch for Node.js environments instead of node-fetch without breaking change

@KnorpelSenf
Copy link
Member

That's a great point. I might remove this again for 2.0 when we migrate to native fetch, we'll see. Either way, I'll open an issue about it first.

Copy link
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@KnorpelSenf KnorpelSenf merged commit 21ac2ed into grammyjs:main Nov 9, 2024
6 checks passed
@KnorpelSenf
Copy link
Member

@IKatsuba consider signing your commits in the future. We usually require this from all contributors, but I made an exception now.

@all-contributors add @IKatsuba for the idea and the code!

Copy link
Contributor

@KnorpelSenf

I've put up a pull request to add @IKatsuba! 🎉

@BilagoNet
Copy link
Contributor

BilagoNet commented Nov 10, 2024

Most likely after the update i.e. version 1.31.1 cloudflare workers stopped working, I think it was because fetch was added to this ApiClientOptions.

Error log in Cloidflare workers:
TypeError: Illegal invocation: function called with incorrect this reference.

image

@KnorpelSenf
Copy link
Member

@IKatsuba do you have time to fix up your changes?

@IKatsuba
Copy link
Contributor Author

@IKatsuba do you have time to fix up your changes?

I will research it during the day today

@KnorpelSenf
Copy link
Member

@IKatsuba do you have time to fix up your changes?

I will research it during the day today

Epic, thank you!

@IKatsuba IKatsuba deleted the fetch-as-option branch November 10, 2024 21:47
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