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: fetch adapter #315

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

lucasthevenet
Copy link

@lucasthevenet lucasthevenet commented May 25, 2023

Closes #188

I'm not really sure if my approach is really the best way to implement this adapter.

In order to use the existing node http handler, I "mocked" the request and response objects to make them compatible with the Fetch API.

A better approach might be to implement a new base http handler (like the one in trpc).
That way we have a little more flexibility in terms of what we can pass as a request object.

Though it may not be worth the trouble since the approach I took does in fact work (however hacky it may be).

Open to any comments or suggestions on how we can improve this.

@jlalmes
Copy link
Owner

jlalmes commented May 30, 2023

Not a bad approach for the time being 👍

The core of this package is currently directly coupled to node, I'd like to spend some time to address this in the future. Probably need a chat with @juliusmarminge to think about how best to publish this library so that you only rely on the required dependencies of your specific adapter.

@reinvanhaaren
Copy link

+1 for the fetch adapter.

@rwieruch
Copy link

Hi @lucasthevenet Thanks for your work on this! I copied src/adapters/fetch.ts into my project and it worked locally. When I deployed it to Vercel, I got the following error when I hit the API:

TypeError: N.hasOwnProperty is not a function
    at (node_modules/safer-buffer/safer.js:13:14)
    at (webpack/bootstrap:21:0)
    at (node_modules/iconv-lite/lib/index.js:5:13)
    at (webpack/bootstrap:21:0)
    at (node_modules/raw-body/index.js:18:12)
    at (webpack/bootstrap:21:0)
    at (node_modules/co-body/lib/json.js:7:12)
    at (webpack/bootstrap:21:0)
    at (node_modules/co-body/lib/any.js:8:13)
    at (webpack/bootstrap:21:0)

Not sure what's different over there. Anyway, thanks for your work and I will investigate further :)

@gabe-zamp
Copy link

Any plans to merge this @jlalmes?

@Jonatthu
Copy link

@jlalmes

@KATT KATT requested a review from jlalmes September 21, 2023 16:13
@yoannfleurydev
Copy link

yoannfleurydev commented Sep 28, 2023

@lucasthevenet I think you forgot a workspaces for a full working PR. Overall, it is working well.

image

@dBianchii
Copy link

I neeeeeed it.
<3

@lucasthevenet
Copy link
Author

Hey everyone, sorry I haven't had much time to continue this PR lately. I think with the last fix for the example everything should be working now.

@rwieruch
Copy link

Looking forward to it @lucasthevenet Thanks for your work here! 💯

@dngpng
Copy link

dngpng commented Nov 12, 2023

Really hope this gets merged soon 🙏

@gabe-zamp
Copy link

Can we get this as a Christmas gift? LOL

@jlalmes

@rogerclark
Copy link

Can someone merge this please?

@itsjoeoui
Copy link

+1 one this! 🙏

@ethndotsh
Copy link

+1, LGTM

@musjj
Copy link

musjj commented Jul 1, 2024

Ripped out src/adapters/fetch.ts and tested it on my very small project. So far it seems to function well both locally and in production (deployed with Vercel).

Would be really cool if this can be merged.

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.

Feat: Fetch adapter