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

Type 'ClientResponse<T, U, F>' is not assignable to type 'void | Response'. #3750

Open
satoshun00 opened this issue Dec 13, 2024 · 9 comments
Labels
enhancement New feature or request.

Comments

@satoshun00
Copy link

satoshun00 commented Dec 13, 2024

What version of Hono are you using?

4.6.13

What runtime/platform is your app running on? (with version if possible)

Cloudflare Workers

What steps can reproduce the bug?

reproduction: https://github.com/satoshun00/hono-client-response-repro

import { Hono } from "hono";
import app from "./index";
import { hc } from "hono/client";

const client = hc<typeof app>("http://localhost:8787");

const app2 = new Hono()
  .post("/", () => {
    return client.index.$post();
  });

What is the expected behavior?

It should be possible to return the response (ClientResponse) from hono/client as the response (globalThis.Response) of the handler.

What do you see instead?

TypeScript Error

No overload matches this call.
  The last overload gave the following error.
    Argument of type '() => Promise<ClientResponse<{ message: string; }, StatusCode, "json">>' is not assignable to parameter of type 'H<BlankEnv, "/", BlankInput, HandlerResponse<any>>'.
      Type '() => Promise<ClientResponse<{ message: string; }, StatusCode, "json">>' is not assignable to type 'MiddlewareHandler<BlankEnv, "/", BlankInput>'.
        Type 'Promise<ClientResponse<{ message: string; }, StatusCode, "json">>' is not assignable to type 'Promise<void | Response>'.
          Type 'ClientResponse<{ message: string; }, StatusCode, "json">' is not assignable to type 'void | Response'.
            Type 'ClientResponse<{ message: string; }, StatusCode, "json">' is not assignable to type 'Response'.
              The types returned by 'json()' are incompatible between these types.
                Type 'Promise<{ message: string; }>' is not assignable to type 'Promise<T>'.
                  Type '{ message: string; }' is not assignable to type 'T'.
                    'T' could be instantiated with an arbitrary type which could be unrelated to '{ message: string; }'.ts(2769)

Additional information

@satoshun00
Copy link
Author

satoshun00 commented Dec 13, 2024

@EdamAme-x
I also tried to reproduce the issue in the TypeScript Playground, but I couldn't. So, I prepared a repository.
I suspect the cause might be related to the Response type from @cloudflare/workers-types 👀

@yusukebe
Copy link
Member

Hi @satoshun00

This is not a bug with unexpected behavior; the type of the response used in the RPC client and the type of the response returned in the handler are different.

@yusukebe yusukebe added not bug and removed triage labels Dec 14, 2024
@satoshun00
Copy link
Author

@yusukebe
Thank you for your response. I understand that the issue arises from the mismatch between the globalThis.Response type and the ClientResponse type. This type mismatch does not occur with the Response type in @types/node, but it seems to occur with @cloudflare/workers-types, and the result appears to be different from what I expected.
I would appreciate it if there is a good solution for this...
I don't think it's a very good idea, but how about allowing ClientResponse as a return type for Hono's HandlerResponse?

@yusukebe
Copy link
Member

The difference between the Response type in @types/node and @cloudfare/workers-types is whether it can accept generics in response.json() or not. For example, the code below will throw a type error with @types/node, but @cloudflare/workers-types's does now throw an error.

const response = new Response()
const data = await response.json<{ foo: string }>()

So, we can prevent the error by changing the definition of ClientResponse like the following though I'm not sure we should add the code as a fix:

diff --git a/src/client/types.ts b/src/client/types.ts
index 29e5790e..62235cad 100644
--- a/src/client/types.ts
+++ b/src/client/types.ts
@@ -89,11 +89,13 @@ export interface ClientResponse<
   url: string
   redirect(url: string, status: number): Response
   clone(): Response
-  json(): F extends 'text'
+  json<JSONT>(): F extends 'text'
     ? Promise<never>
     : F extends 'json'
-    ? Promise<BlankRecordToNever<T>>
-    : Promise<unknown>
+    ? undefined extends JSONT
+      ? Promise<BlankRecordToNever<T>>
+      : JSONT
+    : Promise<JSONT>
   text(): F extends 'text' ? (T extends string ? Promise<T> : Promise<never>) : Promise<string>
   blob(): Promise<Blob>

@satoshun00
Copy link
Author

When creating a Workers application that bypasses access to a specific Hono application, the fix would make me very happy as it eliminates the need for extra type casting. However, my concern would be that it might introduce unnecessary looseness in the ClientResponse.

Copy link

This issue has been marked as stale due to inactivity.

@github-actions github-actions bot added the stale label Dec 24, 2024
@yusukebe
Copy link
Member

I'll label this as an enhancement.

@yusukebe yusukebe added enhancement New feature or request. and removed not bug stale labels Dec 24, 2024
@satoshun00
Copy link
Author

It seems that a type mismatch still occurs even after applying the patch provided by @yusukebe.
Below is the error message:

No overload matches this call.
  The last overload gave the following error.
    Argument of type '() => Promise<ClientResponse<{ message: string; }, ContentfulStatusCode, "json">>' is not assignable to parameter of type 'H<BlankEnv, "/", BlankInput, HandlerResponse<any>>'.
      Type '() => Promise<ClientResponse<{ message: string; }, ContentfulStatusCode, "json">>' is not assignable to type 'MiddlewareHandler<BlankEnv, "/", BlankInput>'.
        Type 'Promise<ClientResponse<{ message: string; }, ContentfulStatusCode, "json">>' is not assignable to type 'Promise<void | Response>'.
          Type 'ClientResponse<{ message: string; }, ContentfulStatusCode, "json">' is not assignable to type 'void | Response'.
            Type 'ClientResponse<{ message: string; }, ContentfulStatusCode, "json">' is not assignable to type 'Response'.
              The types returned by 'json()' are incompatible between these types.
                Type 'undefined extends T ? Promise<{ message: string; }> : Promise<T>' is not assignable to type 'Promise<T>'.
                  Type 'Promise<T> | Promise<{ message: string; }>' is not assignable to type 'Promise<T>'.
                    Type 'Promise<{ message: string; }>' is not assignable to type 'Promise<T>'.
                      Type '{ message: string; }' is not assignable to type 'T'.
                        'T' could be instantiated with an arbitrary type which could be unrelated to '{ message: string; }'.ts(2769)

I couldn't come up with a solution for resolving the issue by modifying the ClientResponse type in Hono.

For now, in my project, I have implemented a temporary workaround by overriding the return type of response.json() from @cloudflare/workers-types in global.d.ts as follows:

declare interface Response extends globalThis.Response {
    readonly json(): Promise<unknown>;
}

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