-
Notifications
You must be signed in to change notification settings - Fork 20
feat(next): Custom user defined validations #196
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
feat(next): Custom user defined validations #196
Conversation
|
Hi @SimoneErba! Thanks for your PR and first of all, sorry for the long delay in getting back to you. I'd like to accept your PR, but we need to make some adjustments. Unfortunately json-logic has no concept of scopes, instances or similar which means it would be possible for multiple uses of createHeadlessForm to add conflicting custom operators. This could happen in a SPA rendering many different forms. My proposal here is that we only add the custom ops before validation via |
aaf2ecd to
12a74bd
Compare
|
Hi @lukad, thanks for your review! It makes sense, I changed the code to add and remove the functions everytime we run the validation |
|
Hi @SimoneErba , there's a small issue with one of the lint jobs, if you can take a look we can move this to the finish line 🙂 |
|
Hi @antoniocapelo, can you run the CI again to see if it's all good now? Thanks |
| if (customJsonLogicOps) { | ||
| if (typeof customJsonLogicOps !== 'object' || customJsonLogicOps === null) { | ||
| throw new TypeError('validationOptions.customJsonLogicOps must be an object.') | ||
| } | ||
|
|
||
| for (const [name, func] of Object.entries(customJsonLogicOps)) { | ||
| if (typeof func !== 'function') { | ||
| throw new TypeError( | ||
| `Custom JSON Logic operator '${name}' must be a function, but received type '${typeof func}'.`, | ||
| ) | ||
| } | ||
| } |
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.
Suggestion
Moving this block to a validateCustomJsonLogicOps function would make things easier to read 🙂
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.
@antoniocapelo There was a validateOptions function, I moved it there
next/src/form.ts
Outdated
| const customJsonLogicOps = options?.validationOptions?.customJsonLogicOps | ||
|
|
||
| try { | ||
| if (customJsonLogicOps) { |
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.
Suggestion
Similar approach in the other comment, we could have 2 functions:
function addCustomJsonLogicOperations(ops) {
if (ops) {
for (const [name, func] of Object.entries(customJsonLogicOps)) {
jsonLogic.add_operation(name, func)
}
}
}
// similar one for removingSo this handleValidation would look like:
const customJsonLogicOps = options?.validationOptions?.customJsonLogicOps
try {
addJsonLogicCustomOperations(customJsonLogicOps)
const updatedSchema = calculateFinalSchema({
schema,
values: value,
options: options.validationOptions,
})
const result = validate(value, updatedSchema, options.validationOptions)
updateFieldProperties(fields, updatedSchema, schema)
return result
}
finally {
removeJsonLogicCustomOperations(customJsonLogicOps)
}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.
@antoniocapelo done
fix test add and remove functions each time rebase try to fix lint fix lint move code to new functions
ff90548 to
d7a0f7b
Compare
|
@SimoneErba I'm sorry about the delay — I'm about to approve the MR but it seems that because of a v1-release file movement, you'll need to also move your new test to the correct path. Sorry for the disturbance — let me know once it's done and I'll approve this + create a new release for the new feature |
|
Hi @antoniocapelo, sorry for the delay. In this branch I did a wrong rebase and so I made a new branch from main, but the problem is that if I run |
|
Hi @SimoneErba! For v1, we updated the Node version and you need to use cc @lukad |
Allow users to write their custom functions to use in jsonLogic. We register them once when we create the form. Added
customJsonLogicOpsto theValidationOptionsthat is a dictionary of name: functionI added the test in the v0 file because the new one doesn't use createHeadlessForm and mocks the jsonLogic