-
-
Notifications
You must be signed in to change notification settings - Fork 652
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
fix(context): single body overrides other returns #3800
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,10 +116,14 @@ interface NewResponse { | |
*/ | ||
interface BodyRespond { | ||
// if we return content, only allow the status codes that allow for returning the body | ||
(data: Data, status?: ContentfulStatusCode, headers?: HeaderRecord): Response | ||
(data: null, status?: StatusCode, headers?: HeaderRecord): Response | ||
(data: Data, init?: ResponseOrInit<ContentfulStatusCode>): Response | ||
(data: null, init?: ResponseOrInit): Response | ||
<U extends ContentfulStatusCode>(data: Data, status?: U, headers?: HeaderRecord): Response & | ||
TypedResponse<unknown, U, 'body'> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yusukebe not sure if this should remain as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
<U extends StatusCode>(data: null, status?: U, headers?: HeaderRecord): Response & | ||
TypedResponse<null, U, 'body'> | ||
<U extends ContentfulStatusCode>(data: Data, init?: ResponseOrInit<U>): Response & | ||
TypedResponse<unknown, U, 'body'> | ||
<U extends StatusCode>(data: null, init?: ResponseOrInit<U>): Response & | ||
TypedResponse<null, U, 'body'> | ||
} | ||
|
||
/** | ||
|
@@ -723,10 +727,14 @@ export class Context< | |
* }) | ||
* ``` | ||
*/ | ||
body: BodyRespond = (data, arg?: StatusCode | RequestInit, headers?: HeaderRecord) => { | ||
return typeof arg === 'number' | ||
? this.#newResponse(data, arg, headers) | ||
: this.#newResponse(data, arg) | ||
body: BodyRespond = ( | ||
data: Data | null, | ||
arg?: StatusCode | RequestInit, | ||
headers?: HeaderRecord | ||
): ReturnType<BodyRespond> => { | ||
return ( | ||
typeof arg === 'number' ? this.#newResponse(data, arg, headers) : this.#newResponse(data, arg) | ||
) as ReturnType<BodyRespond> | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2328,4 +2328,21 @@ describe('status code', () => { | |
// @ts-expect-error 204 is not contentful status code | ||
app.get('/', async (c) => c.html('<h1>title</h1>', { status: 204 })) | ||
}) | ||
|
||
it('.body() not override other responses in hono client', async () => { | ||
const router = app.get('/', async (c) => { | ||
if (c.req.header('Content-Type') === 'application/json') { | ||
return c.text('Hello', 200) | ||
} | ||
|
||
if (c.req.header('Content-Type') === 'application/x-www-form-urlencoded') { | ||
return c.body('Hello', 201) | ||
} | ||
|
||
return c.body(null, 204) | ||
}) | ||
|
||
type Actual = ExtractSchema<typeof router>['/']['$get']['status'] | ||
expectTypeOf<Actual>().toEqualTypeOf<204 | 201 | 200>() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is very good! |
||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.body()
now has a type ofResponse & TypedResponse
, so this change was made. The types are no longer compatible