feat(client): improve handling status code types

[yusukebe]

The PR will improve the handling of TypeScript types of status codes in the Response returned by the client.

Fixes #2719
Fixes #3170
Fixes honojs/middleware#580

Allowing number for res.status

Imagine the following application which throws the HTTPException in the handler:

import { HTTPException } from 'hono/http-exception'
import { Hono } from 'hono'

const app = new Hono()
const flag = {}

const routes = app.get('/foo', (c) => {
  if (flag) {
    throw new HTTPException(500, {
      message: 'Server Error!',
    })
  }
  return c.json({ ok: true }, 200)
})

You will create the client with the types:

import { hc } from 'hono/client'

const client = hc<typeof routes>('/')
const res = await client.foo.$get()

The current pain point is it can't infer the response having 500 status correctly.

if (res.status === 500) {
  const data = await res.json()
}

The error showing in your IDE:

It should be fixed. However, handling the response content and status with an HTTPException or Error object is impossible. So, I've found the solution to enable accepting number for res.status.

if (res.status === 500) { // res.status can be `number`. **No** error!
  const data = await res.json()
}

Accept generics for res.json()

That solution solves the problem of the error of res.status, but the content returned from res.json() will not be typed:

if (res.status === 500) {
  const data = await res.json() // never. not typed
}

Also to set the correct types is impossible for it. So, I'll introduce res.json() accepting generrics. You can write the following:

if (res.status === 500) {
  const data = await res.json<{ message: string }>()
}

This method does not allow automatic inference, but it avoids type errors.

Other improvements

I added some modifications since other tests failed with the changes.

The author should do the following, if applicable

• Add tests
• Run tests
• bun run format:fix && bun run lint:fix to format the code
• Add TSDoc/JSDoc to document the code

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.20%. Comparing base (8f16802) to head (3670716).

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #3292    +/-   ##
========================================
  Coverage   96.20%   96.20%            
========================================
  Files         151      151            
  Lines       15293    15293            
  Branches     2761     2660   -101     
========================================
  Hits        14713    14713            
  Misses        580      580            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yusukebe]

Hey @m-shaka @NamesMT

Can you share your opinion on this PR and review it?

[m-shaka]

Ok, I can see that later!

[NamesMT]

The idea seems good to me,
If possible, I think adding a way to define a default type for unknown/certain status codes would be greatly appreciated, I'll try to bring up some PoC later tonight when I got back home.

[m-shaka]

generic typed json looks nice!

However, number type status may cause some problems.
First, users of the client can't know which status they should handle from status type.
For example, users can understand 200 and 400 should be handled from the type of status, without reading the server code.

import { Hono } from './src/'
import { hc } from './src/client'

const app = new Hono()
    .get('foo/:id', c => {
        if (c.req.param('id') === '1') {
            return c.json({ res: 400 as const }, 400)
        }
        return c.json({ res: 200 as const }, 200)
    })

const client = hc<typeof app>('')

client.foo[':id'].$get({ param: { id: '1' }}).then(async res => {
    switch (res.status) {
        case 200:
            return await res.json()
        case 400:
            return await res.json()
    }
})

Second, projects using @typescript-eslint/switch-exhaustiveness-check will be broken. You need to add default case even if you needn't.

As @NamesMT said, a default unknown status type possibly works

[NamesMT]

I pushed a sample for the DefaultResponse support at NamesMT/hono@9248417,
I don't know if this is most optimal way though, because DefaultResponse was added to many types.

Also, now that we have a DefaultResponse type, we could potentially mark this PR's feature (route's arbitrary status response) as opt-in: NamesMT/hono@2d0bfa7, this would prevent possible breaking of some projects that is currently using switch or if/else on the res.status, as well as prevent spam when user hovers on res.status.

src/_demo.ts for testing:

// server
import { HTTPException } from './http-exception'
import { Hono } from './hono'

const app = new Hono()
const flag = {}

const routes = app.get('/foo', (c) => {
  if (flag) {
    throw new HTTPException(500, {
      message: 'Server Error!',
    })
  }
  return c.json({ ok: true }, 200)
})

// client
import { hc } from './client'

type HCDefaultResponse = (
  {
    code: 'unknownErr'
    message: string
    detail: any
  } |
  {
    code: 'validationErr'
    message: string
    detail: string[][]
  }
)

const client = hc<typeof routes, HCDefaultResponse>('/') // Remove `HCDefaultResponse` to test the opt-in of route's arbitrary status response.

async function demo() {
  const res = await client.foo.$get()

  switch (res.status) {
    case 200:
        return await res.json().then(data => data.ok)
    default: {
      const data = await res.json() // Should be typeof `HCDefaultResponse`

      console.error(data.code === 'validationErr'
        ? { message: data.message, errorCount: data.detail.length }
        : { message: data.message })

      throw new Error(data.code)
    }
  }
}

[yusukebe]

@m-shaka @NamesMT Thank you!

As you said, we should support the default response type.

I like @NamesMT 's implementation. My proposal is to pass the default type generics into client.foo.$ge() instead of hc since a user will want to set default types for each endpoint:

const res = await client.foo.$get<DefaultResponseInFoo>()

How about this?

It includes diff for styling, but we can implement it with the following (diff from main...NamesMT:hono:feat/client-status-code-types_default_opt-in):

diff --git a/src/client/client.ts b/src/client/client.ts
index 13641299..512e7675 100644
--- a/src/client/client.ts
+++ b/src/client/client.ts
@@ -134,17 +134,16 @@ class ClientRequestImpl {
   }
 }
 
-
 /**
  * @param baseUrl The base URL of the Hono instance.
  * @param options The options for the client.
  * @returns A hono API client instance.
- * 
+ *
  * **Typedoc**: <Hono, DefaultResponse>
- * 
+ *
  * - Hono: The type of the Hono instance.
- * 
- * - DefaultResponse: This is used in cases where the response is unknown,  
+ *
+ * - DefaultResponse: This is used in cases where the response is unknown,
  * e.g. checking for an unknown status code:
  * ```ts
  * // The response of status code 400 is processed by a middleware and not explicitly defined in the router type flow.
@@ -153,7 +152,7 @@ class ClientRequestImpl {
  * }
  * ```
  */
-export const hc = <H extends Hono<any, any, any>, DefaultResponse = {}>(
+export const hc = <H extends Hono<any, any, any>>(
   baseUrl: string,
   options?: ClientRequestOptions
 ) =>
@@ -229,4 +228,4 @@ export const hc = <H extends Hono<any, any, any>, DefaultResponse = {}>(
       return req.fetch(opts.args[0], args)
     }
     return req
-  }, []) as UnionToIntersection<Client<H, DefaultResponse>>
+  }, []) as UnionToIntersection<Client<H>>
diff --git a/src/client/types.ts b/src/client/types.ts
index ef54e1fe..664fb4a5 100644
--- a/src/client/types.ts
+++ b/src/client/types.ts
@@ -25,12 +25,18 @@ export type ClientRequestOptions<T = unknown> = {
       headers: T | (() => T | Promise<T>)
     })
 
-export type ClientRequest<S extends Schema, DefaultResponse = {}> = {
+export type ClientRequest<S extends Schema> = {
   [M in keyof S]: S[M] extends Endpoint & { input: infer R }
     ? R extends object
       ? HasRequiredKeys<R> extends true
-        ? (args: R, options?: ClientRequestOptions) => Promise<ClientResponseOfEndpoint<S[M], DefaultResponse>>
-        : (args?: R, options?: ClientRequestOptions) => Promise<ClientResponseOfEndpoint<S[M], DefaultResponse>>
+        ? <DefaultResponse = {}>(
+            args: R,
+            options?: ClientRequestOptions
+          ) => Promise<ClientResponseOfEndpoint<S[M], DefaultResponse>>
+        : <DefaultResponse = {}>(
+            args?: R,
+            options?: ClientRequestOptions
+          ) => Promise<ClientResponseOfEndpoint<S[M], DefaultResponse>>
       : never
     : never
 } & {
@@ -66,8 +72,8 @@ type ClientResponseOfEndpoint<T extends Endpoint = Endpoint, DefaultResponse = {
   ? F extends 'redirect'
     ? ClientResponse<O, S extends RedirectStatusCode ? S : never, 'redirect'>
     : Equal<DefaultResponse, {}> extends true
-      ? ClientResponse<O, S extends StatusCode ? S : never, F extends ResponseFormat ? F : never>
-      :
+    ? ClientResponse<O, S extends StatusCode ? S : never, F extends ResponseFormat ? F : never>
+    :
         | ClientResponse<O, S extends StatusCode ? S : never, F extends ResponseFormat ? F : never>
         | ClientResponse<
             DefaultResponse,
@@ -158,33 +164,27 @@ export type InferRequestOptionsType<T> = T extends (
   ? NonNullable<R>
   : never
 
-type PathChaining<
-  Path extends string,
-  E extends Schema,
-  DefaultResponse = {}
-> = PathToChain<Path, E, Path, DefaultResponse>
+type PathChaining<Path extends string, E extends Schema> = PathToChain<Path, E, Path>
 
 type PathToChain<
   Path extends string,
   E extends Schema,
-  Original extends string = Path,
-  DefaultResponse = {}
+  Original extends string = Path
 > = Path extends `/${infer P}`
-  ? PathToChain<P, E, Path, DefaultResponse>
+  ? PathToChain<P, E, Path>
   : Path extends `${infer P}/${infer R}`
-  ? { [K in P]: PathToChain<R, E, Original, DefaultResponse> }
+  ? { [K in P]: PathToChain<R, E, Original> }
   : {
       [K in Path extends '' ? 'index' : Path]: ClientRequest<
-        E extends Record<string, unknown> ? E[Original] : never,
-        DefaultResponse
+        E extends Record<string, unknown> ? E[Original] : never
       >
     }
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-export type Client<T, DefaultResponse = {}> = T extends Hono<any, infer S, any>
+export type Client<T> = T extends Hono<any, infer S, any>
   ? S extends Record<infer K, Schema>
     ? K extends string
-      ? PathChaining<K, S, DefaultResponse>
+      ? PathChaining<K, S>
       : never
     : never
   : never

[m-shaka]

This is just my opinion, but server side should accept the default response type instead of client methods like $get, because server side is supposed to know which status code it returns.
It' best that you can know which status you have to handle from the type

However, I have a feeling that adding this functionality to server side requires much more effort than adding to client 🤔
The default response type on client works like an escape hatch and resolves the pain point you mentioned first so I think it's ok atm

[yusukebe]

@m-shaka Thank you for your comment!

@NamesMT What do you think of this API? #3292 (comment)

[NamesMT]

Hi @yusukebe, sorry I was a bit busy preparing for my country's holiday with family

All of the implementations make sense to me, though, it would be great if I can declare the default response once on the upper chain and use it for any routes, instead of having to pass it to every API call.

I'm thinking of something like this, it would allow configuring DefaultResponse at any level of API as needed and so hc is not required to be instantiated with DefaultResponse:

type HCDefaultResponse = (
  {
    code: 'unknownErr'
    message: string
    detail: any
  } |
  {
    code: 'validationErr'
    message: string
    detail: string[][]
  }
)

const hClient = hc<AppType>('/') // Current normal client

const hClientWithDefault = hClient.$withDR<HCDefaultResponse>() // Client with DefaultResponse applied from root level

const bookApi = hClient.book.$withDR<HCDefaultResponse>() // Client for book APIs with DefaultResponse applied

// Changing the DefaultResponse type as needed
const specialBookRouteRes = await bookApi.specialRoute.$withDR(SpecialDR).$post()

What do you think about it?

[yusukebe]

Hi @NamesMT

It looks good! We should consider the naming if we go with $withDR or get other names, but I like the idea (I think withDR is not bad).

[NamesMT]

Hi @yusukebe
Just wanna say that I'm super busy until October so I cannot work on this yet, holiday, trips and presenting a new project that I'm leading at work for September.

[yusukebe]

@NamesMT

Okay! Enjoy your time!