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

support fetch client #1387

Closed
7 tasks done
soartec-lab opened this issue May 20, 2024 · 37 comments · Fixed by #1353, #1407, #1457, #1461 or #1476
Closed
7 tasks done

support fetch client #1387

soartec-lab opened this issue May 20, 2024 · 37 comments · Fixed by #1353, #1407, #1457, #1461 or #1476
Assignees
Labels
enhancement New feature or request fetch Fetch client related issue
Milestone

Comments

@soartec-lab
Copy link
Member

soartec-lab commented May 20, 2024

I'm opening this issue to clarify the support status of the fetch client.

Description

I added a new fetch client.
Becouse, since the fetch API is mature, it has the advantage of reducing the bundle size of the application compared to using Axios, and I feel that this is sufficient in some cases.
Furthermore, we think it can be used to select an HTTP client with tanstack query or swr, or when calling an API from a server-side framework.

Ref: https://developer.mozilla.org/en-US/docs/Web/API/fetch

Todo

  • add a minimal fetch client using fetch API.
  • add a sample application using the fetch client with Next.js.
  • add tests for the fetch client.
  • add a document for the fetch client.
  • add other enhancements. For example, customization by specifying mutator.
  • support fetch as http fetcher in the swr client.
  • support fetch as http fetcher in the query client.
@soartec-lab soartec-lab added the enhancement New feature or request label May 20, 2024
@soartec-lab soartec-lab self-assigned this May 20, 2024
@melloware melloware added this to the 6.30.0 milestone May 20, 2024
@melloware melloware linked a pull request May 24, 2024 that will close this issue
3 tasks
@oferitz
Copy link

oferitz commented May 30, 2024

I think the main benefit of using Axios in this context is the easy creation of a custom instance with interceptors. The most common use case is adding an Auth header in a request interceptor. Do you see this as possible with the Fetch API? If so, could you provide an example as part of this effort? If that is a no-brainer, I would happily replace Axios with Fetch

UPDATE
Only after I wrote this did I see you already had a task there: "add other enhancements, for example, customization by specifying mutator." But it would be great to specifically demonstrate an async request interceptor

@soartec-lab
Copy link
Member Author

@oferitz

Thank you for give feed back 🙌
I agree that Axios interceptor is useful. But I didn't think about that for now.
However, I like your idea and will think about how to use that use0case, such as including a sample app or guide.

@jokull
Copy link

jokull commented Jun 2, 2024

Axios doesn't run on winter runtimes like Cloudflare, Vercel Edge and Deno. This is why the fetch adapter would be super useful.

@soartec-lab soartec-lab reopened this Jun 6, 2024
@melloware melloware modified the milestones: 6.30.0, 6.31.0 Jun 7, 2024
@soartec-lab soartec-lab linked a pull request Jun 9, 2024 that will close this issue
3 tasks
@soartec-lab soartec-lab reopened this Jun 11, 2024
@n2k3
Copy link

n2k3 commented Jun 12, 2024

Neat! Thank you @soartec-lab for your implementation. We've already switched to this instead of our own fetchClient.ts via a mutator override.

I've a question regarding handling different baseUrl's for multiple environments (local, test, prod, etc):

  • We've made a custom wrapper to handle authentication serverside, it has the following structure withAuth(fn, fnParams, fetchOverrideParams).
  • In orval.config.ts we've added the following: baseUrl: `$\{process.env.API_URL}`,. This works because the value is just being concatenated during code generation. See the Next.js example, here the value would be return `${process.env.API_URL}/pets`;.
  • This is so that we can keep using the benefit of having the env variable be evaluated at runtime on the server, instead of on build time.

But this probably isn't the intended way to implement this, do you have any suggestions regarding this?

@soartec-lab
Copy link
Member Author

Hi, @n2k3
Thanks for your comment.

baseUrl: $\{process.env.API_URL},

That's certainly works. And as far as I know, this is currently the only workaround for switching variables for each environment. This issue is independent of the fetch client, but other clients have similar problems, so I'll look into it.

@anymaniax
Copy link
Collaborator

@soartec-lab we could have a standalone object that could be overridable like axios is doing. Like here https://gist.github.com/anymaniax/44c1331a5643081a82da070e45f405f0#file-http-instance-ts-L34

@soartec-lab
Copy link
Member Author

It`s great, I would like to implement this when creating a custom instance 👍

@soartec-lab soartec-lab linked a pull request Jun 16, 2024 that will close this issue
3 tasks
@soartec-lab
Copy link
Member Author

Hey, @anymaniax

Could you please tell me how to use the HTTP_INSTANCE defined in the fetch sample you gave?

https://gist.github.com/anymaniax/44c1331a5643081a82da070e45f405f0#file-http-instance-ts-L34

I would like to implement something like an Axios interceptor as a sample.
Is this defined as a global variable? I would like to refer to your usage.

@soartec-lab
Copy link
Member Author

soartec-lab commented Jun 17, 2024

@oferitz @n2k3

The fetch client now supports custom mutators via #1457 🙌
As a result, it is now possible to intercept requests and responses, as well as dynamically transform base URL.

https://github.com/anymaniax/orval/blob/master/samples/next-app-with-fetch/custom-fetch.ts#L16-L29

I'm still looking for feedback, so please let me know if you find anything.

@soartec-lab soartec-lab linked a pull request Jun 18, 2024 that will close this issue
3 tasks
@melloware melloware added the fetch Fetch client related issue label Jun 18, 2024
@soartec-lab
Copy link
Member Author

@oferitz @davidysoards
Hi, there.
In the next version v6.32.0, you will be able to use fetch as an http client with react-query, vue-query, and svelte-query. If you are interested, please check the next version after release.

https://orval.dev/reference/configuration/output#httpclient

@davidysoards
Copy link

wonderful! thank you for all your hard work on this feature @soartec-lab 🙏

@xmd5a
Copy link

xmd5a commented Jul 18, 2024

@soartec-lab have you set a release date for version 6.32.0?

Thank you for this feature - it will help me a lot!

@melloware
Copy link
Collaborator

@xmd5a i just pinged @anymaniax about doing a 6.32.0 release.

@jamesleeht
Copy link
Contributor

Thanks for the great work on the fetch client - I noticed that formdata openapi specifications do not generate as expected:

export const createTranslation = async (createTranslationBody: CreateTranslationBody, options?: RequestInit): Promise<createTranslationResponse> => {
return customFetch<Promise<createTranslationResponse>>(getCreateTranslationUrl(),
  {      
    ...options,
    method: 'POST',
    body: JSON.stringify(
      formData,)
  }
);}

The formData does not exist in this case and as such this generated code will not work. Is this a known limitation at the moment?

@soartec-lab
Copy link
Member Author

soartec-lab commented Jul 21, 2024

@jamesleeht

Thank you for let me know.
I knew that, but i forget fix it. So, i'll fix that 👍

@jamesleeht
Copy link
Contributor

@soartec-lab you are amazing - thanks for such a quick fix! sent you some beer money 🍺

@soartec-lab
Copy link
Member Author

@jamesleeht
Thank you for made this report as well. Let's looking forward the next release together. And from now on too 🍻

@melloware
Copy link
Collaborator

7.0 is released lets close this ticket and let people open new individual bug tickets for fetch against 7.0

@davidysoards
Copy link

7.0 is released lets close this ticket and let people open new individual bug tickets for fetch against 7.0

👍
curious why this is a major release tho? no breaking changes are listed.

@anymaniax
Copy link
Collaborator

Because we had a lot of new feature and refactor lately. And we will completely remove some old api already mark as legacy. Also we moved the repository to an organization so it’s also to start a new era for the project

@jamesleeht
Copy link
Contributor

Is there plans to push 7.0 to the npm registry? It seems like installing from the release tarball causes it to be installed as orval-workspaces instead

@anymaniax
Copy link
Collaborator

It’s already on npm you can install it with orval@next

@koka0012
Copy link

Going to test! Currently working on a CRM and this lib will speed up the development a lot. Where can I leave feedback about this version?

@soartec-lab
Copy link
Member Author

@koka0012
Thanks. You can make new one in issues 🙌

@xmd5a
Copy link

xmd5a commented Jul 23, 2024

Thank you so much for adding this feature!

I have one question regarding the error handler; I see that react-query authors suggest handling fetch errors by explicitly adding the Error exception:
https://tanstack.com/query/latest/docs/framework/react/guides/query-functions#usage-with-fetch-and-other-clients-that-do-not-throw-by-default

What I see in orval is that the code just try to fetch and return data:

	const res = await fetch(getSomething(params), {
		...options,
		method: "GET",
	});

	const data = await res.json();

	return { status: res.status, data };

as a result, this line will fail if we get an error response (404 for example) from BE:
const data = await res.json();

is there a way to explicitly throw a custom error without manually modifying the code from orval?

@soartec-lab
Copy link
Member Author

@xmd5a
Copy link

xmd5a commented Jul 23, 2024

perfect! thank you so much @soartec-lab 🖤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment