FoalTS / foal

Full-featured Node.js framework, with no complexity. 🚀 Simple and easy to use, TypeScript-based and well-documented.
https://foalts.org/
MIT License
1.89k stars 139 forks source link

Request: Support $data on AJV configuration #920

Closed ZakRabe closed 3 years ago

ZakRabe commented 3 years ago

Today I was trying to use the ValidateMultipartFormDataBody hook to validate user registration data. But I ran into an issue with the AJV instance

If I wanted to reference the value in the password, for the password confirmation, I would use a schema that looks like this

password: {
  type: "string",
  maxLength: 255,
  minLength: 7,
  pattern: "^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-]).{7,}$",
},
confirmPassword: {
  const: {
    $data: "1/password",
  },
  type: "string",
},

However AJV doesn't support this feature unless the $data: true option is passed at initialization. See here in the AJV documentation for the attribute.

I saw in #633 that not all the options for the AJV instance are supported in the configuration, and you'd be adding support as they are requested.

I also confirmed for myself that the option isn't being passed.

https://github.com/FoalTS/foal/blob/1dea1e481209cc65693f26db3ca655998c5898d7/packages/core/src/common/utils/get-ajv-instance.ts#L23-L34

Obviously its pretty simple to do this sort of check in the actual controller code, or another hook, so this issue isn't blocking. However I think this usage of the JSON Schema to validate password confirmations is a pretty common use case, and warrants supporting in FoalTS.

If you'd like me to submit a PR, I'd be willing to, as it's a pretty trivial change. I'm just not sure how to best test a change like this to pass the PR approval process.

Thanks.

LoicPoullain commented 3 years ago

Sounds a good idea. 👍 Feel free to open a PR @ZakRabe .

About the tests, the should accept custom configuration from the Config. will probably need an update. We will also probably need a test similar to should throw a ConfigTypeError when the value of "settings.ajv.allErrors" has an invalid type. but with the $data setting.

LoicPoullain commented 3 years ago

Feature added in v2.4.0