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 'some' and 'every' as additional modifiers #57
Comments
Uhm, I don't think the position of some is sound in your example. I believe the modifier should be on the thing that is checked. As you wrote it one could assume that Generally I think the idea is nice. If you want to submit a PR for discussion do feel free. I'm not a member, but the feature is useful enough to be a core feature in my opinion. This would be better syntax: v8n()
.number()
.some.even()
.test([1, 2, 3]); // true
v8n()
.number()
.some.even()
.test([1, 3]); // false
v8n()
.number()
.every.even()
.test([1, 2, 3]); // false
v8n()
.number()
.every.even()
.test([2, 4]); // true Modifiers so far only apply to the rule they are attached to, so they would need to be repeated for Please do implement it though and add some tests to show useful examples and syntax. |
Ah, yes, the
I will start working on an implementation to share so it is easier to discuss. Also, thank you for the opportunity to try and contribute. 😃 |
Considering I'm not formally part of the project I can't give you the opportunity, but you are always free to give us your ideas and implementations. About your points:
|
Cool, let me see what I can whip up. |
Hey everybody, this feature looks very interesting indeed. The syntax suggested by @sebastianbarfurth sounds a little better for me. I like the idea to make validations for working upon arrays in some cases: v8n()
.array()
.length(4)
.every.positive()
.test([1, 2, 3, -2]); // false This is going to save us from having to write a lot of array specific validations, and it will give us so much flexibility. Actually, the v8n()
.some.exact(5) // .includes(5)
.test([1, 2, 3, 4, 5]); // true For this behavior to be achieved we don't need to change how modifiers are supposed to work right now. I think we should keep them tied to the next rule. But the problem is that today we are passing the 'invert' argument to the Rule constructor. Maybe we need to change it to receive a collection of modifiers instead. And this is going to break the API though. |
Yes, I agree the syntax suggested by @sebastianbarfurth is better. Based on my reading of the code the use of test(value) {
return this.chain.every(rule => {
try {
return rule.fn(value) !== rule.invert;
} catch (ex) {
return rule.invert;
}
});
} is tightly coupled to the implementation of In order to allow Check out a reference implementation here for a clarity. Speaking out aloud:
v8n()
.array()
.some()
.number()
.even()
.test([1, 2, 3]); // true |
For now, let's keep modifiers for rules only, without globals. It looks more clear to me when we are declaring validation strategies. I agree with you that the What we can do is to move the |
Sounds good. I will implement a draft where every |
Maybe you can move the |
Interesting, I will evaluate that. |
@adityasharat are you making progress on this feature? This is a tricky one, and it requires a lot of core refactoring. |
I was planning to work on this on Monday. I will publish a PR by Monday EOD if you want to see some progress. Definitely a little tricky, but I have some ideas. |
@adityasharat Actually, I was playing around with this feature trying to get a better understanding of the problem context, and I end up with a complete implementation. I'm not sure if this is the ideal implementation, but I can make a PR and you maybe can help me to validate that? |
Wow, sure man! Please go ahead, I would love to see the implementation. |
I made the PR at #61 |
This is pretty much in line with what I was planning. Awesome! Would love to see this change in the next version. |
Yes, this is going to be in 1.1.0 😁 |
Proposing to add
some
andevery
as new modifiers for testing array elements.An argument can be made for the following instead:
But personally, I find the former more elegant.
p.s. I can work on this feature request.
The text was updated successfully, but these errors were encountered: