class-validator icon indicating copy to clipboard operation
class-validator copied to clipboard

Inheriting Validation decorators

Open ghost opened this issue 4 years ago • 11 comments

Description

From the docs:

If a property is redefined in the descendant class decorators will be applied on it both from that and the base class.

I'm not sure it works as it should.

Reproduction

import { MinLength, IsUppercase } from "class-validator";
import { validate } from "class-validator";

class BaseClass {
    @MinLength(10)
    someProperty: string;
}

class ChildClass extends BaseClass {
    @IsUppercase()
    someProperty: string;
}

const someObj = new ChildClass();
someObj.someProperty = 'HELLO';

validate(someObj).then(result => {
    console.log(result);
});

// the output is empty array []

From what I'm reading in docs, I thought @MinLength(10) is used alongside @IsUppercase(), but I don't see it. Moreover, I'm not sure what empty array means.

Environment

  • [x] nodejs: v12.17.0
  • [x] typescript: 3.9.5

ghost avatar Jun 11 '20 12:06 ghost

I'm seeing the same. It does not appear to behave as documented.

@shymanel The empty array output from your example indicates no validation errors.

cduff avatar Jul 15 '20 14:07 cduff

@cduff yep, thanks, I figured it out. I though there should be an error, since HELLO is not 10 characters long

ghost avatar Jul 15 '20 14:07 ghost

FYI, since commenting on this issue earlier I've been using class-validator more and have come to realise that not having it inherit decorators is useful sometimes.

For example, I can have code like:

class User {
  @Matches(hashPattern)
  passwordHash?: string;
}

class RegistrationRequest extends User {
  @Length(minLength, maxLength)
  password?: string;

  @IsEmpty()
  passwordHash?: string;
}

For a User a valid passwordHash is required. But for a RegistrationRequest a valid password should be supplied instead, from which the server will generate a valid passwordHash. This wouldn't work if the Matches decorator was inherited to the RegistrationRequest.passwordHash property. To achieve the same would require some verbose groups usage I imagine, noting that in reality the User has many other properties not shown above.

So now I'm wondering if the decorators are not inheriting by design?

cduff avatar Jul 23 '20 05:07 cduff

I encounter the same issue described by @shymanel. At least with version 0.11.1 it used to work as mentioned in the documentation, that decorators were inherited from base class.

https://github.com/typestack/class-validator#inheriting-validation-decorators "user.password = 'too short'; // password wil be validated not only against IsString, but against MinLength as well"

I would like to update to version 0.12.2, but this seems to be broken now and affects a lot of unit tests in my project.

ragrub avatar Sep 01 '20 07:09 ragrub

There is now a PR for this

NickKelly1 avatar Sep 08 '20 01:09 NickKelly1

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.

cduff avatar Feb 09 '21 08:02 cduff

While we are waiting for merge, you can make your own decorator and call it in child class.

import {getMetadataStorage} from 'class-validator';

function inheritParentDecorators() {
  return (
    target: any,
    propertyKey: string,
  ) => {
    const storage = getMetadataStorage();
    const parent = Object.getPrototypeOf(target.constructor);

    if (!parent) {
      return;
    }

    const targetMetadatas = storage.getTargetValidationMetadatas(
      parent,
      parent.name,
      false,
      false,
    );

    targetMetadatas
      .filter(e => e.propertyName === propertyKey)
      .forEach(e => {
        registerDecorator({
          name: e.type,
          target: target.constructor,
          propertyName: e.propertyName,
          // preserving custom error messages (thanks [slavco86](https://github.com/slavco86))
          options: {...e.validationTypeOptions, message: e.message},
          constraints: e.constraints,
          validator: e.constraintCls,
        });
      });
  };
}

class BaseClass {
    @MinLength(10)
    someProperty: string;
}

class ChildClass extends BaseClass {
    @inheritParentDecorators()
    @IsUppercase()
    someProperty: string;
}

Arcdie avatar Apr 27 '22 09:04 Arcdie

Please, please, please - someone pick this up urgently. This is a clear regression from V11 and we are on V13 already without this being fixed. 🙏

slavco86 avatar Jul 25 '22 20:07 slavco86

Just FYI.

It only makes sense to inherit if the decorators don't conflict with each other.

arturohernandez10 avatar Sep 27 '22 21:09 arturohernandez10

PR

hey bro, how did you fix this :(

huybuidac avatar Jun 12 '23 00:06 huybuidac

The documentation says inheritance should work: https://github.com/typestack/class-validator/issues/633. But, it doesn't for me, I get:

Parameters violate constraints: policy must be one of the following values: [object Object]

I tried the solution from https://github.com/typestack/class-validator/issues/633#issuecomment-1110790596 and got the same error.

This is from @IsIn decorator on the parent class, but same error happens with other validators. I haven't dug in to find out why.

Within NestJS I've found a solution/hack that does the inheritance:

import { OmitType } from "@nestjs/swagger";

class ChildClass extends OmitType(BaseClass, []) {
    @IsUppercase()
    someProperty: string;
}

For now I can use the type helpers (pick/omit) from @nestjs/swagger but it would be great to see a better way to do this, even if its a custom decorator like @inheritParentDecorators.

Edit: clarified that I've read the docs.

josephdpurcell avatar Mar 16 '24 12:03 josephdpurcell