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

Possible request validation support? #32

Closed
tobysmith568 opened this issue Apr 17, 2022 · 4 comments · Fixed by #35
Closed

Possible request validation support? #32

tobysmith568 opened this issue Apr 17, 2022 · 4 comments · Fixed by #35

Comments

@tobysmith568
Copy link
Contributor

  • I'm submitting a ...
    [x] feature request

Hey, would you consider adding built-in support for request model validation? Perhaps using a library like Zod?
(If so, I'd be happy to contribute / submit a PR with my thoughts on how it could look)

You can see here a file of mine which uses your library and has a custom method for validating the incoming request body.

Here you can see it after I refactored it to use Zod.

Zod + your library's catching of Error is a pretty good mix. The Zod method parse (used on line 20) throws a ZodError which means that posting an incorrect request body to my API in my project might result in the following example response:

{
  "success": false,
  "message": "[\n  {\n    \"code\": \"invalid_type\",\n    \"expected\": \"string\",\n    \"received\": \"undefined\",\n    \"path\": [\n      \"message\"\n    ],\n    \"message\": \"Required\"\n  }\n]"
}

Would you consider adding richer support for either Zod or another library of your choice so that the API response doesn't return JSON in a string?

I have a couple of thoughts on approaches:

  1. The lighter of the two could be modifications to your error-handler.ts file to specifically look for the ZodError and handle it there with zero config/setup required for consumers of your library. Users would still need to do what I've done on the first line of my postHandler where I call the validators parse method myself.

  2. The more in-depth approach could be something like:

const router = new RouterBuilder();
router.post(postHandler).withZodValidation(myZodValidator);
export default router.build();

Ideally this second approach would somehow modify the NextApiRequest type to have a strongly-typed req.body field. In this approach users wouldn't need to call the validator themselves because your library would call it for them.

Thanks for reading, I'm happy to hear any thoughts! :)

@Howard86
Copy link
Owner

Hi @tobysmith568, from my current project, the input validation (or request validation) is actually done in a middleware layer

For example, in the middleware example, we validate the email with external service, which can be replaced with other validation tools like zod, joi & yup

I have also thought about this as a standard middleware adaptor but it really depends on people's usage

@tobysmith568
Copy link
Contributor Author

Hey @Howard86.
Yeah that's on me, I should have looked into your middleware more deeply before posting.

Perhaps I'm missing something, is there a way to run middleware on specific HTTP methods?

I'd want to validate GET/POST/DELETE/etc differently, perhaps something like this?

router.use(myAuthMiddleware);
router.useOnPost(postBodyValidatorMiddleware);
router.useOnDelete(deleteBodyValidatorMiddleware);

router.get(getHandler);
router.post(postHandler);
router.delete(deleteHandler);

@Howard86
Copy link
Owner

Perhaps I'm missing something, is there a way to run middleware on specific HTTP methods?

I'd want to validate GET/POST/DELETE/etc differently, perhaps something like this?

Hmm looks like a quick solution. Do you have references that other libraries do something similar?

This new api will change the implementation of middleware and how it works with router, let me think about it...

If I were in a similar situation, I would probably finish everything in one handler (if this middleware is not supposed to be shared)

router.post(req => {
  const userDto = validateUser(req)
  return createUesr(userDto)
})

@tobysmith568
Copy link
Contributor Author

tobysmith568 commented Apr 17, 2022

No I don't have an example of another library which follows this pattern.

I would probably finish everything in one handler

Yeah, currently the first line of my handler is calling what would be the middleware.

The reuse I'm envisioning would be across endpoints rather than across http methods.
Perhaps for three different endpoints the GETs can be hit unauthorised but POSTs and DELETEs need to run through an auth middleware.

There's still no reason why the first line of all the POSTs and DELETEs can't be the call to the middleware though.

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

Successfully merging a pull request may close this issue.

2 participants