sindresorhus / ow

Function argument validation for humans
https://sindresorhus.com/ow/
MIT License
3.81k stars 107 forks source link

Use Ow in conditional checks #114

Open sindresorhus opened 5 years ago

sindresorhus commented 5 years ago

I would like to be able to use Ow directly in conditionals:

if (ow(options.foo, ow.boolean)) {

}

The above doesn't work as Ow doesn't return a boolean when the validation passes.

We have ow.isValid(), but it only returns a boolean. I need it to return true when it passes and throw an error when it doesn't.


Alternatives considered: Having Ow accept a callback that is only executed when the condition passes. However, I would prefer using an if-statement if possible.

SamVerschueren commented 5 years ago

I don't see any downside in letting ow return true when it passes. 👌

SamVerschueren commented 5 years ago

I was thinking yesterday as to why we need this?

Aren't these identical?

function foo(input) {
    if (ow(input, ow.string)) {
         // heavy computation
    }
}

And

function foo(input) {
    ow(input, ow.string);

    // heavy computation
}

Because if it throws, it exits the function so why wrap it in an if-statement then? When it passes it will always continue.

sindresorhus commented 5 years ago

Yeah, my example is flawed. It makes more sense when you see my actual use-case. I didn't include that as it would also need .optional to work. https://github.com/imagemin/imagemin-pngquant/blob/0e0edd2ce0a379623ec06dd40236276027abf606/index.js#L52-L55

The key is that I only want to do a computation when the input is both defined and the correct value. If the input is undefined I do not want to do anything.

So I thought this could work:

if (ow(options.verbose, ow.optional.boolean)) {
    args.push('--verbose');
}

But that would return true when options.verbose === undefined, so what I'm really looking for is something like:

if (ow.isDefinedAndValid(options.verbose, ow.optional.boolean)) {
    args.push('--verbose');
}

Maybe I'm trying too hard to over-optimize this and maybe a callback would indeed be better for this use-case. I'm not sure anymore.

Shinigami92 commented 5 years ago

@sindresorhus If I understand you correctly, do you want something like that?:

ow.isDefinedAndValid(options.verbose, ow.optional.boolean, (err, value) => {
    if (err) {
        /* ... */
    }
    args.push("--verbose");
});

ow.isDefinedAndValid(options.verbose, ow.optional.boolean)
  .then(value => args.push("--verbose"))
  .catch(err => /* ... */);

try {
    const value = await ow.isDefinedAndValid(options.verbose, ow.optional.boolean);
    args.push("--verbose");
} catch (error) {
    /* ... */
}
sindresorhus commented 5 years ago

@Shinigami92 No, I would prefer an if-statement. If it were to be a callback, it would be:

ow.isDefinedAndValid(options.verbose, ow.optional.boolean, () => {
    args.push('--verbose');
});