search

vacekj / express-json-validator-middleware

Express middleware for validating requests against JSON schema
https://npm.im/express-json-validator-middleware
MIT License
159 stars 26 forks source link

Suggestion/Question: Incorporate @apideck/better-ajv-errors option #123

Open shane-js opened 4 months ago

shane-js commented 4 months ago

@apideck/better-ajv-errors is a pretty popular library for parsing ajv errors, at the time of writing this npmjs.com shows it at roughly ~2.8m downloads a week (https://www.npmjs.com/package/@apideck/better-ajv-errors).

The main function exposed by that library is betterAjvErrors() which needs to be passed not just the errors from ajv but the schema and the data that was used with ajv's validate function.

Being that this package (express-json-validator-middleware) sits in the middle of ajv & express by the time we can access the errors it is too far downstream to have on hand references to the data or the schema that was passed to the validate middleware method.

Would there be appetite for officially supporting an option to have the errors that are passed to next() be parsed by @apideck/better-ajv-errors?

Some pro's & con's I can think of quickly:

Pros: 1) this is already a library that is very convenient so seems to double down on the purpose 2) currently missing any flexibility on errors that make it to next() so it puts users in a position of having to incorporate parsing retroactively in the error handler rather than error creation 3) May bring some new life into this package as better-ajv-errors is pretty popular

Cons: 1) adds a dependency to @apideck/better-ajv-errors 2) I believe @apideck/better-ajv-errors only supports JsonSchema6 (I think this currently allows 5, 6, or 7).

I've taken a really quick & dirty stab at roughly what it could look like here (most likely missing somethings or not 100% right. Contains some breaking changes regarding constructor/middleware parameters as well.): https://gist.github.com/shane-js/58f1765e62ce6a170d3113e71c9b492c

If there is no interest in the above: If we didn't want to couple express-json-validator-middleware to that specific ajv error parsing package, perhaps instead we can look at a way to at least rework what gets passed to next() to include the data + schema that was used to produce the errors? That way at least downstream users could rework the errors as they see fit and to the needs of whatever they are exposing the errors to.

simonplend commented 2 months ago

Thanks for putting together this detailed proposal and proof of concept, and sorry for the delayed reply.

Unfortunately as @apideck/better-ajv-errors only supports JSON Schema Draft-06, and this library uses Ajv v8, which supports Draft-07 by default (docs), I don't think it's a good idea to integrate @apideck/better-ajv-errors.

I'd love to see a PR which adds data and schema into the error object that is passed to next() though. That would be a nice improvement and make things much more flexible for end users, who can then chose to integrate a library like @apideck/better-ajv-errors if they wish.