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

Switch to the multipart fetch exchange for urql to support file uploads #14

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

airhorns
Copy link
Contributor

@airhorns airhorns commented Nov 21, 2021

This switches us over to using the built in bits of urql that support file uploads over graphql. We're gonna use https://github.com/jaydenseric/graphql-upload on the backend which @urql/exchange-multipart-fetch works with well! This exchange is a drop in replacement for the normal fetch exchange if you're not uploading files also, so we can roll it out ahead of time.

All the integration testing of this thing is gonna happen Gadget side.

See https://formidable.com/open-source/urql/docs/api/multipart-fetch-exchange/ for more info on it.

Copy link
Contributor

@thegedge thegedge left a comment

Choose a reason for hiding this comment

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

Well that was easy! 🎉

@@ -257,7 +258,7 @@ export class GadgetConnection {
fetch: this.fetch,
exchanges: [
dedupExchange,
fetchExchange,
multipartFetchExchange,
Copy link
Contributor

Choose a reason for hiding this comment

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

Any performance issues to using this over fetchExchange? If yes, perhaps down the road we could provide a set of default exchanges from the generated API clients, based on whether or not there are file fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The multipart fetch exchange does exactly the same thing as the fetchExchange when there's no files involved, it only does multipart stuff if there are files, so I don't think so! Woop woop!

@airhorns airhorns merged commit 8736c20 into main Nov 22, 2021
@airhorns airhorns deleted the multipart-fetch-exchange branch November 22, 2021 14:30
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.

2 participants