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

Proposal: defineValidatedEventHandler for typed body/query $fetch options #2244

Closed
1 task done
DallasHoff opened this issue Mar 11, 2024 · 7 comments
Closed
1 task done

Comments

@DallasHoff
Copy link

DallasHoff commented Mar 11, 2024

Describe the feature

Currently, the type of the method option in $fetch can be inferred from the server files present in the project. I would like to propose a way to infer the types for body and query for routes that use a new defineValidatedEventHandler function, implemented using similar techniques to the type inference for method. Here is roughly how I think it could work.

defineValidatedEventHandler is similar to h3's getValidatedQuery and readValidatedBody functions in that you pass a validation function (such as the parse method of a Zod schema) that it will then run to validate the input and infer the type. defineValidatedEventHandler would accept a function for query and/or body in an object before the handler argument.

function defineValidatedEventHandler<B, Q>(validators: {body?: ValidateFunction<B>, query?: ValidateFunction<Q>}, handler: EventHandler)

defineValidatedEventHandler would then use the validation functions to populate the generic of defineEventHandler.

const validatedBody = validators.body ? validators.body(body) : undefined;
const validatedQuery = validators.query ? validators.query(query) : undefined;

return defineEventHandler<{
  body: typeof validatedBody,
  query: typeof validatedQuery
}>(handler);

This would open the door for Nitro to extract the types for body and query from the event handler and use them in the NitroFetchOptions type alongside the method type it already has.

interface NitroFetchOptions<R extends NitroFetchRequest, M extends AvailableRouterMethod<R> = AvailableRouterMethod<R>> extends FetchOptions {
    method?: Uppercase<M> | M;
    body?: HandlerBodyType<R, M>;
    query? HandlerQueryType<R, M>;
}

This would probably involve generating a second interface in nitro-routes.d.ts alongside InternalApi to get the type from defineValidatedEventHandler.

interface InternalApiHandlers {
  '/api/foo': {
    'post': typeof import('../../server/api/foo.post').default
  }
}

With that, Nitro can create a type similar to the existing AvailableRouterMethod that extracts the types for body and query. Something along the lines of this with some extra type extends checks:

type HandlerBodyType<R, M> =
  InternalApiHandlers[MatchedRoutes<R>][M] extends EventHandler<infer R>
    ? R['body']
    : any;

With this all in place, the goal would be to have type inference for body and query in the options of $fetch and useFetch when calling any API route that uses defineValidatedEventHandler.

const bodySchema = z.object({
  id: z.string()
});

export default defineValidatedEventHandler(
  {
    body: bodySchema.parse,
  },
  async (event) => {
    // ...
  },
);
await $fetch('/api/foo', {
  method: 'POST',
  body: {}, // <-- body inferred as { id: string } from the Zod Schema, so this is a TS error
});

This completes the full circle of type integration between the Nitro API and the consumer by allowing both the inputs and outputs of fetches to be typed. Please let me know what you think.

Additional information

  • Would you be willing to help implement this feature?
@DallasHoff
Copy link
Author

I'd love to work on this. I just wanted to make sure that the concept sounds acceptable and does not conflict with any of the existing plans for Nitro. @pi0

@pi0
Copy link
Member

pi0 commented Apr 8, 2024

Hi dear @DallasHoff thanks for sharing idea and proposal. I think we want to introduce it as an option for defineEventHandler with object syntax to allow composition and avoid introducing more top level define methods.

@DallasHoff
Copy link
Author

@pi0 Okay, so you would favor the API to be a new overload of defineEventHandler called like this. Did I understand that correctly?

export default defineEventHandler({
  bodyValidator: bodySchema.parse,
  queryValidator: querySchema.parse,
  handler: async (event) => {}
});

Does the general idea seem sound otherwise?

@DallasHoff
Copy link
Author

@pi0 I went ahead and opened a draft pull request for the h3 half of the changes.

@DallasHoff
Copy link
Author

Draft PRs for both nitro and h3 are now up. The functionality is there for initial feedback; then I can work on adding additional tests and documentation.

h3 PR: unjs/h3#742
Nitro PR: #2405

@pi0
Copy link
Member

pi0 commented Jan 7, 2025

Let's track via unjs/h3#742 / unjs/h3#496

@pi0 pi0 closed this as completed Jan 7, 2025
@marknelissen
Copy link

@pi0 could you edit the link? The autolink from GitHub points to an unrelated nitro PR. The post of @DallasHoff contains the right one, but I will not be the only one at first confused when clicking on the link in your comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants