TypeScript: Validator return type is to broad when a list of choices is given

[kachkaev]

👋 @af!

Here is a small improvement idea for the library, given that it is written in TypeScript now. When using choices and passing a specific type to a validator, the return type seems to remain too broad. Here is an example:

import * as envalid from "envalid";

type AB = "A" | "B";

const env = envalid.cleanEnv(process.env, {
  CHOICE: envalid.str<AB>({
    default: "A",
    devDefault: "B",
    choices: ["A", "B"],
  })
});

const choice = env.CHOICE;
// typeof choice → string (expeced AB)

Adding <AB> after envalid.str already helps – I am no longer able to type default: "C" for example. This is great. However, for const choice to become AB, it is sill necessary to typecast the value of env.CHOICE as a workaround:

const choice = env.CHOICE as AB;
// typeof choice → AB

It’d be ideal if envalid did this type casting for us when the value for choices is provided!

[boennemann]

I'm having the exact same issue and it's funny enough you came across this just two hours ago :P

https://github.com/af/envalid/blob/main/src/validators.ts#L65-L66

To me it seems that the validator would need to pass T on to makeValidator and cast the input accordingly.
I tried by creating a custom validator, and it seems to work.

I have no idea of the implications for the rest of the library though, if this were to go into the default str function, or any of the validators.

[af]

I feel like this is a bug (and possible regression from v6?)– AB should definitely be the return type in your case. I'll try and investigate in the next week but probably won't get to it for the next day or two. PRs welcome in the meantime 👍

There was some previous discussion about changing how choices are represented in the API. I considered doing this for v7 but wanted to get it out the door and there were already a bunch of API changes. But it'd be nice to have this kind of API where the type param wouldn't be necessary for this common use case

[jkomyno]

I have bumped into the same error today, and I think I found a solution. PR coming soon.

[jkomyno]

Hi @af, @kachkaev, I have solved this problem, and hopefully I haven't introduced any regressions.
Please check my PR #146.

[af]

Closing as this should be addressed in #146. Check out the examples on that PR and please chime in if you foresee any issues before the next release