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

Add a .catching() method to Predicate to allow nesting ow validators #154

Closed
wants to merge 8 commits into from

Conversation

Jblew
Copy link

@Jblew Jblew commented Jul 29, 2019

I love ow and use it heavily in my TS projects. I use a pattern in which I create a validator function for my interfaces. This validator function uses multiple ow calls to guard the properties. Often an interface is used as a type of a property inside another interface, and that interface also has a validator. It would be very handful if I could just validate this specific property against an already defined validator. Speaking code my pull request allows to do the following:

import ow from "ow";
import { PublishableOperation } from "./PublishableOperation";

export interface PublishJob {
    ops: PublishableOperation[];
    delegator: string;
}

export namespace PublishJob {
    export function validate(job: PublishJob) {
        ow(
            job.ops,
            "job.ops",
            ow.array
                .ofType(ow.object.catching(o => PublishableOperation.validate(o as PublishableOperation))),
        );
        ...
    }
}

This is a way I am achiving it now: https://github.com/wise-team/wise-hub/blob/b0ef49a78ff51e3c8a866513a7d13a68d4b8959a/backend/src/publisher/entities/PublishJob.ts My pull request allows the above way which is far more clean and readable. Of course I've added unit tests for the feature.

I named the method .catching() because it simply catches errors. Also, 'catch' is a keyword and I was not sure it is safe to use it in a property-method context. I am not sure If it is the best name.

I am really looking forward to use this feature!

Thank you in advance,
Jędrzej

@sindresorhus
Copy link
Owner

The feature makes sense. I'm just not convinced on the name. Too me it's not immediately obvious what's happening when reading ow.object.catching.

@sindresorhus sindresorhus changed the title Add a .catching() method to Predicate to allow nesting ow validators Add a .catching() method to Predicate to allow nesting ow validators Nov 12, 2019
@sindresorhus
Copy link
Owner

It feels more like an alternative version / overload of .validate(), but not sure we can reuse the name unambiguously.

@@ -240,6 +240,29 @@ ow(1, 'input', ow.number.validate(value => ({

This can be useful for creating your own reusable validators which can be extracted to a separate npm package.

#### catching(fn)

Use a custom validation function that throws error. The function should throw an error if value is invalid or return nothing when value is valid. This allows to use specific validators for complex property or array types.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear whether it support any error or only an Ow ArgumentError. If both, is there any different behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say let's support only the ow one. This way we are not swallowing anyones errors.


ow(animals, ow.array.ofType(ow.object.catching(animal => validateAnimal(animal as Animal))));
//=> ArgumentError: (array `animals`) (object) Expected number `Animal.weight` to be finite, got Infinity
```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this to the doc comment too?

@SamVerschueren
Copy link
Collaborator

So basically it's a shorthand for this?

const nestedOwValidator = (value: SpecificType) => {
	try {
		ow(value.x, ow.number.finite);
		return true;
	} catch (error) {
		return error.message;
	}
};

const arrayOfNumbers: SpecificType [] = [
	{x: 1},
	{x: Number.POSITIVE_INFINITY}
];

ow(arrayOfNumbers, ow.array.ofType(ow.object.is(o => nestedOwValidator(o))));

I agree that catching does not immediately tell you what it does. Not sure about a better name though 🤔 .

@sindresorhus
Copy link
Owner

@Jblew Still interested in finishing this?

@Jblew
Copy link
Author

Jblew commented Mar 10, 2020

Sorry for late response. Interested! .catching can be misleading I agree.

Then — what would you say for .nest(o => ow()) ?

@sindresorhus
Copy link
Owner

How about .custom()?

@sindresorhus
Copy link
Owner

Bump

@sindresorhus
Copy link
Owner

Closing for lack of response.

#188

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants