Fix: IsOptional decorator incorrectly ignores validations for null va…
…lues
Fix implementation to ensure @IsOptional only ignores validations when properties are undefined/missing, not when they're explicitly null.
Description
Checklist
- [x] the pull request title describes what this PR does (not a vague title like
Update index.md) - [x] the pull request targets the default branch of the repository (
develop) - [x] the code follows the established code style of the repository
-
npm run prettier:checkpasses -
npm run lint:checkpasses
-
- [x] tests are added for the changes I made (if any source code was modified)
- [x] documentation added or updated
- [x] I have run the project locally and verified that there are no errors
Fixes
fixes #[issue number], fixes #[issue number]
This intent behind this change contradicts what's currently stated in the documentation
@IsOptional() Checks if given value is empty (=== null, === undefined) and if so, ignores all the validators on the property.
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 undefined would trigger validation:
'bar' in { bar: undefined }; // true
The 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.
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 null as different from "omitted", so I agree that the need to distinguish the two use cases is real, and I agree that a new distinct decorator would probably the only feasible solution that does not impact preexisting projects.
a new distinct decorator would probably the only feasible solution that does not impact preexisting projects.
This would work, but could you also not have an opt-in configuration option when calling validate, similar to how skipMissingProperties works?
e.g.
validate(obj, { strictIsOptional: true });
When strictIsOptional is true, the behavior of @IsOptional() will match what's in this PR. When false (default) it would behave like it always has.