fix(event-handler): http response body validation typings - #5125
Conversation
|
Thanks for the PR! The types in this part of the code are quite complex and I think we need to be quite careful here. I got Claude to generate some type level tests (which I should have added in my original PR) and I think I've found an issue. describe('Response validation typing', () => {
it('infers the output type for a standard response schema', () => {
const responseSchema = z.object({ id: z.string(), name: z.string() });
type Config = { res: { body: typeof responseSchema } };
expectTypeOf<InferResBody<Config>>().toEqualTypeOf<{
id: string;
name: string;
}>();
});
// passes on main, fails on your branch
it('infers the output type for a coerced response schema', () => {
const responseSchema = z.object({
id: z.coerce.string(),
name: z.string(),
});
type Config = { res: { body: typeof responseSchema } };
expectTypeOf<InferResBody<Config>>().toEqualTypeOf<{
id: string;
name: string;
}>();
});
it('rejects handler returning wrong types with a standard schema', () => {
const app = new Router();
const responseSchema = z.object({ id: z.string(), name: z.string() });
app.get(
'/users/:id',
// @ts-expect-error - number is not assignable to string for id
() => {
return { id: 123, name: 'John' };
},
{ validation: { res: { body: responseSchema } } }
);
});
// fails on main, passes on your branch
it('accepts handler returning pre-coercion types with z.coerce', () => {
const app = new Router();
const responseSchema = z.object({
id: z.coerce.string(),
name: z.string(),
});
app.get(
'/users/:id',
() => {
return { id: 123, name: 'John' };
},
{ validation: { res: { body: responseSchema } } }
);
});
// fails on main, passes on your branch
it('accepts handler returning a Response object with response validation', () => {
const app = new Router();
const responseSchema = z.object({ id: z.string(), name: z.string() });
app.get(
'/users/:id',
() => {
return Response.json({ id: '123', name: 'John' });
},
{ validation: { res: { body: responseSchema } } }
);
});
it('infers validated response body as output type for a standard schema', () => {
const responseSchema = z.object({ id: z.string(), name: z.string() });
type Config = { res: { body: typeof responseSchema } };
expectTypeOf<InferResSchema<Config>>().toEqualTypeOf<{
body: { id: string; name: string };
headers: undefined;
}>();
});
it('infers validated response body as output type for a coerced schema', () => {
const responseSchema = z.object({
id: z.coerce.string(),
name: z.string(),
});
type Config = { res: { body: typeof responseSchema } };
expectTypeOf<InferResSchema<Config>>().toEqualTypeOf<{
body: { id: string; name: string };
headers: undefined;
}>();
});
}); Here's what I think is happening: This doesn't currently break What we could do instead though is rather than changing type InferResBodyInput<V extends ValidationConfig> = V extends {
res: { body: infer S extends StandardSchemaV1 };
}
? StandardSchemaV1.InferInput<S>
: HandlerResponse; Then in the router overloads, use This way the handler return type uses the input type (accepts pre-coercion values) and |
pull-request-size
Bot
added
size/L
|
Isn't the input typing of z.object({
id: z.coerce.string(),
name: z.string(),
})results in that from the inferred input? Where as: z.object({
id: z.number(),
name: z.string(),
})
.transform((r) => ({
...r,
id: String(r.id),
}));has an input of I added the type checks you proposed, but they didn't seem to highlight an issue; and i was having a hard time following the type complexity that you were explaining. |
|
Additionally, is this expected to fail? A coerced response schema isn't the actual result body. it('validates a coerced response successfully', async () => {
// Prepare
const responseSchema = z.object({
id: z.coerce.string(),
name: z.string(),
});
app.get(
'/users/:id',
() => {
return { id: 123, name: 'John' };
},
{
validation: { res: { body: responseSchema } },
}
);
const event = createTestEvent('/users/123', 'GET', {});
event.pathParameters = { id: '123' };
// Act
const result = await app.resolve(event, context);
// Assess
expect(result.statusCode).toBe(200);
expect(result.body).toEqual('{"id":"123","name":"John"}');
}); |
Oh actually you're right! I got confused here with the coercion. Good to have these tests though, as I said, I should have added them originally. |
…s-lambda-typescript into http-validation-types
@svozza This isn't directly related to this issue; would you like to move to a separate discussion? |
Yes, makes sense. |
…s-lambda-typescript into http-validation-types
|
|
LGTM. Approved! |
|
@nateiler I think something similar is happening around the req path and headers if coercion attempted |
|
Hi @rcaughtlaf - if possible, please open a dedicated issue and reference this PR or the linked issue. Thanks! |
4 participants
Summary
The new http validation res body throws some type error when a schema's input and output are not the same. It also throws an error when returning a
Responseobject.Changes
Updated
TypedRouteHandlertypes to include aResponseobject and the input of the res body schema.On another note, it would be interesting to explore an option where the validated response could automatically update/replace the returned response.
const responseSchema = z.object({ id: z.coerce.string(), name: z.string() });That schema technically validates
{ id: 123, name: 'John' }, but the result is not{ id: "123", name: 'John' }as the response schema coerced it.Issue number: closes #5124
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.