-
-
Notifications
You must be signed in to change notification settings - Fork 616
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: change #toString()
behaviour for arrays
#1002
feat: change #toString()
behaviour for arrays
#1002
Conversation
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.
This is great. Thanks!
oops, just realised you didn't mark it as ready for review 😓 |
😁 I was still thinking about a few edge cases, but if they all pass I will just implement tests. |
Any edge cases that you could think of? 🙂 |
Maybe wildcards and our custom validators for arrays. |
1ac546b
to
12db9f4
Compare
564341f
to
40230df
Compare
taking this one over! |
347358a
to
1384718
Compare
To answer this:
|
994caa9
to
93b3761
Compare
da9246e
to
7145fea
Compare
if (this.negated ? result : !result) { | ||
context.addError(this.message, value, meta); | ||
} | ||
const values = Array.isArray(value) ? value : [value]; |
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.
@fedeci This row is causing issue for some validation.
For example, isInt, IsIn validators are not validating correctly for empty array value.
const validator = checkSchema({ field: { isInt:{} }});
const req = { body: { field: [] } };
await validator.run(req);
const errors = validationResult(req);
For above code, errors shouldn't be empty. But errors become empty because of above row.
Any idea?
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.
Yes, it was meant to be like that. If you want to validate each item of the array you need to use
const validator = checkSchema({ 'field.*': { isInt:{} }});
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.
No, field
should be number. But there can be case that field
in request is array. In that case, error should be triggered.
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.
I think it is bug. Right? @fedeci
Any idea to fix?
Description
To-do list