fix: overwrite validation decorators from parent class when defined in child

[NickKelly1]

Description

Fixes a bug that caused decorated properties which are also decorated in inherited classes, to skip their inherited validation.

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:check passes
  • npm run lint:check passes
• [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 #633 , fixes #622

[NickKelly1]

Please don't merge this yet, I've added some additional tests and found some issues. I Will comment again once resolved.

[NickKelly1]

This PR is now ready for merge

Please note:

Currently in v0.12.2, validator-decorated properties of subclasses remove validators registered against those properties from parent classes (see 633). It appears this has been a problem for a long time. Validator equality is done by comparing a validators type, which in v0.12.2 is "customValidation" for almost every single validator.

The bug became apparent after v0.11.1 (https://github.com/typestack/class-validator/commit/11a7b8bb59c83d55bc723ebb236fdca912f49d88#diff-6a5d045641599b923bd58ec30032f09dL1) wherein most validators had a unique ValidationType allowing subclassing to override them (note that custom validators still had type "customValidation", and thus would all override each-other by subclassing).

This PR changes equality to be based on name, rather than type, and falling back to type if name is not given. This makes subclassing to work closer to as described in the documentation. If name is now used to uniquely identify a validator, that implies it should be a required property. To not break backwards compatibility, it's left optional here with TODO's to add it where necessary if it becomes required later.

[cduff]

Any update on this? I got hit by it again today. It's quite confusing at the moment as some decorators are inherited (e.g. @IsOptional) and others aren't.

[rafaelbattesti]

Hey folks, any expectations on merging this change? It seems to me the semantic commit + PR is blocking it. Thanks!

[FacundoVazquez]

Please merge this fix, more than one is forced to use version 0.11.1. Thanks!

@NoNameProvided

[NoNameProvided]

Hi! I pinned the tab, I will take a look next week.

[StefanSafeguard]

Any update on the merge?

[Clashsoft]

Looking forward to this to be merged as well. In the meantime, I worked around the problem using ValidationTypes.isValid = () => true;, but it broke validation (everything is valid now). I'd like to use the type for a project that seems to depend on this PR completely.

[slavco86]

@NickKelly1 Any chance you can have a look at the conflicts, please? Dying to get this merged and released. Please.

[pahuta]

@NoNameProvided , @NickKelly1 , did you decide to do not merge it?

[kaseyreed]

👋🏻 Hi there, just wondering if this fix will land or if y'all have decided not to support this?

[slavco86]

This should really be merged... this is obviously a gap/bug, which anyone requiring this functionality would expect to be working? I mean this is a recursiveness problem, which should not be present in a production grade package. Quite surprised it's still not merged/fixed 😢

[AdirTrichter]

Any updates on this PR? Would it merge soon?

[AdirTrichter]

Does anyone know a workaround that can be done so it would inherit the validations from the parent till this PR merges? *other than rewriting the validations from the inherited class ofc 🙏