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.

Sign up for GitHub

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

Jump to bottom

v2: Does not respect @IsOptional props #188

Closed
jbjhjm opened this issue Jan 26, 2022 · 4 comments
Closed

v2: Does not respect @IsOptional props #188

jbjhjm opened this issue Jan 26, 2022 · 4 comments
Labels
done

Comments

@jbjhjm
Copy link

jbjhjm commented Jan 26, 2022 •
edited
Loading

Oh dear, I found another one :O
Using a model like

class User {
  @Expose()
  name:string;
  @IsOptional()
  @Expose()
  @IsEmail({})
  email:string;
}

customValidateErrors will always contain an error for User.email.

Though with @IsOptional being set, validation should be skipped if email is undefined or null (and possibly empty string, too).
@jbjhjm
Copy link
Author

jbjhjm commented Jan 26, 2022

Another unexpected behavior: form.statusChanges emits VALID even though there are customValidateErrors.
It seems this happens if the only customValidateErrors are on fields that haven't been touched so far.

I don't know if statusChanges was intended to work correctly or if it is recommended to only subscribe to customValidateErrors instead. If so, I suggest to add to documentation which features of the original FormGroup should not be used.

EndyKaufman added a commit that referenced this issue Jan 30, 2022
@EndyKaufman
feat: add support native Angular validators (#183), support correct w…
7621b23 
…ork with isOptional (#188), add excludeGroups for fix recursive errors and unexpected creation of submodels (#185)

Add support native Angular validators #183
```
    this.form = this.fb.rootFormGroup(
      ComboCompany,
      {
        name: undefined, // issue #188
        regionNum: undefined, // issue #188
      },
      { angularValidators: { regionNum: [Validators.required] } } // issue #183
    );
```

v2: "Greedy" creation of nested Models causes recursive errors and unexpected creation of submodels #185
```
export class Company {
...
  @ValidateNested()
  @type(() => Department)
  @expose({ groups: ['nested'] })
  department?: Department | undefined;
...
}
...
    this.form = this.formBuilder.rootFormGroup(
      Department,
      this.getEmptyDepartment(),
      {
        classTransformOptions: {
          excludeGroups: ['nested'],
        },
      }
    );
...
```
@EndyKaufman EndyKaufman mentioned this issue Jan 30, 2022
Merged
EndyKaufman added a commit that referenced this issue Jan 30, 2022
@EndyKaufman
feat: add support native Angular validators (#183), support correct w…
d9d8c77 
…ork with isOptional (#188), add excludeGroups for fix recursive errors and unexpected creation of submodels (#185)

Add support native Angular validators #183
```
    this.form = this.fb.rootFormGroup(
      ComboCompany,
      {
        name: undefined, // issue #188
        regionNum: undefined, // issue #188
      },
      { angularValidators: { regionNum: [Validators.required] } } // issue #183
    );
```

v2: "Greedy" creation of nested Models causes recursive errors and unexpected creation of submodels #185
```
export class Company {
...
  @ValidateNested()
  @type(() => Department)
  @expose({ groups: ['nested'] })
  department?: Department | undefined;
...
}
...
    this.form = this.formBuilder.rootFormGroup(
      Department,
      this.getEmptyDepartment(),
      {
        classTransformOptions: {
          excludeGroups: ['nested'],
        },
      }
    );
...
```
@EndyKaufman
Copy link
Owner

error with IsOptional - fixed, about statusChanges please create example with fork https://stackblitz.com/edit/ngx-dynamic-form-builder-new-api

@EndyKaufman
Copy link
Owner

@jbjhjm

@jbjhjm
Copy link
Author

jbjhjm commented Jan 31, 2022

Thanks @EndyKaufman ! About reproduction, it will take a some time before I can free some time to write one. Will let you know then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
done
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jbjhjm @EndyKaufman