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: type of validation function #4283
Conversation
@Eomm ptal |
@@ -133,6 +138,8 @@ The optional parameter `httpPart`, if provided, is forwarded directly | |||
the `ValidationCompiler`, so it can be used to compile the validation | |||
function if a custom `ValidationCompiler` is provided for the route. | |||
|
|||
This function has property errors. Errors encountered during the last validation |
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.
Let’s add a small note just to indicate that this will only work this way of they’re using the default Fastify’s setup, if they’re tricking its own ValidationCompiler, most likely the behavior won’t be the same.
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.
could you arrange the text, please?
@@ -14,6 +15,11 @@ export interface RequestGenericInterface { | |||
Headers?: RequestHeadersDefault; | |||
} | |||
|
|||
export interface ValidationFunction { | |||
(input: any): boolean | |||
errors?: null | ErrorObject[]; |
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.
Shall we have also an unknown
type for allowing inference maybe?
Just to match it with my prior point that devs can implement its own compiler which bifurcates from the default behaviour?
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.
it seems to me that a separate issue with tests is needed here to account for this factor
In any case, I will not be able to handle this case (
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.
Could you please include a tsd test?
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.
lgtm
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Checklist
npm run test
andnpm run benchmark
and the Code of conduct