AlexJPotter / fluentvalidation-ts

A TypeScript-first library for building strongly-typed validation rules
Apache License 2.0
87 stars 6 forks source link

Checking the Result #30

Open leherv opened 2 years ago

leherv commented 2 years ago

Hi, really like the library. I was looking at class-validator and immediately hit the problem of not being able to validate properties together or set a validator for a nested object. I also have my roots in C# so I found your library via fluentvalidation. The only problem I am having right now is that it is not very convenient to check if the result is actually failed validation or a successful one. Is this intentional? I would expect to be able to call .isValid() on the result of the validation. How would you recommend checking for success? Checking if there are no keys on the result via Object.keys(result).length === 0?

leherv commented 1 year ago

I ended up using the above described solution and have another suggestion: I just noticed that properties which were not set to "type | undefined" using "?" are not required by default. They are not even required when I use e.g.: "NotEmpty()". That is kind of confusing as I have to call .NotNull().NotEmpty() to check that something exists AND is not empty even though the property has to exist on the type and can not be undefined. Additionally using NotNull for this purpose is misleading as null and undefined are two different things.

celluj34 commented 1 year ago

I would also really prefer to have an isValid property on the result. Otherwise I have to repeatedly call Object.keys and that's cumbersome.

Here's a subclass I typed up quick to refactor the isValid property:

export class MyValidator<T> extends Validator<T> {
  constructor() {
    super();
  }

  validate = (value: T) => {
    const result = super.validate(value);

    return {
      isValid: Object.keys(result).length === 0,
      ...result,
    };
  };
}
AlexJPotter commented 8 months ago

Hi @leherv - thanks for your feedback 😃

The only problem I am having right now is that it is not very convenient to check if the result is actually failed validation or a successful one. Is this intentional?

This is due to the way the library was initially designed to plug and play as seamlessly as possible with Formik, which expects an object with keyed validation errors. I will consider how I can make it easier to check for validity, but for now the wrapper class proposed by @celluj34 above looks great (thanks!).

I just noticed that properties which were not set to "type | undefined" using "?" are not required by default. They are not even required when I use e.g.: "NotEmpty()". That is kind of confusing as I have to call .NotNull().NotEmpty() to check that something exists AND is not empty even though the property has to exist on the type and can not be undefined.

I'm not sure I understand fully what you mean here - would you be able to provide some code samples to show the behaviour you're mentioning? If a property on your type is required (not optional) yet is missing in values at runtime then I would say your type should make that property optional, and you will need to validate it explicitly with a .NotNull rule in your validator. TypeScript types are a compile-time concept - at runtime there is no way for fluentvalidation-ts to detect that a property in your type is required and implicitly perform a null-check. I hope that makes sense!

Additionally using NotNull for this purpose is misleading as null and undefined are two different things.

This is a fair point - I will consider adding a .NotUndefined rule and adding a parameter to .NotNull to configure the behaviour of doing a specific (triple-equals) null-check rather than the default of a double-equals check (that would include undefined).