Skip to content
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

Inheriting Validation decorators #633

Open
2 tasks done
ghost opened this issue Jun 11, 2020 · 11 comments · May be fixed by #748
Open
2 tasks done

Inheriting Validation decorators #633

ghost opened this issue Jun 11, 2020 · 11 comments · May be fixed by #748
Labels
type: fix Issues describing a broken feature.

Comments

@ghost
Copy link

ghost commented Jun 11, 2020

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

  • nodejs: v12.17.0
  • typescript: 3.9.5
@cduff
Copy link
Contributor

cduff commented Jul 15, 2020

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.

@ghost
Copy link
Author

ghost commented Jul 15, 2020

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

@cduff
Copy link
Contributor

cduff commented Jul 23, 2020

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?

@Kiliandeca Kiliandeca added the type: fix Issues describing a broken feature. label Aug 27, 2020
@ragrub
Copy link

ragrub commented Sep 1, 2020

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.

NickKelly1 added a commit to NickKelly1/class-validator that referenced this issue Sep 7, 2020
@NickKelly1
Copy link
Contributor

There is now a PR for this

@cduff
Copy link
Contributor

cduff commented Feb 9, 2021

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.

@Arcdie
Copy link

Arcdie commented Apr 27, 2022

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;
}

@slavco86
Copy link

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.
🙏

@arturohernandez10
Copy link

Just FYI.

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

NoNameProvided pushed a commit to NickKelly1/class-validator that referenced this issue Dec 3, 2022
@huybuidac
Copy link

PR

hey bro, how did you fix this :(

@josephdpurcell
Copy link

josephdpurcell commented Mar 16, 2024

The documentation says inheritance should work: #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 #633 (comment) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: fix Issues describing a broken feature.
9 participants