-
Notifications
You must be signed in to change notification settings - Fork 841
Fix: IsOptional decorator incorrectly ignores validations for null va… #2615
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
base: develop
Are you sure you want to change the base?
Conversation
…lues Fix implementation to ensure @IsOptional only ignores validations when properties are undefined/missing, not when they're explicitly null.
|
This intent behind this change contradicts what's currently stated in the documentation
and what's been the de-facto behavior of the decorator for 5 years. Aside from that, the change is still incorrect, since a key explicitly set to 'bar' in { bar: undefined }; // trueThe PR should be rejected. |
|
Thanks for the feedback, I understand that the current behavior of @IsOptional() is to skip all validations if the value is null or undefined, even if the key is explicitly present in the request body. I also see that this behavior has been consistent for years and is part of the public contract. However, I believe this can lead to unintuitive results — for example, when a client explicitly sends null or undefined, validations like @isnotempty() are silently ignored. In practice, it might be more useful to skip validation only if the key is missing, and still run validations if the key is present, regardless of whether the value is undefined or null. That’s why I proposed this change. But I completely agree that changing the behavior of @IsOptional() directly would be a breaking change. Would it make sense to introduce a new decorator, such as @IsOptionalIfPresent() or @ValidateIfKeyExists(), that follows this alternative logic? This way we can cover this use case without affecting existing applications. |
(Just to avoid confusion: I am not a maintainer/contributor) speaking of unintuitive results: the whole library requires paying a great deal of attention (and I would add, of trust) to the documentation because inherently everything it does is magic (not really the lib's implementation's fault, but rather of its decorator-based nature); i stumbled upon this PR while looking myself if there was a way to treat |
This would work, but could you also not have an opt-in configuration option when calling e.g. validate(obj, { strictIsOptional: true });When |
…lues
Fix implementation to ensure @IsOptional only ignores validations when properties are undefined/missing, not when they're explicitly null.
Description
Checklist
Update index.md)develop)npm run prettier:checkpassesnpm run lint:checkpassesFixes
fixes #[issue number], fixes #[issue number]