fix: skip optional validation for whitespace-only values #1349
Conversation
|
Thanks for the review request! |
src/context.ts
Outdated
| (value: any) => (optional === 'null' ? value != null : true), | ||
| (value: any) => (optional === 'falsy' ? value : true), | ||
|
|
||
| (value: any) => !(typeof value === 'string' && value === ''), |
There was a problem hiding this comment.
This is a breaking change. Would like to avoid it. What do you think of adding a new type of optional value, called empty?
It's very similar to falsy, except it's for strings only.
| // #331 - When multiple locations are involved, all of them must pass the validation. | ||
| // If none of the locations contain the field, we at least include one for error reporting. | ||
| // #458, #531 - Wildcards are an exception though: they may yield 0..* instances with different | ||
| // paths, so we may want to skip this filtering. |
There was a problem hiding this comment.
This is important context to retain
src/optional-whitespace.spec.ts
Outdated
There was a problem hiding this comment.
Instead of this suite that's heavily mirroring your examples, you should add a test for the new option to the following suite instead:
express-validator/src/context.spec.ts
Line 426 in 24da8b7
|
Thanks for the feedback — that makes sense. I’ll avoid changing existing optional behavior and instead add a new optional('empty') type for string-only empty values. I’ll also restore the removed context comments and move the tests into context.spec.ts as suggested. Updating the PR now. |
|
I’ve updated the implementation to avoid the breaking change and introduced a new optional('empty') mode that skips validation only for empty strings. I restored the explanatory comments you pointed out and moved the tests into context.spec.ts under #getData(). The separate spec file has been removed. All tests and lint checks are passing locally. Please let me know if any further adjustments are needed. |
4 participants
Fixes #1348
This PR improves validation behavior for optional fields when values contain
only whitespace and are trimmed to an empty string.
When optional() is used, whitespace-only values are now skipped cleanly
instead of producing confusing validation errors. The change is minimal,
backward-compatible, and covered by targeted tests.
All existing tests pass and new tests cover the reported edge cases.