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

Checking the Result #30

Open
leherv opened this issue Nov 25, 2022 · 3 comments
Open

Checking the Result #30

leherv opened this issue Nov 25, 2022 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@leherv
Copy link

leherv commented Nov 25, 2022

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
Copy link
Author

leherv commented Dec 12, 2022

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
Copy link

celluj34 commented Mar 5, 2023

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
Copy link
Owner

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

@AlexJPotter AlexJPotter added enhancement New feature or request question Further information is requested labels Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants