fix: allow boolean for some validators in validation recipe

[aguscha333]

This PR fixes #4527

The Typing for ValidationRecipewas not consistent for some validators with the docs and what the actual VALIDATORS that validate uses accept for options.

I am refering to absence, acceptance, email, numericality, and presence accepting boolean for options but the ValidationRecipe only accepting the object version of options for each.

As you can see in the reported issue, this results in a type error when using boolean as options for a validator when using validate (the overload with optional messages) Here is the image of said error:

With my change, the error goes away:

The overload that requires messages shouldn't be affected by this change since it uses ValidationWithMessagesRecipe that overrides all validator typings. So you shouldn't be able to not pass a message to that overload by simply passing a boolean instead.
As you can see here for that case I receive the correct error from typescript:

[simoncrypta]

Hey @aguscha333 Thanks for this PR, I will take a look on it soon!
Meanwhile @dac09 or @cannikin can be interested to review it.

[aguscha333]

I see that there is a tests failing:

Not very familiar with the code yet and I am not sure how my changes could have affected that. Would apreciate some help debugging.

[simoncrypta]

Not very familiar with the code yet and I am not sure how my changes could have affected that. Would apreciate some help debugging.

Don't worry, It's not cause by your changes.

@jtoar I think your last PR didn't work out for this #4526

[jtoar]

@simoncrypta thanks! I'll keep an eye on it. We're also dealing with a OOM (or something along those lines) error in the Lint, Build & Test workflow that's suddenly sprung up on the Windows runner.

[cannikin]

This is Typescript so I'd put very little confidence in my review of this PR, but: looks good to me? 😬

Maybe the TS engine or linter or whatever does it isn't checking for type errors in tests? We've got plenty that use that boolean form:

expect(() => validate(val, 'email', { email: true })).toThrow(
  ValidationErrors.EmailValidationError
)

And there aren't any errors...maybe type problems are only warnings and don't stop the test suite?

[dac09]

Hi @aguscha333 thanks so much for this PR!

From a cursory glance, it doesn't quite look like the correct typing to me.

I think the fix would be in this line type WithOptionalMessage<T = Record<string, unknown>> = T & { here, where TS is actually complaining that boolean can't be assigned to unknown

You're totally right that this is a problem though. @cannikin let's catch up tomorrow to see if I can suggest a different fix? I get the TS, but I don't get validators, so our powers combined could be formidable ;)

Just going to hold this PR for now, I will update with recommendations soon

[aguscha333]

@dac09 thanks for taking a look.
Not sure I understand what solution you are thinking of. Looking at the line you mentioned, the def of WithOptionalMessage:

type WithOptionalMessage<T = Record<string, unknown>> = T & {
  /**
   * A message to be shown if the validation fails.
   */
  message?: string
}

If I understand correctly, it is not that boolean can't be assiged to unknown but that it can't be assigned to Record<String, unknown> as a whole. This is because instead of passing an object with options for some cases we just send a boolean since we already have all the default options for that particuar validator when the message is optional.

Doing something like:

type WithOptionalMessage<T = Record<string, unknown>> = boolean | (T & {
  /**
   * A message to be shown if the validation fails.
   */
  message?: string
})

Doesn't feel right either because not all validators that extend WithOptionalMessage are allowed to be passed just a boolean as value instead of the proper record. Only absence, acceptance, email, numericality, and presence accept a boolean as options. Something like length would require to be passed some options like min or max even if the message is optional.

At least that's the way I understand it, would love to hear your thoughts since I am not an expert in TypeScript.

[dac09]

Looked a bit further into this!

Thanks for your patience @aguscha333 - and you're correct this is the expected typing.

would love to hear your thoughts since I am not an expert in TypeScript

👍 Your TS is great, apparently my reading skills needed some improvement 😉 - sometimes hard to follow with the folded code in PRs