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

fix: ValidateNested do not pay attention of empty array as an item of an array #2264

Open
mtnt opened this issue Oct 4, 2023 · 1 comment
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.

Comments

@mtnt
Copy link

mtnt commented Oct 4, 2023

Description

I tried to validate some property expected as an array of objects. And ValidateNested worked ok with all invalid data except an empty array (fulfilled array was handled ok).

Minimal code-snippet showcasing the problem

import { IsOptional, IsNumber, ArrayNotEmpty, ValidateNested, IsBoolean } from 'class-validator';
import { Type } from 'class-transformer';

export class OneDTO {
  @ArrayNotEmpty({ always: true})
  @Type(() => TwoDTO)
  @ValidateNested({ always: true })
  data!: TwoDTO[];
}

class TwoDTO {
  @IsNumber({}, { always: true})
  id!: number;
}

Expected behavior

  • { data: 11 } - failed
  • { data: true } - failed
  • { data: [] } - failed
  • { data: [1] } - failed
  • { data: [[]] } - failed
  • { data: [[1]] } - failed
  • { data: [true] } - failed
  • { data: [{}] } - failed
  • { data: [{id: ''}] } - failed
  • { data: [{id: 1}] } - passed

Actual behavior

  • { data: 11 } - failed
  • { data: true } - failed
  • { data: [] } - failed
  • { data: [1] } - failed
  • { data: [[]] } - passed <<<<<<<<
  • { data: [[1]] } - failed
  • { data: [true] } - failed
  • { data: [{}] } - failed
  • { data: [{id: ''}] } - failed
  • { data: [{id: 1}] } - passed
@mtnt mtnt added status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature. labels Oct 4, 2023
@emir-gradient
Copy link

I think that @ValidateNested() is overwhelmed in a way that it has one responsibility too much.

Being able to handle this correctly:

class MySubClass {
      @MinLength(5)
      name: string;
    }

    class MyClass {
      @Contains('hello')
      title: string;

      @ValidateNested()
      mySubClass: MySubClass;

      @ValidateNested()
      mySubClasses: MySubClass[];

      @ValidateNested()
      mySubSubClasses: MySubClass[][];

      @ValidateNested()
      mySubSubSubClasses: MySubClass[][][];
    }

makes it unable to identify empty array as the problem. New empty array, for this validator, means just one more level in the main array, and not wrong object.

I think that probably best long-term solution (to not make breaking changes in sooner period for Users) is to make @ValidateNested() deprecated, and split it into two validators, @ValidateNestedObject() that would accept nested objects only (non array-like ones), and @ValidateNestedArray() that would have one additional mandatory parameter depth or arrayDimension where correct array dimension would be written by User.

Of course, the issue is redundant implementation, and that all Users can not upgrade to version of class-validator which gets @ValidateNested() removed from the library without completely removing it from their projects.

On the other side, to preserve @ValidateNested(), solution that I would say is not so nice, but might be acceptable, is to extend ValidationOptions interface with the optional nestedArrayDimension attribute, and if it is inserted by User, it would be respected, but if it is not, @ValidateNested() would behave as it currently does.

Finally, a an User that utilizes class-validator in many of my projects, I am very interested to contribute to it and try to support it to become even better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.
Development

No branches or pull requests

2 participants