Feat: replace yup with joi for cleaner validation #2962
Conversation
|
Deploy preview for docusaurus-2 ready! Built with commit 8d89fcb |
| const UserOptionsSchema = Joi.object<UserPluginOptions>({ | ||
| fromExtensions: Joi.array().items(Joi.string().required().min(0)), | ||
| toExtensions: Joi.array().items(Joi.string().required().min(0)), | ||
| redirects: Joi.array().items(RedirectPluginOptionValidation) as any, // TODO Joi expect weird typing here |
There was a problem hiding this comment.
same problem with Joi here?
| from: PathnameValidator.required(), | ||
| to: PathnameValidator.required(), | ||
| }); | ||
|
|
||
| export function validateRedirect(redirect: RedirectMetadata) { | ||
| try { | ||
| RedirectSchema.validateSync(redirect, { | ||
| strict: true, |
There was a problem hiding this comment.
Not sure about this, but wouldn't it be better to be strict by default.
Ie using convert: false with Joi, to be more strict so that if we expect a number in the conf, the user does not try to pass "42" instead of 42 for example.
There was a problem hiding this comment.
we will be moving this validation to be done in life cycle method after the plugin is imported as done with other plugins
joi has different wording for the error so the snapshot test are failing. |
|
Then you can update the snapshots so that I can check the new error messages are decent :p and merge the PR |
|
@anshulrgoyal should we merge this before your other pr? can you resolve conflicts? |
|
Yes I can |
|
@slorber fixed merge conflict |
|
thanks :) |
slorber
added
the
pr: maintenance
Motivation
Replace yup with joi since joi provide better validation options out of the box
Have you read the Contributing Guidelines on pull requests?
yes
Test Plan
The existing test should be sufficient