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

A way to pass RequestInit on hc instance #2542

Closed
AlexisWalravens opened this issue Apr 23, 2024 · 9 comments
Closed

A way to pass RequestInit on hc instance #2542

AlexisWalravens opened this issue Apr 23, 2024 · 9 comments
Labels
enhancement New feature or request.

Comments

@AlexisWalravens
Copy link

AlexisWalravens commented Apr 23, 2024

What is the feature you are proposing?

Hi,

I'm using NextJS and I would like to be able to pass the RequestInit param of the fetch instance on individual calls.

Why ?

Because NextJS overrides fetch's RequestInit type to also include the next key to handle cache at the fetch level:

fetch('https://...', {
  next: { 
    revalidate: 3600,
    tags: ['some', 'tags'],
    cache: '...'
  }
})

https://nextjs.org/docs/app/building-your-application/data-fetching/fetching-caching-and-revalidating#time-based-revalidation

For now we only can override the whole fetch function, but I would like to be able to only override it's params with maybe something like this:

  const res = await hc.my.request.$get(undefined, {
    fetchOptions: {
      next: {
        revalidate: 3600
      }
    }
  })

It makes sense to me that since hc is just a fetch wrapper around fetch we should be able to pass it the same params.

For reference

I'm used to use ofetch (which is not NextJs related) which is a wrapper of fetch with some built in features like retries, errors on non 200/300 statusCode, json parsing, etc...

And as ofetch just extends RequestInit params from fetch I can use the next key directly in it just like I would with the basic fetch:

  const res = await ofetch('https://...', {
    method: 'GET',
    next: {
      revalidate: 3600
    }
  }) 

Maybe we could extends RequestInit and exclude the method & body keys since they are already defined by the hc method.

Thanks for reading me,
And Keep up with the good stuff 🔥 !

@AlexisWalravens AlexisWalravens added the enhancement New feature or request. label Apr 23, 2024
@yusukebe
Copy link
Member

Hi @AlexisWalravens

Because NextJS overrides fetch's RequestInit type to also include the next key to handle cache at the fetch level

Ahhhh, indeed, Next.js overrides fetch!

So, will the fetchOptions be like this?

type FetchOptions<ExtraTypes = Record<string, unknown>> = Omit<RequestInit, 'method' | 'body'> &
  ExtraTypes

@AlexisWalravens
Copy link
Author

AlexisWalravens commented Apr 24, 2024

I think it should work yes!

For reference, here is how NextJs overrides RequestInit
https://github.com/vercel/next.js/blob/850f4b9d8e31601ff9383fd11d79f67650fcc9e8/packages/next/types/global.d.ts#L25

@yusukebe yusukebe added the v4.3 label Apr 29, 2024
@yusukebe
Copy link
Member

yusukebe commented May 1, 2024

@AlexisWalravens

Sorry for the late response.

I've thought it. A method like client.index.$get can receive the fetch option, so perhaps you can pass the next option though the type warning is thrown. Can try it?

client.index.$get(
  {},
  {
    fetch(input, requestInit) {
      return fetch(input, {
        ...requestInit,
        next: {
          revalidate: 3600,
        },
      })
    },
  }
)

@AlexisWalravens
Copy link
Author

AlexisWalravens commented May 1, 2024

It works, but I think having to redeclare the entire fetch function every time is hurting the DX.

Also one more thing since we are talking about fetch, as I said above I'm used to use ofetch so I don't have to await res.json() or check if res.ok is true in case of 4xx/5xx responses.

I would like to propose a fetchRequester parameter to the hono client init:

That fetchRequester would be a function that would be used to make the requests instead of the default fetch.
And would of type:

type Requester<C = Record<string, never>> = <TResponse = unknown>(
  input: RequestInfo,
  init: RequestInit,
  options?: C
) => Promise<TResponse>

Here I would create a fetchRequester that would use ofetch instead of fetch:

type ExcludedFetchOptions<ExtraTypes = Record<string, unknown>> = Omit<
  FetchOptions,
  'method' | 'body' | 'responseType'
> &
  ExtraTypes

const fetchRequester: Requester = async <TResponse = unknown>(
  input: RequestInfo,
  init: RequestInit,
  options?: ExcludedFetchOptions
) => {
  return await ofetch<TResponse>(input, {
    ...init,
    ...options
  })
}

const client = hc<AppType>('https://api.example.com', {
  fetchRequester: fetchRequester
})

Here since we passed a fetchRequester, options would be typed directly from the fetchRequester's options parameter.
So here it would be typed as ExcludedFetchOptions instead of RequestInit since it's what I defined in the fetchRequester function.

const res = await hc.my.request.$get({}, options)

If no fetchRequest is passed, it would use the default fetch:

const client = hc<AppType>('https://api.example.com')
const res = await hc.my.request.$get({}, options)

and options in each endpoint request would just be typed as RequestInit.
Also if options is directly a RequestInit object, headers can be passed directly to it instead of a separate object.

Here I used ofetch, but it could be any other library that makes requests, like axios, ky, etc. Since we are able to get everything we need from the RequestInfo and RequestInit parameters.

The only thing missing now, would the ability to pass headers to the hono client initialization.
But we could make the params of hc like this:

type ExcludedRequestInit<ExtraTypes = Record<string, unknown>> = Omit<
  RequestInit,
  'method' | 'body' | 'responseType'
> &
  ExtraTypes

type HonoClientParams = {
  fetchRequester?: Requester
  requestInit?: ExcludedRequestInit
}

const hc = <T>(basePath: string, params?: HonoClientParams) => {
  // ...
}

// And then we could pass headers like this:
const client = hc<AppType>('https://api.example.com', {
  requestInit: {
    headers: {
      Authorization: `Bearer ${token}`
    }
  }
})

In that way the requestInit from the hc init would be the default passed to each request, but they could be overriden in each request if needed, like with the NextJS example RequestInit override.

In that way hc would have basically almost the same defaults, but could be used with any HTTP client and extended for complex use cases.

@yusukebe
Copy link
Member

yusukebe commented May 2, 2024

Hi @AlexisWalravens

Thank you for suggesting and explaining!

It works, but I think having to redeclare the entire fetch function every time is hurting the DX.

The hc function can receive fetch option which can set custom fetch method when client is initialized. Is it not enough?

const client = hc<typeof app>('http://localhoste', {
  fetch: (input, requestInit) => {
    return fetch(input, {
      ...requestInit,
      next: {
        revalidate: 3600,
      },
    })
  },
})

Also one more thing since we are talking about fetch, as I said above I'm used to use ofetch so I don't have to await res.json() or check if res.ok is true in case of 4xx/5xx responses.

Does this mean that if you use ofetch as fetchRequester, the res will not be fetch s Response? What will be assigned?

const res = await hc.my.request.$get({}, options) // If fetchRequester uses ofetch, res is not Response??

We designed the client by hc, which should return Response for fetch compatibility.

@yusukebe yusukebe removed the v4.3 label May 4, 2024
@devjmetivier
Copy link

@yusukebe Found this Issue with the same needs as @AlexisWalravens.

I find that your suggestions satisfy the need, but I wonder if there's some sort of higher order abstraction that could make this easier? Like a separate hono client that accepts Next's custom fetch options and passes them downstream?

export const client = hcNext<ApiType>('[baseUrl]');

//...

await client.your.route.here.$get({ query: {} }, { next: { tags: [...] } })

@yusukebe
Copy link
Member

Hi @devjmetivier and @AlexisWalravens !

#2592 introduced init option to each method:

const res = await client.about.me.$get(
  {},
  {
    init: {
      next: {
        revalidate: 3600,
      },
    },
  }
)

Does it not satisfy your request?

@devjmetivier
Copy link

#2592 introduced init option to each method:

Works perfectly! Thanks 😁

@yusukebe
Copy link
Member

@devjmetivier

Goood! I think we can close this issue. If you have any problem with this, feel free to re-open it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request.
Projects
None yet
Development

No branches or pull requests

3 participants